Composite frames that are cross-origin to their parent

This patch fixes a bug in https://crrev.com/732194 where subframes that
were cross-origin to their parent were not composited. This mistake was
due to a misreading of Frame::IsCrossOriginSubframe which returns true
if the frame is cross origin to the main frame. "IsCrossOriginSubframe"
has been renamed to "IsCrossOriginToMainFrame" in
https://crrev.com/737198 to prevent similar mistakes.

This patch adds Frame::IsCrossOriginToParentFrame and uses it when
deciding whether to composite an iframe.

Bug: 1047497, 1014273
Change-Id: Iecef138be2b083953559fb856d60939cdfbc8c60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2034031
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739016}

third_party/blink/renderer/core/dom/document.cc

@@ -5739,6 +5739,8 @@ void Document::setDomain(const String& raw_domain,
                           ? WebFeature::kDocumentDomainSetWithDefaultPort
                           : WebFeature::kDocumentDomainSetWithNonDefaultPort);
     bool was_cross_origin_to_main_frame = frame_->IsCrossOriginToMainFrame();
+    bool was_cross_origin_to_parent_frame =
+        frame_->IsCrossOriginToParentFrame();
     GetMutableSecurityOrigin()->SetDomainFromDOM(new_domain);
     bool is_cross_origin_to_main_frame = frame_->IsCrossOriginToMainFrame();
     if (FrameScheduler* frame_scheduler = frame_->GetFrameScheduler())
@@ -5760,6 +5762,21 @@ void Document::setDomain(const String& raw_domain,
       }
     }
 
+    if (View() && was_cross_origin_to_parent_frame !=
+                      frame_->IsCrossOriginToParentFrame()) {
+      View()->CrossOriginToParentFrameChanged();
+    }
+    // Notify all child frames if their cross-origin-to-parent status changed.
+    // TODO(pdr): This will notify even if |Frame::IsCrossOriginToParentFrame|
+    // is the same. Track whether each child was cross-origin-to-parent before
+    // and after changing the domain, and only notify the changed ones.
+    for (Frame* child = frame_->Tree().FirstChild(); child;
+         child = child->Tree().NextSibling()) {
+      auto* child_local_frame = DynamicTo<LocalFrame>(child);
+      if (child_local_frame && child_local_frame->View())
+        child_local_frame->View()->CrossOriginToParentFrameChanged();
+    }
+
     frame_->GetScriptController().UpdateSecurityOrigin(GetSecurityOrigin());
   }
 }

third_party/blink/renderer/core/frame/frame.cc

@@ -136,6 +136,22 @@ bool Frame::IsCrossOriginToMainFrame() const {
       Tree().Top().GetSecurityContext()->GetSecurityOrigin());
 }
 
+bool Frame::IsCrossOriginToParentFrame() const {
+  DCHECK(GetSecurityContext());
+  if (IsMainFrame())
+    return false;
+  Frame* parent = Tree().Parent();
+  const SecurityOrigin* parent_security_origin =
+      parent->GetSecurityContext()->GetSecurityOrigin();
+  // TODO(dcheng, crbug.com/1049612): The parent security origin should not be
+  // null.
+  if (!parent_security_origin)
+    return true;
+  const SecurityOrigin* security_origin =
+      GetSecurityContext()->GetSecurityOrigin();
+  return !security_origin->CanAccess(parent_security_origin);
+}
+
 HTMLFrameOwnerElement* Frame::DeprecatedLocalOwner() const {
   return DynamicTo<HTMLFrameOwnerElement>(owner_.Get());
 }

third_party/blink/renderer/core/frame/frame.h

@@ -107,7 +107,7 @@ class CORE_EXPORT Frame : public GarbageCollected<Frame> {
   //
   // Important notes:
   // - This function is not appropriate for determining if a subframe is
-  //   cross-origin to its parent.
+  //   cross-origin to its parent (see: |IsCrossOriginToParentFrame|).
   // - The return value must NOT be cached. A frame can be reused across
   //   navigations, so the return value can change over time.
   // - The return value is inaccurate for a detached frame: it always
@@ -115,6 +115,9 @@ class CORE_EXPORT Frame : public GarbageCollected<Frame> {
   // TODO(dcheng): Move this to LocalDOMWindow and figure out the right
   // behavior for detached windows.
   bool IsCrossOriginToMainFrame() const;
+  // Returns true if this frame is a subframe and is cross-origin to the parent
+  // frame. See |IsCrossOriginToMainFrame| for important notes.
+  bool IsCrossOriginToParentFrame() const;
 
   FrameOwner* Owner() const;
   void SetOwner(FrameOwner*);

third_party/blink/renderer/core/frame/local_frame_view.cc

@@ -3985,9 +3985,6 @@ void LocalFrameView::DeliverSynchronousIntersectionObservations() {
 }
 
 void LocalFrameView::CrossOriginToMainFrameChanged() {
-  if (auto* owner = frame_->DeprecatedLocalOwner())
-    owner->FrameCrossOriginStatusChanged();
-
   // If any of these conditions hold, then a change in cross-origin status does
   // not affect throttling.
   if (lifecycle_updates_throttled_ || IsSubtreeThrottled() ||
@@ -4004,6 +4001,11 @@ void LocalFrameView::CrossOriginToMainFrameChanged() {
                                true);
 }
 
+void LocalFrameView::CrossOriginToParentFrameChanged() {
+  if (auto* owner = frame_->DeprecatedLocalOwner())
+    owner->FrameCrossOriginToParentFrameChanged();
+}
+
 void LocalFrameView::VisibilityForThrottlingChanged() {
   if (FrameScheduler* frame_scheduler = frame_->GetFrameScheduler()) {
     // TODO(szager): Per crbug.com/994443, maybe this should be:

third_party/blink/renderer/core/frame/local_frame_view.h

@@ -621,6 +621,7 @@ class CORE_EXPORT LocalFrameView final
   void MapLocalToRemoteRootFrame(TransformState&);
 
   void CrossOriginToMainFrameChanged();
+  void CrossOriginToParentFrameChanged();
 
   // The visual viewport can supply scrollbars.
   void VisualViewportScrollbarsChanged();

third_party/blink/renderer/core/html/html_frame_owner_element.cc

@@ -561,7 +561,7 @@ void HTMLFrameOwnerElement::ParseAttribute(
   }
 }
 
-void HTMLFrameOwnerElement::FrameCrossOriginStatusChanged() {
+void HTMLFrameOwnerElement::FrameCrossOriginToParentFrameChanged() {
   if (base::FeatureList::IsEnabled(
           blink::features::kCompositeCrossOriginIframes)) {
     SetNeedsCompositingUpdate();

third_party/blink/renderer/core/html/html_frame_owner_element.h

@@ -74,7 +74,7 @@ class CORE_EXPORT HTMLFrameOwnerElement : public HTMLElement,
     return embedded_content_view_;
   }
 
-  void FrameCrossOriginStatusChanged();
+  void FrameCrossOriginToParentFrameChanged();
 
   class PluginDisposeSuspendScope {
     STACK_ALLOCATED();

third_party/blink/renderer/core/layout/layout_embedded_content.cc

@@ -133,7 +133,7 @@ bool LayoutEmbeddedContent::RequiresAcceleratedCompositing() const {
       return true;
     if (base::FeatureList::IsEnabled(
             blink::features::kCompositeCrossOriginIframes) &&
-        content_frame->IsCrossOriginToMainFrame()) {
+        content_frame->IsCrossOriginToParentFrame()) {
       return true;
     }
   }

third_party/blink/renderer/core/loader/document_loader.cc

@@ -1507,8 +1507,8 @@ void DocumentLoader::InstallNewDocument(
     previous_security_origin = frame_->GetDocument()->GetSecurityOrigin();
   }
 
-  bool was_cross_origin_to_main_frame =
-      previous_security_origin && frame_->IsCrossOriginToMainFrame();
+  bool was_cross_origin_to_parent_frame =
+      previous_security_origin && frame_->IsCrossOriginToParentFrame();
 
   // In some rare cases, we'll re-use a LocalDOMWindow for a new Document. For
   // example, when a script calls window.open("..."), the browser gives
@@ -1567,9 +1567,10 @@ void DocumentLoader::InstallNewDocument(
   if (!loading_url_as_javascript_)
     DidCommitNavigation();
 
-  if (was_cross_origin_to_main_frame != frame_->IsCrossOriginToMainFrame()) {
+  if (was_cross_origin_to_parent_frame !=
+      frame_->IsCrossOriginToParentFrame()) {
     if (auto* owner = frame_->DeprecatedLocalOwner())
-      owner->FrameCrossOriginStatusChanged();
+      owner->FrameCrossOriginToParentFrameChanged();
   }
 
   if (initiator_origin) {

third_party/blink/renderer/core/paint/compositing/compositing_test.cc

@@ -1104,6 +1104,34 @@ TEST_P(CompositingSimTest, PromoteCrossOriginIframeAfterLoading) {
   EXPECT_TRUE(CcLayerByDOMElementId("iframe"));
 }
 
+// An iframe that is cross-origin to the parent should be composited. This test
+// sets up nested frames with domains A -> B -> A. Both the child and grandchild
+// frames should be composited because they are cross-origin to their parent.
+TEST_P(CompositingSimTest, PromoteCrossOriginToParent) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitWithFeatureState(
+      blink::features::kCompositeCrossOriginIframes, true);
+
+  SimRequest main_resource("https://origin-a.com/a.html", "text/html");
+  SimRequest child_resource("https://origin-b.com/b.html", "text/html");
+  SimRequest grandchild_resource("https://origin-a.com/c.html", "text/html");
+
+  LoadURL("https://origin-a.com/a.html");
+  main_resource.Complete(R"HTML(
+      <!DOCTYPE html>
+      <iframe id="main_iframe" src="https://origin-b.com/b.html"></iframe>
+  )HTML");
+  child_resource.Complete(R"HTML(
+      <!DOCTYPE html>
+      <iframe id="child_iframe" src="https://origin-a.com/c.html"></iframe>
+  )HTML");
+  grandchild_resource.Complete("<!DOCTYPE html>");
+  Compositor().BeginFrame();
+
+  EXPECT_TRUE(CcLayerByDOMElementId("main_iframe"));
+  EXPECT_TRUE(CcLayerByDOMElementId("child_iframe"));
+}
+
 // Initially the iframe is cross-origin and should be composited. After changing
 // to same-origin, the frame should no longer be composited.
 TEST_P(CompositingSimTest, PromoteCrossOriginIframeAfterDomainChange) {
@@ -1137,4 +1165,51 @@ TEST_P(CompositingSimTest, PromoteCrossOriginIframeAfterDomainChange) {
   EXPECT_FALSE(CcLayerByDOMElementId("iframe"));
 }
 
+// This test sets up nested frames with domains A -> B -> A. Initially, the
+// child frame and grandchild frame should be composited. After changing the
+// child frame to A (same-origin), both child and grandchild frames should no
+// longer be composited.
+TEST_P(CompositingSimTest, PromoteCrossOriginToParentIframeAfterDomainChange) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitWithFeatureState(
+      blink::features::kCompositeCrossOriginIframes, true);
+
+  SimRequest main_resource("https://origin-a.com/a.html", "text/html");
+  SimRequest child_resource("https://sub.origin-a.com/b.html", "text/html");
+  SimRequest grandchild_resource("https://origin-a.com/c.html", "text/html");
+
+  LoadURL("https://origin-a.com/a.html");
+  main_resource.Complete(R"HTML(
+      <!DOCTYPE html>
+      <iframe id="main_iframe" src="https://sub.origin-a.com/b.html"></iframe>
+  )HTML");
+  child_resource.Complete(R"HTML(
+      <!DOCTYPE html>
+      <iframe id="child_iframe" src="https://origin-a.com/c.html"></iframe>
+  )HTML");
+  grandchild_resource.Complete("<!DOCTYPE html>");
+  Compositor().BeginFrame();
+
+  EXPECT_TRUE(CcLayerByDOMElementId("main_iframe"));
+  EXPECT_TRUE(CcLayerByDOMElementId("child_iframe"));
+
+  auto* main_iframe_element =
+      To<HTMLIFrameElement>(GetDocument().getElementById("main_iframe"));
+  NonThrowableExceptionState exception_state;
+  GetDocument().setDomain(String("origin-a.com"), exception_state);
+  auto* child_iframe_element = To<HTMLIFrameElement>(
+      main_iframe_element->contentDocument()->getElementById("child_iframe"));
+  child_iframe_element->contentDocument()->setDomain(String("origin-a.com"),
+                                                     exception_state);
+  main_iframe_element->contentDocument()->setDomain(String("origin-a.com"),
+                                                    exception_state);
+
+  // We may not have scheduled a visual update so force an update instead of
+  // using BeginFrame.
+  UpdateAllLifecyclePhases();
+
+  EXPECT_FALSE(CcLayerByDOMElementId("main_iframe"));
+  EXPECT_FALSE(CcLayerByDOMElementId("child_iframe"));
+}
+
 }  // namespace blink