Skip to content

Commit

Permalink
Stash the fenced frame implementation type on page.
Browse files Browse the repository at this point in the history
The lookup of the fenced frame implementation type is expensive
because the value causes string compares. Stash the computation of the
value on the Page object.

BUG=1329402,1329393,1329318,1329273

(cherry picked from commit ad2bde2)

Change-Id: Ibb10fc3f31ca3f91ea31c44aff093f69c98254b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3669175
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Adithya Srinivasan <adithyas@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1008285}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3680558
Cr-Commit-Position: refs/branch-heads/5060@{#418}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
dtapuska authored and Chromium LUCI CQ committed May 31, 2022
1 parent 9a587f8 commit 078f30e
Show file tree
Hide file tree
Showing 14 changed files with 160 additions and 102 deletions.
37 changes: 25 additions & 12 deletions third_party/blink/renderer/core/frame/frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,13 @@ void Frame::RenderFallbackContentWithResourceTiming(
}

bool Frame::IsInFencedFrameTree() const {
if (!blink::features::IsFencedFramesEnabled())
if (IsDetached())
return false;
auto ff_impl = GetPage()->FencedFramesImplementationType();
if (!ff_impl)
return false;

switch (blink::features::kFencedFramesImplementationTypeParam.Get()) {
switch (ff_impl.value()) {
case blink::features::FencedFramesImplementationType::kMPArch:
return GetPage() && GetPage()->IsMainFrameFencedFrameRoot();
case blink::features::FencedFramesImplementationType::kShadowDOM:
Expand All @@ -375,13 +378,14 @@ absl::optional<mojom::blink::FencedFrameMode> Frame::GetFencedFrameMode()
const {
DCHECK(!IsDetached());

if (!blink::features::IsFencedFramesEnabled())
auto ff_impl = GetPage()->FencedFramesImplementationType();
if (!ff_impl)
return absl::nullopt;

if (!IsInFencedFrameTree())
return absl::nullopt;

switch (blink::features::kFencedFramesImplementationTypeParam.Get()) {
switch (ff_impl.value()) {
case blink::features::FencedFramesImplementationType::kMPArch:
return GetPage()->FencedFrameMode();
case blink::features::FencedFramesImplementationType::kShadowDOM:
Expand All @@ -395,10 +399,11 @@ absl::optional<mojom::blink::FencedFrameMode> Frame::GetFencedFrameMode()
}

bool Frame::IsInShadowDOMOpaqueAdsFencedFrameTree() const {
if (!blink::features::IsFencedFramesEnabled())
auto ff_impl = GetPage()->FencedFramesImplementationType();
if (!ff_impl)
return false;

switch (blink::features::kFencedFramesImplementationTypeParam.Get()) {
switch (ff_impl.value()) {
case blink::features::FencedFramesImplementationType::kMPArch:
return false;
case blink::features::FencedFramesImplementationType::kShadowDOM: {
Expand All @@ -412,10 +417,11 @@ bool Frame::IsInShadowDOMOpaqueAdsFencedFrameTree() const {
}

bool Frame::IsInMPArchOpaqueAdsFencedFrameTree() const {
if (!blink::features::IsFencedFramesEnabled())
auto ff_impl = GetPage()->FencedFramesImplementationType();
if (!ff_impl)
return false;

switch (blink::features::kFencedFramesImplementationTypeParam.Get()) {
switch (ff_impl.value()) {
case blink::features::FencedFramesImplementationType::kMPArch:
return GetPage() && GetPage()->FencedFrameMode() ==
mojom::blink::FencedFrameMode::kOpaqueAds;
Expand Down Expand Up @@ -636,13 +642,19 @@ void Frame::SetOpenerDoNotNotify(Frame* opener) {
}

Frame* Frame::Parent(FrameTreeBoundary frame_tree_boundary) const {
// |parent_| will be null if detached, return early before accessing
// Page.
if (!parent_)
return nullptr;

// TODO(crbug.com/1123606): Remove this once we use MPArch as the underlying
// fenced frames implementation, instead of the
// `FencedFrameShadowDOMDelegate`.
if (frame_tree_boundary == FrameTreeBoundary::kFenced && Owner() &&
Owner()->GetFramePolicy().is_fenced &&
features::IsFencedFramesEnabled() &&
features::IsFencedFramesShadowDOMBased()) {
GetPage()->FencedFramesImplementationType().has_value() &&
GetPage()->FencedFramesImplementationType().value() ==
features::FencedFramesImplementationType::kShadowDOM) {
return nullptr;
}

Expand Down Expand Up @@ -710,10 +722,11 @@ bool Frame::FocusCrossesFencedBoundary() {
}

bool Frame::ShouldAllowScriptFocus() {
if (!features::IsFencedFramesEnabled())
auto ff_impl = GetPage()->FencedFramesImplementationType();
if (!ff_impl)
return true;

switch (blink::features::kFencedFramesImplementationTypeParam.Get()) {
switch (ff_impl.value()) {
case blink::features::FencedFramesImplementationType::kMPArch:
// For a newly-loaded page, no page will have focus. We allow a non-fenced
// frame to get the first focus before enforcing if a page already has
Expand Down
37 changes: 15 additions & 22 deletions third_party/blink/renderer/core/frame/local_frame_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -361,12 +361,10 @@ void LocalFrameView::ForAllChildViewsAndPlugins(const Function& function) {
if (Frame* frame = portal->GetFrame())
function(*frame->View());
}
if (features::IsFencedFramesMPArchBased()) {
for (HTMLFencedFrameElement* fenced_frame :
DocumentFencedFrames::From(*document).GetFencedFrames()) {
if (Frame* frame = fenced_frame->ContentFrame())
function(*frame->View());
}
for (HTMLFencedFrameElement* fenced_frame :
DocumentFencedFrames::From(*document).GetFencedFrames()) {
if (Frame* frame = fenced_frame->ContentFrame())
function(*frame->View());
}
}
}
Expand Down Expand Up @@ -442,14 +440,11 @@ void LocalFrameView::ForAllRemoteFrameViews(const Function& function) {
function(*view);
}
}
if (features::IsFencedFramesMPArchBased()) {
for (HTMLFencedFrameElement* fenced_frame :
DocumentFencedFrames::From(*document).GetFencedFrames()) {
if (RemoteFrame* frame =
To<RemoteFrame>(fenced_frame->ContentFrame())) {
if (RemoteFrameView* view = frame->View())
function(*view);
}
for (HTMLFencedFrameElement* fenced_frame :
DocumentFencedFrames::From(*document).GetFencedFrames()) {
if (RemoteFrame* frame = To<RemoteFrame>(fenced_frame->ContentFrame())) {
if (RemoteFrameView* view = frame->View())
function(*view);
}
}
}
Expand Down Expand Up @@ -4317,14 +4312,12 @@ bool LocalFrameView::UpdateViewportIntersectionsForSubtree(
}
}

if (features::IsFencedFramesMPArchBased()) {
for (HTMLFencedFrameElement* fenced_frame :
DocumentFencedFrames::From(*frame_->GetDocument()).GetFencedFrames()) {
if (Frame* frame = fenced_frame->ContentFrame()) {
needs_occlusion_tracking |=
frame->View()->UpdateViewportIntersectionsForSubtree(
flags, monotonic_time);
}
for (HTMLFencedFrameElement* fenced_frame :
DocumentFencedFrames::From(*frame_->GetDocument()).GetFencedFrames()) {
if (Frame* frame = fenced_frame->ContentFrame()) {
needs_occlusion_tracking |=
frame->View()->UpdateViewportIntersectionsForSubtree(flags,
monotonic_time);
}
}
return needs_occlusion_tracking;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,9 @@ void ForEachRemoteFrameChildrenControlledByWidget(
callback.Run(remote_frame);
}
// Iterate on any fenced frames owned by a local frame.
if (features::IsFencedFramesMPArchBased()) {
for (HTMLFencedFrameElement* fenced_frame :
DocumentFencedFrames::From(*document).GetFencedFrames()) {
callback.Run(To<RemoteFrame>(fenced_frame->ContentFrame()));
}
for (HTMLFencedFrameElement* fenced_frame :
DocumentFencedFrames::From(*document).GetFencedFrames()) {
callback.Run(To<RemoteFrame>(fenced_frame->ContentFrame()));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,31 +32,29 @@ DocumentFencedFrames::DocumentFencedFrames(Document& document)

void DocumentFencedFrames::RegisterFencedFrame(
HTMLFencedFrameElement* fenced_frame) {
DCHECK(features::IsFencedFramesMPArchBased());
fenced_frames_.push_back(fenced_frame);

if (features::IsFencedFramesMPArchBased()) {
LocalFrame* frame = GetSupplementable()->GetFrame();
if (!frame)
return;
if (Page* page = frame->GetPage())
page->IncrementSubframeCount();
}
LocalFrame* frame = GetSupplementable()->GetFrame();
if (!frame)
return;
if (Page* page = frame->GetPage())
page->IncrementSubframeCount();
}

void DocumentFencedFrames::DeregisterFencedFrame(
HTMLFencedFrameElement* fenced_frame) {
DCHECK(features::IsFencedFramesMPArchBased());
wtf_size_t index = fenced_frames_.Find(fenced_frame);
if (index != WTF::kNotFound) {
fenced_frames_.EraseAt(index);
}

if (features::IsFencedFramesMPArchBased()) {
LocalFrame* frame = GetSupplementable()->GetFrame();
if (!frame)
return;
if (Page* page = frame->GetPage()) {
page->DecrementSubframeCount();
}
LocalFrame* frame = GetSupplementable()->GetFrame();
if (!frame)
return;
if (Page* page = frame->GetPage()) {
page->DecrementSubframeCount();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,20 @@
#include "third_party/blink/renderer/core/frame/local_frame_client.h"
#include "third_party/blink/renderer/core/frame/remote_frame.h"
#include "third_party/blink/renderer/core/html/fenced_frame/document_fenced_frames.h"
#include "third_party/blink/renderer/core/layout/layout_object.h"
#include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h"

namespace blink {

FencedFrameMPArchDelegate::FencedFrameMPArchDelegate(
HTMLFencedFrameElement* outer_element)
: HTMLFencedFrameElement::FencedFrameDelegate(outer_element) {
DCHECK_EQ(features::kFencedFramesImplementationTypeParam.Get(),
DCHECK_EQ(outer_element->GetDocument()
.GetFrame()
->GetPage()
->FencedFramesImplementationType()
.value(),
features::FencedFramesImplementationType::kMPArch);

DocumentFencedFrames::From(GetElement().GetDocument())
Expand Down Expand Up @@ -47,4 +53,27 @@ void FencedFrameMPArchDelegate::Dispose() {
.DeregisterFencedFrame(&GetElement());
}

void FencedFrameMPArchDelegate::AttachLayoutTree() {
if (GetElement().GetLayoutEmbeddedContent() && GetElement().ContentFrame()) {
GetElement().SetEmbeddedContentView(GetElement().ContentFrame()->View());
}
}

bool FencedFrameMPArchDelegate::SupportsFocus() {
return true;
}

void FencedFrameMPArchDelegate::FreezeFrameSize() {
// With MPArch, mark the layout as stale. Do this unconditionally because
// we are rounding the size.
GetElement().GetLayoutObject()->SetNeedsLayoutAndFullPaintInvalidation(
"Froze MPArch fenced frame");

// Stop the `ResizeObserver`. It is needed only to compute the
// frozen size in MPArch. ShadowDOM stays subscribed in order to
// update the CSS on the inner iframe element as the outer container's
// size changes.
GetElement().StopResizeObserver();
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ class CORE_EXPORT FencedFrameMPArchDelegate

void Navigate(const KURL&) override;
void Dispose() override;
void AttachLayoutTree() override;
bool SupportsFocus() override;
void FreezeFrameSize() override;

private:
mojo::AssociatedRemote<mojom::blink::FencedFrameOwnerHost> remote_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,23 @@

#include "third_party/blink/public/common/features.h"
#include "third_party/blink/renderer/core/dom/shadow_root.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/core/html/html_collection.h"
#include "third_party/blink/renderer/core/html/html_iframe_element.h"
#include "third_party/blink/renderer/core/html/html_style_element.h"
#include "third_party/blink/renderer/core/page/page.h"
#include "third_party/blink/renderer/platform/weborigin/kurl.h"

namespace blink {

FencedFrameShadowDOMDelegate::FencedFrameShadowDOMDelegate(
HTMLFencedFrameElement* outer_element)
: HTMLFencedFrameElement::FencedFrameDelegate(outer_element) {
DCHECK_EQ(features::kFencedFramesImplementationTypeParam.Get(),
DCHECK_EQ(outer_element->GetDocument()
.GetFrame()
->GetPage()
->FencedFramesImplementationType()
.value(),
features::FencedFramesImplementationType::kShadowDOM);
GetElement().EnsureUserAgentShadowRoot();

Expand Down Expand Up @@ -61,4 +67,10 @@ void FencedFrameShadowDOMDelegate::Navigate(const KURL& url) {
internal_iframe->setAttribute(html_names::kSrcAttr, url_string);
}

void FencedFrameShadowDOMDelegate::FreezeFrameSize() {
// With Shadow DOM, update the CSS `transform` property whenever
// |content_rect_| or |frozen_frame_size_| change.
GetElement().UpdateInnerStyleOnFrozenInternalFrame();
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class CORE_EXPORT FencedFrameShadowDOMDelegate
explicit FencedFrameShadowDOMDelegate(HTMLFencedFrameElement* outer_element);

void Navigate(const KURL&) override;
void FreezeFrameSize() override;

private:
void AddUserAgentShadowContent(ShadowRoot&);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,16 @@ class FencedFrameShadowDOMDelegateTest : private ScopedFencedFramesForTest,
public:
FencedFrameShadowDOMDelegateTest()
: ScopedFencedFramesForTest(true),
RenderingTest(MakeGarbageCollected<SingleChildLocalFrameClient>()) {}
RenderingTest(MakeGarbageCollected<SingleChildLocalFrameClient>()) {
enabled_feature_list_.InitWithFeaturesAndParameters(
{{blink::features::kFencedFrames,
{{"implementation_type", "shadow_dom"}}}},
{/* disabled_features */});
}

protected:
void SetUp() override {
RenderingTest::SetUp();
base::FieldTrialParams params;
params["implementation_type"] = "shadow_dom";
enabled_feature_list_.InitAndEnableFeatureWithParameters(
features::kFencedFrames, params);

SecurityContext& security_context =
GetDocument().GetFrame()->DomWindow()->GetSecurityContext();
security_context.SetSecurityOriginForTesting(nullptr);
Expand Down

0 comments on commit 078f30e

Please sign in to comment.