Implement rel=opener as a proactive BrowsingInstance swap opt-out [chromium/src : main]
Kevin McNee (Gerrit)
Kevin McNee added 1 comment![]( "Open in Gerrit")
Related details
- Charlie Reis
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Charlie Reis (Gerrit)
Charlie Reis added 1 comment![]( "Open in Gerrit")
Great! I'm excited to look through this, but I'm also a bit slower than usual because two other large reviews have come in at the same time. Rakina has offered to help review this as well, to get it moving more quickly-- thanks!
Related details
- Charlie Reis
- Rakina Zata Amni
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rakina Zata Amni (Gerrit)
Rakina Zata Amni added 5 comments![]( "Open in Gerrit")
Thanks! Glad that the actual logic change isn't that big. Mostly LGTM, just requesting some additional tests and comments :)
Let's add a brief comment on what these tests are about.
Can you add a test for cross-site navigations as well?
// navigation would break scripting in this case. See crbug.com/40281878 forMaybe it's a good idea to add an explicit test for this case? (Maybe even as a tentative WPT?)
bool explicit_opener = false;nitty nit: I think the name might be a bit confusing out of context, so it might be good to have a more descriptive name for this. Maybe something like `retain_opener_if_possible` (or maybe even `disallow_proactive_bcg_swap`)?
Related details
- Charlie Reis
- Kevin McNee
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Charlie Reis (Gerrit)
Charlie Reis added 6 comments![]( "Open in Gerrit")
Thanks! Basically looks good to me as well, once we decide on a name for the new parameter that gets plumbed everywhere.
// Matches content::ShouldSwapBrowsingInstance.Is it a problem that these two types don't actually line up? We're introducing 23 here and 22 there. I'm guessing it's fine, but maybe this comment could help explain why?
bool explicit_opener) {I'm wondering if we can find a better name (throughout the CL) than `explicit_opener`, which is a bit ambiguous and sounds like it could be specifying what the opener should be, rather than signaling that interactions with openers are expected. Would something like `has_rel_opener` work?
// mechanism to opt-out of proactive swaps is to use an explicit "opener" rel.nit: Add something like: ", which signals that interactions with the opener are expected."
// preserved, such as with rel="opener".Similar to earlier comment: Since rel="opener" is the only planned way to do this, I'd be ok with s/such as with/via/ and calling this `has_rel_opener`.
bool explicit_opener = false;nitty nit: I think the name might be a bit confusing out of context, so it might be good to have a more descriptive name for this. Maybe something like `retain_opener_if_possible` (or maybe even `disallow_proactive_bcg_swap`)?
Ah, same thought. :) I'm suggesting `has_rel_opener` as another option.
Related details
- Kevin McNee
- Rakina Zata Amni
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kevin McNee (Gerrit)
Kevin McNee added 2 comments![]( "Open in Gerrit")
// Matches content::ShouldSwapBrowsingInstance.Is it a problem that these two types don't actually line up? We're introducing 23 here and 22 there. I'm guessing it's fine, but maybe this comment could help explain why?
It looks like it's due to the now unused SHOULD_SWAP_BROWSING_INSTANCE_NO (see https://chromium-review.googlesource.com/c/chromium/src/+/3001855 )
bool explicit_opener = false;Charlie Reisnitty nit: I think the name might be a bit confusing out of context, so it might be good to have a more descriptive name for this. Maybe something like `retain_opener_if_possible` (or maybe even `disallow_proactive_bcg_swap`)?
Ah, same thought. :) I'm suggesting `has_rel_opener` as another option.
- Charlie Reis
- Rakina Zata Amni
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kevin McNee (Gerrit)
Kevin McNee added 6 comments![]( "Open in Gerrit")
Kevin McNeeLet's add a brief comment on what these tests are about.
Done
Kevin McNeeCan you add a test for cross-site navigations as well?
Done
I'm wondering if we can find a better name (throughout the CL) than `explicit_opener`, which is a bit ambiguous and sounds like it could be specifying what the opener should be, rather than signaling that interactions with openers are expected. Would something like `has_rel_opener` work?
Done
// navigation would break scripting in this case. See crbug.com/40281878 forMaybe it's a good idea to add an explicit test for this case? (Maybe even as a tentative WPT?)
Done
Added a browser test along with the others for now. WPTs to follow this CL.
Similar to earlier comment: Since rel="opener" is the only planned way to do this, I'd be ok with s/such as with/via/ and calling this `has_rel_opener`.
Done
Charlie Reisnitty nit: I think the name might be a bit confusing out of context, so it might be good to have a more descriptive name for this. Maybe something like `retain_opener_if_possible` (or maybe even `disallow_proactive_bcg_swap`)?
Kevin McNeeAh, same thought. :) I'm suggesting `has_rel_opener` as another option.
Yeah, I'm leaning towards `has_rel_opener`.
Done
Though I kept the name just for the WindowFeatures where it should be clear, I think.
Related details
- Charlie Reis
- Rakina Zata Amni
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kevin McNee (Gerrit)
Kevin McNee added 1 comment![]( "Open in Gerrit")
// mechanism to opt-out of proactive swaps is to use an explicit "opener" rel.nit: Add something like: ", which signals that interactions with the opener are expected."
Done
Related details
- Charlie Reis
- Rakina Zata Amni
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rakina Zata Amni (Gerrit)
Rakina Zata Amni voted and added 2 comments![]( "Open in Gerrit")
Votes added by Rakina Zata Amni
| Code-Review | +1 |
LGTM, thanks!
popup_nav_observer.Wait();nit: Maybe check the URL as well just to be extra sure that the navigation that happened is the one triggered above?
Related details
- Charlie Reis
- Kevin McNee
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Charlie Reis (Gerrit)
Charlie Reis voted and added 2 comments![]( "Open in Gerrit")
Votes added by Charlie Reis
| Code-Review | +1 |
2 comments
Thanks! content/ LGTM given Rakina's review. I didn't review the Blink changes closely.
// Matches content::ShouldSwapBrowsingInstance.Kevin McNeeIs it a problem that these two types don't actually line up? We're introducing 23 here and 22 there. I'm guessing it's fine, but maybe this comment could help explain why?
It looks like it's due to the now unused SHOULD_SWAP_BROWSING_INSTANCE_NO (see https://chromium-review.googlesource.com/c/chromium/src/+/3001855 )
Thanks. Looks like the numbers don't really line up at all, though. kYes_CrossSiteProactiveSwap is 12 vs 2 here in the proto, and kNo_NotMainFrame is 2 vs 5 here.
Can we change this comment to say that there are corresponding values in content::ShouldSwapBrowsingInstance (rather than that it "matches"), and that those values get converted appropriately in ShouldSwapBrowsingInstanceToProto?
Related details
- Kevin McNee
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kevin McNee (Gerrit)
Kevin McNee voted and added 2 comments![]( "Open in Gerrit")
Votes added by Kevin McNee
| Commit-Queue | +1 |
2 comments
// Matches content::ShouldSwapBrowsingInstance.Kevin McNeeIs it a problem that these two types don't actually line up? We're introducing 23 here and 22 there. I'm guessing it's fine, but maybe this comment could help explain why?
Charlie ReisIt looks like it's due to the now unused SHOULD_SWAP_BROWSING_INSTANCE_NO (see https://chromium-review.googlesource.com/c/chromium/src/+/3001855 )
Thanks. Looks like the numbers don't really line up at all, though. kYes_CrossSiteProactiveSwap is 12 vs 2 here in the proto, and kNo_NotMainFrame is 2 vs 5 here.
Can we change this comment to say that there are corresponding values in content::ShouldSwapBrowsingInstance (rather than that it "matches"), and that those values get converted appropriately in ShouldSwapBrowsingInstanceToProto?
Done
nit: Maybe check the URL as well just to be extra sure that the navigation that happened is the one triggered above?
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kevin McNee (Gerrit)
Kevin McNee added 1 comment![]( "Open in Gerrit")
Rakina: I think the test failure could be a bug in RenderDocument.
Without this CL, I tried the following:
1. Visit a page
2. Run window.addEventListener('unload', () => {});
3. Same origin navigate away
4. Run var w = window.open(location.href, 'namedWindow');
w.previouslyOpened = true;
5. Go back in the original tab
6. Run window.open('', 'namedWindow').previouslyOpened;
With --enable-features=RenderDocument:level/all-frames , step 6 opens a new blank window. Without that flag, it reuses the window from step 4.
Everything's in the same site instance, but for the RenderDocument case, the `RelatedPages()` on the renderer side is empty.
Related details
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kevin McNee (Gerrit)
Kevin McNee added 1 comment![]( "Open in Gerrit")
Rakina: I think the test failure could be a bug in RenderDocument.
Without this CL, I tried the following:
1. Visit a page
2. Run window.addEventListener('unload', () => {});
3. Same origin navigate away
4. Run var w = window.open(location.href, 'namedWindow');
w.previouslyOpened = true;
5. Go back in the original tab
6. Run window.open('', 'namedWindow').previouslyOpened;With --enable-features=RenderDocument:level/all-frames , step 6 opens a new blank window. Without that flag, it reuses the window from step 4.
Everything's in the same site instance, but for the RenderDocument case, the `RelatedPages()` on the renderer side is empty.
Okay, so the renderer knows about the other page (it's in `OrdinaryPages()`), but it's not linked to the related pages. If I replace RelatedPages with a naive loop over OrdinaryPages, the rest of the test works. It looks like in `LinkRelatedPagesIfNeeded` for the back navigation, there's no opener, causing it to skip this.
Related details
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rakina Zata Amni (Gerrit)
Rakina Zata Amni voted and added 1 comment![]( "Open in Gerrit")
Votes added by Rakina Zata Amni
| Code-Review | +1 |
1 comment
Kevin McNeeRakina: I think the test failure could be a bug in RenderDocument.
Without this CL, I tried the following:
1. Visit a page
2. Run window.addEventListener('unload', () => {});
3. Same origin navigate away
4. Run var w = window.open(location.href, 'namedWindow');
w.previouslyOpened = true;
5. Go back in the original tab
6. Run window.open('', 'namedWindow').previouslyOpened;With --enable-features=RenderDocument:level/all-frames , step 6 opens a new blank window. Without that flag, it reuses the window from step 4.
Everything's in the same site instance, but for the RenderDocument case, the `RelatedPages()` on the renderer side is empty.
Okay, so the renderer knows about the other page (it's in `OrdinaryPages()`), but it's not linked to the related pages. If I replace RelatedPages with a naive loop over OrdinaryPages, the rest of the test works. It looks like in `LinkRelatedPagesIfNeeded` for the back navigation, there's no opener, causing it to skip this.
Sorry, I completely missed this in my inbox. I think I did make some changes for RenderDocument around that part of the code to exclude a "fake" RemoteFrame for the speculative main RFH ([doc](https://docs.google.com/document/d/1ajCoI-3CKGBcbzp26qDR_QqjXBic1XQ5PVnkBfsUnOU/edit#heading=h.re8zdlzaiwkv)), but I will look into how that can relate to this next week.
Related details
- Kevin McNee
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rakina Zata Amni (Gerrit)
Rakina Zata Amni added 1 comment![]( "Open in Gerrit")
Kevin McNeeRakina: I think the test failure could be a bug in RenderDocument.
Without this CL, I tried the following:
1. Visit a page
2. Run window.addEventListener('unload', () => {});
3. Same origin navigate away
4. Run var w = window.open(location.href, 'namedWindow');
w.previouslyOpened = true;
5. Go back in the original tab
6. Run window.open('', 'namedWindow').previouslyOpened;With --enable-features=RenderDocument:level/all-frames , step 6 opens a new blank window. Without that flag, it reuses the window from step 4.
Everything's in the same site instance, but for the RenderDocument case, the `RelatedPages()` on the renderer side is empty.
Rakina Zata AmniOkay, so the renderer knows about the other page (it's in `OrdinaryPages()`), but it's not linked to the related pages. If I replace RelatedPages with a naive loop over OrdinaryPages, the rest of the test works. It looks like in `LinkRelatedPagesIfNeeded` for the back navigation, there's no opener, causing it to skip this.
Sorry, I completely missed this in my inbox. I think I did make some changes for RenderDocument around that part of the code to exclude a "fake" RemoteFrame for the speculative main RFH ([doc](https://docs.google.com/document/d/1ajCoI-3CKGBcbzp26qDR_QqjXBic1XQ5PVnkBfsUnOU/edit#heading=h.re8zdlzaiwkv)), but I will look into how that can relate to this next week.
OK, it looks like this can be solved by just swapping the new page in place of the old page in the related pages list. I'll send out the fix in crrev.com/c/5644621 soon. Thanks for finding this!!
Kevin McNee (Gerrit)
Kevin McNee added 1 comment![]( "Open in Gerrit")
Kevin McNeeRakina: I think the test failure could be a bug in RenderDocument.
Without this CL, I tried the following:
1. Visit a page
2. Run window.addEventListener('unload', () => {});
3. Same origin navigate away
4. Run var w = window.open(location.href, 'namedWindow');
w.previouslyOpened = true;
5. Go back in the original tab
6. Run window.open('', 'namedWindow').previouslyOpened;With --enable-features=RenderDocument:level/all-frames , step 6 opens a new blank window. Without that flag, it reuses the window from step 4.
Everything's in the same site instance, but for the RenderDocument case, the `RelatedPages()` on the renderer side is empty.
Rakina Zata AmniOkay, so the renderer knows about the other page (it's in `OrdinaryPages()`), but it's not linked to the related pages. If I replace RelatedPages with a naive loop over OrdinaryPages, the rest of the test works. It looks like in `LinkRelatedPagesIfNeeded` for the back navigation, there's no opener, causing it to skip this.
Rakina Zata AmniSorry, I completely missed this in my inbox. I think I did make some changes for RenderDocument around that part of the code to exclude a "fake" RemoteFrame for the speculative main RFH ([doc](https://docs.google.com/document/d/1ajCoI-3CKGBcbzp26qDR_QqjXBic1XQ5PVnkBfsUnOU/edit#heading=h.re8zdlzaiwkv)), but I will look into how that can relate to this next week.
OK, it looks like this can be solved by just swapping the new page in place of the old page in the related pages list. I'll send out the fix in crrev.com/c/5644621 soon. Thanks for finding this!!
I've filed https://crbug.com/349132649 for this issue. In the meantime, I've added a GTEST_SKIP for the part of the test that hits this issue with RenderDocument enabled, in order to proceed with this CL. And just in case, I've added another test case covering an additional navigation before trying to reuse the window.
Related details
- Rakina Zata Amni
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kevin McNee (Gerrit)
Kevin McNee added 1 comment![]( "Open in Gerrit")
Adding japhet@ for blink
Adding nuskos@ for tracing (adding enum value)
Adding IPC reviewers for mojom changes
PTAL.
Related details
- Chromium IPC Reviews
- Nate Chapin
- Rakina Zata Amni
- Stephen Nusko
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: tis...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): tis...@chromium.org
Reviewer source(s):
tis...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Brendon Tiszka
- Nate Chapin
- Rakina Zata Amni
- Stephen Nusko
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rakina Zata Amni (Gerrit)
Rakina Zata Amni voted and added 1 comment![]( "Open in Gerrit")
Votes added by Rakina Zata Amni
| Code-Review | +1 |
Kevin McNeeRakina: I think the test failure could be a bug in RenderDocument.
Without this CL, I tried the following:
1. Visit a page
2. Run window.addEventListener('unload', () => {});
3. Same origin navigate away
4. Run var w = window.open(location.href, 'namedWindow');
w.previouslyOpened = true;
5. Go back in the original tab
6. Run window.open('', 'namedWindow').previouslyOpened;With --enable-features=RenderDocument:level/all-frames , step 6 opens a new blank window. Without that flag, it reuses the window from step 4.
Everything's in the same site instance, but for the RenderDocument case, the `RelatedPages()` on the renderer side is empty.
Rakina Zata AmniOkay, so the renderer knows about the other page (it's in `OrdinaryPages()`), but it's not linked to the related pages. If I replace RelatedPages with a naive loop over OrdinaryPages, the rest of the test works. It looks like in `LinkRelatedPagesIfNeeded` for the back navigation, there's no opener, causing it to skip this.
Rakina Zata AmniSorry, I completely missed this in my inbox. I think I did make some changes for RenderDocument around that part of the code to exclude a "fake" RemoteFrame for the speculative main RFH ([doc](https://docs.google.com/document/d/1ajCoI-3CKGBcbzp26qDR_QqjXBic1XQ5PVnkBfsUnOU/edit#heading=h.re8zdlzaiwkv)), but I will look into how that can relate to this next week.
Kevin McNeeOK, it looks like this can be solved by just swapping the new page in place of the old page in the related pages list. I'll send out the fix in crrev.com/c/5644621 soon. Thanks for finding this!!
I've filed https://crbug.com/349132649 for this issue. In the meantime, I've added a GTEST_SKIP for the part of the test that hits this issue with RenderDocument enabled, in order to proceed with this CL. And just in case, I've added another test case covering an additional navigation before trying to reuse the window.
- Brendon Tiszka
- Kevin McNee
- Nate Chapin
- Stephen Nusko
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stephen Nusko (Gerrit)
Stephen Nusko voted and added 1 comment![]( "Open in Gerrit")
Related details
- Brendon Tiszka
- Kevin McNee
- Nate Chapin
Brendon Tiszka (Gerrit)
Brendon Tiszka voted and added 1 comment![]( "Open in Gerrit")
Votes added by Brendon Tiszka
| Code-Review | +1 |
1 comment
- Kevin McNee
- Nate Chapin
Kevin McNee (Gerrit)
Kevin McNee added 1 comment![]( "Open in Gerrit")
- Jeremy Roman
- Nate Chapin
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jeremy Roman (Gerrit)
Jeremy Roman voted and added 1 comment![]( "Open in Gerrit")
Votes added by Jeremy Roman
| Code-Review | +1 |
1 comment
- Kevin McNee
- Nate Chapin
Kevin McNee (Gerrit)
Kevin McNee voted Commit-Queue+2![]( "Open in Gerrit")
- Nate Chapin
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Implement rel=opener as a proactive BrowsingInstance swap opt-out
To address breakage of named window reuse across a back navigation
when a proactive BrowsingInstance swap has happened, we offer the
option to use an explicit opener relation for same-window navigations
as an opt-out from proactive swaps. This applies to elements that
support rel=opener (a, area, and form). We also introduce "opener"
as a window feature (for window.open()).
Usage looks like the following:
Before:
<a href="next.html">next</a>
After:
<a href="next.html" rel="opener">next</a>
Before:
location.href = getNextPageUrl();
After:
window.open(getNextPageUrl(), '_self', 'opener');
Note that the opt-out only affects proactive swaps. It cannot be used
to bypass swaps that are required (e.g. for COOP).
Usage of conflicting rel types (rel="noopener opener") resolves in
favour of noopener.
WPTs still need to be added in a future CL.
Explainer: https://github.com/explainers-by-googlers/future-browsing-context-group-dependency-hint
Intent to Prototype: https://groups.google.com/u/1/a/chromium.org/g/blink-dev/c/sI1zySADmNs
- M base/tracing/protos/chrome_track_event.proto
- M content/browser/back_forward_cache_not_restored_reasons_browsertest.cc
- M content/browser/fenced_frame/fenced_frame.cc
- M content/browser/loader/navigation_url_loader_impl_unittest.cc
- M content/browser/loader/navigation_url_loader_unittest.cc
- M content/browser/renderer_host/back_forward_cache_can_store_document_result.cc
- M content/browser/renderer_host/back_forward_cache_metrics.cc
- M content/browser/renderer_host/navigation_controller_impl.cc
- M content/browser/renderer_host/navigation_controller_impl.h
- M content/browser/renderer_host/navigation_request.cc
- M content/browser/renderer_host/navigation_request.h
- M content/browser/renderer_host/navigator.cc
- M content/browser/renderer_host/navigator.h
- M content/browser/renderer_host/proactively_swap_browsing_instances_browsertest.cc
- M content/browser/renderer_host/render_frame_host_impl.cc
- M content/browser/renderer_host/render_frame_host_manager.cc
- M content/browser/renderer_host/render_frame_host_manager.h
- M content/browser/renderer_host/render_frame_proxy_host.cc
- M content/browser/renderer_host/should_swap_browsing_instance.h
- M content/browser/security_exploit_browsertest.cc
- M content/browser/service_worker/service_worker_client_utils.cc
- M content/public/browser/navigation_controller.cc
- M content/public/browser/navigation_controller.h
- M content/public/browser/page_navigator.h
- M content/renderer/render_frame_impl.cc
- M content/test/navigation_simulator_impl.cc
- M content/test/test_render_frame_host.cc
- M third_party/blink/public/mojom/frame/remote_frame.mojom
- M third_party/blink/public/mojom/navigation/navigation_params.mojom
- M third_party/blink/public/web/web_navigation_params.h
- M third_party/blink/public/web/web_window_features.h
- M third_party/blink/renderer/core/frame/local_frame_client.h
- M third_party/blink/renderer/core/frame/local_frame_client_impl.cc
- M third_party/blink/renderer/core/frame/local_frame_client_impl.h
- M third_party/blink/renderer/core/frame/remote_frame.cc
- M third_party/blink/renderer/core/html/html_anchor_element.cc
- M third_party/blink/renderer/core/inspector/inspector_page_agent.cc
- M third_party/blink/renderer/core/loader/empty_clients.cc
- M third_party/blink/renderer/core/loader/empty_clients.h
- M third_party/blink/renderer/core/loader/form_submission.cc
- M third_party/blink/renderer/core/loader/form_submission.h
- M third_party/blink/renderer/core/loader/frame_load_request.h
- M third_party/blink/renderer/core/loader/frame_loader.cc
- M third_party/blink/renderer/core/page/create_window.cc
- M third_party/blink/renderer/core/page/window_features_test.cc
- M third_party/blink/renderer/platform/runtime_enabled_features.json5
Code-Review: +1 by Rakina Zata Amni, +1 by Jeremy Roman, +1 by Stephen Nusko, +1 by Brendon Tiszka, +1 by Charlie Reis
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |