[M144][PEPC]: Move CachedPermissionStatus to ExecutionContext [chromium/src : refs/branch-heads/7559]
0 views
Skip to first unread message
Thomas Nguyen (Gerrit)
9:59 AM (10 hours ago) 9:59 AM
to Balazs Engedy, AyeAye, blink-rev...@chromium.org, permissio...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org
Attention needed from Balazs Engedy and Joey Arhar
Thomas Nguyen added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Thomas Nguyen . resolved
This is back merge CL request for https://chromium-review.googlesource.com/c/chromium/src/+/7421209
Note that Supplementable class is not available in M144, we have manually added CachedPermissionStatus to ExecutionContext, which provides the same functionality as Supplementable class.
Related details
Attention is currently required from:
- Balazs Engedy
- Joey Arhar
Submit Requirements:
Code-Owners
Code-Review
Lint
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Balazs Engedy (Gerrit)
10:07 AM (10 hours ago) 10:07 AM
to Thomas Nguyen, Chromium LUCI CQ, AyeAye, blink-rev...@chromium.org, permissio...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org
Attention needed from Joey Arhar and Thomas Nguyen
Balazs Engedy voted and added 2 comments![]( "Open in Gerrit")
Votes added by Balazs Engedy
| Code-Review | +1 |
2 comments
Patchset-level comments
Balazs Engedy . resolved
LGTM % one comment.
Confirmed that the `html_permission_element` changes and tests are the same, the only difference that is not a trivial merge is how CachedPermissionStatus is now keyed to ExecutionContext, which required some mechanical changes given the refactoring work on removing `Supplementable`'s that was done in M144 but reverted in M145.
File third_party/blink/renderer/core/execution_context/execution_context.h
Line 880, Patchset 2 (Latest):
Member<CachedPermissionStatus> cached_permission_status_;Balazs Engedy . unresolved
Do we need to remove this member from LocalDOMWindow?
Related details
Attention is currently required from:
- Joey Arhar
- Thomas Nguyen
Submit Requirements:
Code-Owners
Code-Review
Lint
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Nguyen (Gerrit)
10:17 AM (10 hours ago) 10:17 AM
to AyeAye, Balazs Engedy, Chromium LUCI CQ, blink-rev...@chromium.org, permissio...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org
Attention needed from Balazs Engedy and Joey Arhar
Thomas Nguyen voted and added 1 comment![]( "Open in Gerrit")
Votes added by Thomas Nguyen
| Commit-Queue | +1 |
1 comment
File third_party/blink/renderer/core/execution_context/execution_context.h
Line 880, Patchset 2:
Member<CachedPermissionStatus> cached_permission_status_;Balazs Engedy . resolved
Do we need to remove this member from LocalDOMWindow?
Attention is currently required from:
- Balazs Engedy
- Joey Arhar
Submit Requirements:
Code-Owners
Code-Review
Lint
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Balazs Engedy (Gerrit)
10:23 AM (10 hours ago) 10:23 AM
to Thomas Nguyen, AyeAye, Chromium LUCI CQ, blink-rev...@chromium.org, permissio...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org
Attention needed from Joey Arhar and Thomas Nguyen
Balazs Engedy voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Joey Arhar
- Thomas Nguyen
Submit Requirements:
Code-Owners
Code-Review
Lint
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
12:09 PM (8 hours ago) 12:09 PM
to Thomas Nguyen, Balazs Engedy, AyeAye, Chromium LUCI CQ, blink-rev...@chromium.org, permissio...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org
Attention needed from Thomas Nguyen
Joey Arhar voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Thomas Nguyen
Submit Requirements:
Code-Owners
Code-Review
Lint
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Nguyen (Gerrit)
1:38 PM (7 hours ago) 1:38 PM
to Balazs Engedy, AyeAye, Chromium LUCI CQ, blink-rev...@chromium.org, permissio...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org
Thomas Nguyen voted Commit-Queue+2![]( "Open in Gerrit")
Code-Owners
Code-Review
Lint
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
1:43 PM (7 hours ago) 1:43 PM
to Thomas Nguyen, Balazs Engedy, AyeAye, blink-rev...@chromium.org, permissio...@chromium.org, blink-revi...@chromium.org, blink-...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
[M144][PEPC]: Move CachedPermissionStatus to ExecutionContext
Refactor `CachedPermissionStatus` to be a supplement of
`ExecutionContext` rather than `LocalDOMWindow`.
By anchoring the cache to `ExecutionContext`, call sites can now
safely use `GetExecutionContext()`. This prevents the null pointer
dereference of `domWindow()` when the document does not have a
browsing context (for example document is created from
`cloneNode()`). Typically in this case the document still has a valid
`ExecutionContext` (the `domWindow()` of the Document in which they were
created)
Fixed: 468659211, 475127000
(cherry picked from commit bd22794b22d95b035dc2d79ce3b1584c1ad842e7)
Change-Id: I347526d67ad45130b7217b7e938007e8d9d212a6
Reviewed-by: Balazs Engedy <eng...@chromium.org>
Commit-Queue: Balazs Engedy <eng...@chromium.org>
Reviewed-by: Joey Arhar <jar...@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1567040}
Commit-Queue: Thomas Nguyen <tun...@chromium.org>
Cr-Commit-Position: refs/branch-heads/7559@{#3357}
Cr-Branched-From: 223dfbac1c7542a06b422390d954afe5b560b607-refs/heads/main@{#1552494}
Files:
- M chrome/browser/permissions/permission_element_browsertest.cc
- M third_party/blink/renderer/core/execution_context/execution_context.cc
- M third_party/blink/renderer/core/execution_context/execution_context.h
- M third_party/blink/renderer/core/frame/cached_permission_status.cc
- M third_party/blink/renderer/core/frame/cached_permission_status.h
- M third_party/blink/renderer/core/frame/local_dom_window.cc
- M third_party/blink/renderer/core/frame/local_dom_window.h
- M third_party/blink/renderer/core/html/html_geolocation_element_test.cc
- M third_party/blink/renderer/core/html/html_permission_element.cc
- M third_party/blink/renderer/core/html/html_permission_element_test.cc
Change size: M
Delta: 10 files changed, 57 insertions(+), 38 deletions(-)
Branch: refs/branch-heads/7559
Submit Requirements:
Code-Review: +1 by Balazs Engedy, +1 by Joey Arhar
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages