Reland "Fenced frames: Remove mode attribute [2/N]" [chromium/src : main]

Garrett Tanzer (Gerrit)

unread,

Mar 20, 2023, 2:32:07 PM3/20/23

to alexmo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, iclella...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, subresource-f...@chromium.org, David Bokan, Daniel Cheng, Nate Chapin, Luna Lu, Mike Taylor, Victor Tan, Findit, Jeremy Roman, Alex Turner, Dominic Farolino, Chromium LUCI CQ, Alex Moshchuk, chromium...@chromium.org

Attention is currently required from: Alex Moshchuk, Alex Turner, Dominic Farolino, Garrett Tanzer, Jeremy Roman.

Patch set 3:Commit-Queue +1

View Change

1 comment:

Patchset:
- Patch Set #3:
  Reland. Had to change test expectations for one content browser test slightly, to handle different behavior when bfcache is disabled.

To view, visit change 4353654. To unsubscribe, or for help writing mail filters, visit settings.

Alex Turner (Gerrit)

unread,

Mar 20, 2023, 2:34:33 PM3/20/23

to Garrett Tanzer, alexmo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, iclella...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, subresource-f...@chromium.org, David Bokan, Daniel Cheng, Nate Chapin, Luna Lu, Mike Taylor, Victor Tan, Findit, Jeremy Roman, Dominic Farolino, Chromium LUCI CQ, Alex Moshchuk, chromium...@chromium.org

Attention is currently required from: Alex Moshchuk, Dominic Farolino, Garrett Tanzer, Jeremy Roman.

Patch set 3:Code-Review +1

View Change

1 comment:

Patchset:
- Patch Set #3:
  ad_tagging_browsertest.cc SLGTM

To view, visit change 4353654. To unsubscribe, or for help writing mail filters, visit settings.

Alex Moshchuk (Gerrit)

unread,

Mar 20, 2023, 2:41:10 PM3/20/23

to Garrett Tanzer, alexmo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, iclella...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, subresource-f...@chromium.org, Alex Turner, David Bokan, Daniel Cheng, Nate Chapin, Luna Lu, Mike Taylor, Victor Tan, Findit, Jeremy Roman, Dominic Farolino, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Dominic Farolino, Garrett Tanzer, Jeremy Roman.

Patch set 3:Code-Review +1

View Change

1 comment:

File content/browser/fenced_frame/fenced_frame_browsertest.cc:
- Patch Set #3, Line 3842:
```
shell()
          ->web_contents()
          ->GetController()
          .GetBackForwardCache()
          .IsBackForwardCacheFeatureEnabled()
```
  nit: just BackForwardCache::IsBackForwardCacheFeatureEnabled(), like line 3768 above for example?

To view, visit change 4353654. To unsubscribe, or for help writing mail filters, visit settings.

Dominic Farolino (Gerrit)

unread,

Mar 20, 2023, 3:12:09 PM3/20/23

to Garrett Tanzer, alexmo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, iclella...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, subresource-f...@chromium.org, Alex Moshchuk, Alex Turner, David Bokan, Daniel Cheng, Nate Chapin, Luna Lu, Mike Taylor, Victor Tan, Findit, Jeremy Roman, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Garrett Tanzer, Jeremy Roman.

Patch set 3:Code-Review +1

View Change

To view, visit change 4353654. To unsubscribe, or for help writing mail filters, visit settings.

Garrett Tanzer (Gerrit)

unread,

Mar 20, 2023, 3:31:39 PM3/20/23

to alexmo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, iclella...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, subresource-f...@chromium.org, Dominic Farolino, Alex Moshchuk, Alex Turner, David Bokan, Daniel Cheng, Nate Chapin, Luna Lu, Mike Taylor, Victor Tan, Findit, Jeremy Roman, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Jeremy Roman.

Patch set 4:Commit-Queue +1

View Change

1 comment:

File content/browser/fenced_frame/fenced_frame_browsertest.cc:

Patch Set #3, Line 3842:

shell()
          ->web_contents()
          ->GetController()
          .GetBackForwardCache()
          .IsBackForwardCacheFeatureEnabled()

nit: just BackForwardCache::IsBackForwardCacheFeatureEnabled(), like line 3768 above for example?

- Thanks, didn't find that when I searched for some reason.

To view, visit change 4353654. To unsubscribe, or for help writing mail filters, visit settings.

Jeremy Roman (Gerrit)

unread,

Mar 20, 2023, 5:54:24 PM3/20/23

to Garrett Tanzer, alexmo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, iclella...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, subresource-f...@chromium.org, Jeremy Roman, Dominic Farolino, Alex Moshchuk, Alex Turner, David Bokan, Daniel Cheng, Nate Chapin, Luna Lu, Mike Taylor, Victor Tan, Findit, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Garrett Tanzer.

Patch set 4:Code-Review +1

View Change

To view, visit change 4353654. To unsubscribe, or for help writing mail filters, visit settings.

Garrett Tanzer (Gerrit)

unread,

Mar 20, 2023, 6:39:14 PM3/20/23

to alexmo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, iclella...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, subresource-f...@chromium.org, Jeremy Roman, Dominic Farolino, Alex Moshchuk, Alex Turner, David Bokan, Daniel Cheng, Nate Chapin, Luna Lu, Mike Taylor, Victor Tan, Findit, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Garrett Tanzer.

Patch set 4:Commit-Queue +2

View Change

To view, visit change 4353654. To unsubscribe, or for help writing mail filters, visit settings.

Chromium LUCI CQ (Gerrit)

unread,

Mar 20, 2023, 6:42:19 PM3/20/23

to Garrett Tanzer, alexmo...@chromium.org, blink-re...@chromium.org, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, iclella...@chromium.org, ipc-securi...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, subresource-f...@chromium.org, Jeremy Roman, Dominic Farolino, Alex Moshchuk, Alex Turner, David Bokan, Daniel Cheng, Nate Chapin, Luna Lu, Mike Taylor, Victor Tan, Findit, chromium...@chromium.org

Chromium LUCI CQ submitted this change.

View Change

Approvals:
  Jeremy Roman: Looks good to me
  Alex Moshchuk: Looks good to me
  Dominic Farolino: Looks good to me
  Alex Turner: Looks good to me
  Garrett Tanzer: Commit

Reland "Fenced frames: Remove mode attribute [2/N]"

This is a reland of commit b7b098100da8488daf23dd4d9c1d3072408cfa24
which fixes test expectations for the content browser test
"NavigateUnfencedTopAndGoBack" when bfcache is disabled.

Original change's description:
> Fenced frames: Remove mode attribute [2/N]
>
> This CL deletes the FencedFrameMode enum and disables the `mode` attribute of HTMLFencedFrameElement. (It will be fully removed in a follow-up CL.) Instead, the mode information is stored in
> `FencedFrameConfig` and `FencedFrameProperties`. This also means that
> the mode can change across different embedder-initiated navigations.
>
> This involves one significant change to fenced frame information flow:
> Instead of the FencedFrameProperties being stored in the root
> FrameTreeNode in NavigationRequest::DidCommitNavigation, it is stored
> slightly earlier in RenderFrameHostImpl::DidCommitNewDocument. This is
> so that the mode has already been set when ResetPermissionsPolicy() is
> called. This is purely a refactor.
>
> Removing the fenced frame mode attribute has one effect on WP-visible
> behavior: now there is no such thing as opaque ads mode using a
> non-opaque url. Therefore this CL also changes all tests that previously
> used opaque ads mode with a non-opaque url, to use a urn/config instead.
>
> Bug: 1347953
> Change-Id: I88ab14184dcc5eb10a115ed070aad6a3e83cfe8c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4184456
> Reviewed-by: Jeremy Roman <jbr...@chromium.org>
> Commit-Queue: Garrett Tanzer <gta...@chromium.org>
> Reviewed-by: Alex Turner <ale...@chromium.org>
> Reviewed-by: Alex Moshchuk <ale...@chromium.org>
> Reviewed-by: Dominic Farolino <d...@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1118694}

Bug: 1347953
Change-Id: I149833dfbae6d1d6ee6a65998e3678b48b24d768
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4353654
Reviewed-by: Jeremy Roman <jbr...@chromium.org>
Reviewed-by: Alex Moshchuk <ale...@chromium.org>
Reviewed-by: Alex Turner <ale...@chromium.org>
Commit-Queue: Garrett Tanzer <gta...@chromium.org>
Reviewed-by: Dominic Farolino <d...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1119643}
---
M chrome/browser/subresource_filter/ad_tagging_browsertest.cc
M content/browser/attribution_reporting/attribution_host_unittest.cc
M content/browser/attribution_reporting/attribution_src_browsertest.cc
M content/browser/client_hints/client_hints.cc
M content/browser/fenced_frame/fenced_frame.cc
M content/browser/fenced_frame/fenced_frame.h
M content/browser/fenced_frame/fenced_frame_browsertest.cc
M content/browser/fenced_frame/fenced_frame_config.cc
M content/browser/fenced_frame/fenced_frame_url_mapping.cc
M content/browser/interest_group/interest_group_browsertest.cc
M content/browser/renderer_host/frame_tree_node.cc
M content/browser/renderer_host/frame_tree_node.h
M content/browser/renderer_host/navigation_request.cc
M content/browser/renderer_host/navigation_request.h
M content/browser/renderer_host/render_frame_host_impl.cc
M content/browser/renderer_host/render_frame_host_impl.h
M content/browser/renderer_host/render_view_host_impl.cc
M content/browser/security_exploit_browsertest.cc
M content/browser/shared_storage/shared_storage_browsertest.cc
M content/common/frame.mojom
M content/public/test/fenced_frame_test_util.cc
M content/public/test/fenced_frame_test_util.h
M content/public/test/test_renderer_host.h
M content/renderer/render_frame_impl.cc
M content/test/content_unittests_bundle_data.filelist
D content/test/data/fenced_frames/opaque_ads.html
D content/test/data/fenced_frames/opaque_ads.html.mock-http-headers
M content/test/test_render_frame_host.cc
M content/test/test_render_frame_host.h
M third_party/blink/common/permissions_policy/permissions_policy.cc
M third_party/blink/common/permissions_policy/permissions_policy_unittest.cc
M third_party/blink/public/common/permissions_policy/permissions_policy.h
M third_party/blink/public/mojom/frame/frame.mojom
M third_party/blink/public/mojom/frame/frame_policy.mojom
M third_party/blink/public/web/web_view.h
M third_party/blink/renderer/core/execution_context/security_context_init.cc
M third_party/blink/renderer/core/exported/web_view_impl.cc
M third_party/blink/renderer/core/exported/web_view_impl.h
M third_party/blink/renderer/core/frame/frame.cc
M third_party/blink/renderer/core/frame/frame.h
M third_party/blink/renderer/core/frame/frame_test_helpers.cc
M third_party/blink/renderer/core/frame/frame_test_helpers.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/local_frame_view_test.cc
M third_party/blink/renderer/core/frame/web_local_frame_impl.cc
M third_party/blink/renderer/core/frame/web_local_frame_impl.h
M third_party/blink/renderer/core/html/fenced_frame/fenced_frame_mparch_delegate.cc
M third_party/blink/renderer/core/html/fenced_frame/html_fenced_frame_element.cc
M third_party/blink/renderer/core/html/fenced_frame/html_fenced_frame_element.h
M third_party/blink/renderer/core/html/fenced_frame/html_fenced_frame_element_test.cc
M third_party/blink/renderer/core/loader/document_loader.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/page/frame_tree.cc
M third_party/blink/renderer/core/page/page.h
M third_party/blink/renderer/core/paint/timing/image_paint_timing_detector_test.cc
M third_party/blink/renderer/core/scheduler_integration_tests/frame_scheduler_test.cc
M third_party/blink/renderer/core/testing/fake_local_frame_host.cc
M third_party/blink/renderer/core/testing/fake_local_frame_host.h
M third_party/blink/renderer/core/testing/sim/sim_test.cc
M third_party/blink/renderer/core/testing/sim/sim_test.h
M third_party/blink/renderer/modules/ad_auction/navigator_auction.cc
M third_party/blink/web_tests/wpt_internal/fenced_frame/can-load-api.https.html
M third_party/blink/web_tests/wpt_internal/fenced_frame/default-enabled-features-allow-none.https.html
M third_party/blink/web_tests/wpt_internal/fenced_frame/default-enabled-features-allow-self.https.html
M third_party/blink/web_tests/wpt_internal/fenced_frame/default-enabled-features-attribute-change.https.html
M third_party/blink/web_tests/wpt_internal/fenced_frame/nested-opaque-ad-sizes.https.html
M third_party/blink/web_tests/wpt_internal/fenced_frame/resources/default-enabled-features-helper.js
M third_party/blink/web_tests/wpt_internal/fenced_frame/resources/utils.js
M third_party/blink/web_tests/wpt_internal/fenced_frame/set-automatic-beacon.https.html
M third_party/blink/web_tests/wpt_internal/fenced_frame/unfenced-top.https.html
73 files changed, 532 insertions(+), 447 deletions(-)

To view, visit change 4353654. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward