Propagate scrolling/marginwidth/marginheight property values to child frame.

content/browser/site_per_process_browsertest.cc

Index: content/browser/site_per_process_browsertest.cc
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index 8a907d52d7d44a38dfba270d7488ff27cf59572c..58b452701926d4f2fe9b5ce5629f2e1aab6efd1a 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -1836,6 +1836,169 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   }
 }
 
+// Verify that "scrolling" property on frame elements propagates to child frames
+// correctly.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       FrameOwnerPropertiesPropagationScrolling) {
+  GURL main_url(embedded_test_server()->GetURL(
+      "a.com", "/frame_owner_properties_scrolling.html"));
+  NavigateToURL(shell(), main_url);
+
+  // It is safe to obtain the root frame tree node here, as it doesn't change.
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+  ASSERT_EQ(1u, root->child_count());
+
+  EXPECT_EQ(
+      " Site A ------------ proxies for B\n"
+      "   +--Site B ------- proxies for A\n"
+      "Where A = http://a.com/\n"
+      "      B = http://b.com/",
+      DepictFrameTree(root));
+
+  FrameTreeNode* child = root->child_at(0);
+
+  auto has_scrollbar = [](RenderFrameHostImpl* rfh) {

[Charlie Reis, 2015/10/14 23:34:17]

Please add an explanatory comment, since the implementation of this lambda isn't
obviously connected to determining whether there's a scrollbar.

[lazyboy, 2015/10/15 02:31:39]

Done.

+    int client_width;
+    EXPECT_TRUE(ExecuteScriptAndExtractInt(rfh,
+        "window.domAutomationController.send(document.body.clientWidth);",
+        &client_width));
+    // Add a threshold to avoid any off-by-some pixels issue.
+    const int kThreshold = 4;

[Charlie Reis, 2015/10/14 23:34:17]

How wide is the scrollbar itself?  Is this lambda trying to detect the presence
of the scrollbar by the width, and hoping that the margin for error doesn't hide
the presence of the scrollbar itself?  Seems like we could be cutting it close.

[lazyboy, 2015/10/15 02:31:39]

This was supposed to prevent off by one -ish errors. Because there's default
border in iframes, we can be off by 4 (2 * 2px).
However, since both you and Alex seem to had comments around it, this
suggests this isn't a good idea. I've removed this and set the frame's
border to 0.

+    const int kFrameElementWidth = 200;
+    return client_width + kThreshold < kFrameElementWidth;
+  };
+
+  auto set_scrolling_property = [](RenderFrameHostImpl* parent_rfh,
+                                   const std::string& value) {
+    EXPECT_TRUE(ExecuteScript(
+        parent_rfh,
+        base::StringPrintf(
+            "document.getElementById('child-1').setAttribute("
+            "    'scrolling', '%s');", value.c_str())));
+  };
+
+  GURL urls[] = {

[Charlie Reis, 2015/10/14 23:34:17]

// Run the test over a variety of parent/child cases.

[lazyboy, 2015/10/15 02:31:39]

Done.

+    // Remote to remote.
+    embedded_test_server()->GetURL("b.com", "/tall_page.html"),

[Charlie Reis, 2015/10/14 23:34:17]

Isn't this a no-op, since we start on b.com?  c.com might be a more interesting
case, since it requires a remote-to-remote process swap.

[lazyboy, 2015/10/15 02:31:39]

Done.

+    // Remote to local.
+    embedded_test_server()->GetURL("a.com", "/tall_page.html"),
+    // Local to remote.
+    embedded_test_server()->GetURL("b.com", "/tall_page.html")
+  };
+  const std::string scrolling_values[] = {
+    "yes", "auto", "no"
+  };
+
+  for (size_t i = 0; i < arraysize(scrolling_values); ++i) {
+    bool expect_scrollbar = scrolling_values[i] != "no";
+    set_scrolling_property(root->current_frame_host(), scrolling_values[i]);
+    for (size_t j = 0; j < arraysize(urls); ++j) {
+      NavigateFrameToURL(child, urls[j]);
+
+      // TODO(alexmos): This can be removed once TestFrameNavigationObserver is
+      // fixed to use DidFinishLoad.
+      EXPECT_TRUE(WaitForRenderFrameReady(child->current_frame_host()));
+
+      EXPECT_EQ(expect_scrollbar, has_scrollbar(child->current_frame_host()));
+    }
+  }
+}
+
+// Verify that "marginwidth" and "marginheight" properties on frame elements
+// propagate to child frames correctly.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       FrameOwnerPropertiesPropagationMargin) {
+  GURL main_url(embedded_test_server()->GetURL(
+      "a.com", "/frame_owner_properties_margin.html"));
+  NavigateToURL(shell(), main_url);
+
+  // It is safe to obtain the root frame tree node here, as it doesn't change.
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+  ASSERT_EQ(1u, root->child_count());
+
+  EXPECT_EQ(
+      " Site A ------------ proxies for B\n"
+      "   +--Site B ------- proxies for A\n"
+      "Where A = http://a.com/\n"
+      "      B = http://b.com/",
+      DepictFrameTree(root));
+
+  FrameTreeNode* child = root->child_at(0);
+
+  std::string margin_width;
+  EXPECT_TRUE(ExecuteScriptAndExtractString(
+      child->current_frame_host(),
+      "window.domAutomationController.send("
+      "document.body.getAttribute('marginwidth'));",
+      &margin_width));
+  EXPECT_EQ("10", margin_width);
+
+  std::string margin_height;
+  EXPECT_TRUE(ExecuteScriptAndExtractString(
+      child->current_frame_host(),
+      "window.domAutomationController.send("
+      "document.body.getAttribute('marginheight'));",
+      &margin_height));
+  EXPECT_EQ("50", margin_height);
+
+  GURL urls[] = {
+    // Remote to remote.
+    embedded_test_server()->GetURL("b.com", "/title2.html"),

[Charlie Reis, 2015/10/14 23:34:17]

Same.

[lazyboy, 2015/10/15 02:31:39]

Done.

+    // Remote to local.
+    embedded_test_server()->GetURL("a.com", "/title1.html"),
+    // Local to remote.
+    embedded_test_server()->GetURL("b.com", "/title2.html")
+  };
+
+  int current_margin_width = 15;
+  int current_margin_height = 25;
+
+  // Before each navigation, we change the marginwidth and marginheight
+  // properties of the frame. We then check whether those properties are applied
+  // correctly after the navigation has completed.
+  for (size_t i = 0; i < arraysize(urls); ++i) {
+    // Change marginwidth and marginheight before navigating.
+    EXPECT_TRUE(ExecuteScript(
+        root->current_frame_host(),
+        base::StringPrintf(
+            "document.getElementById('child-1').setAttribute("
+            "    'marginwidth', '%d');", current_margin_width)));
+    EXPECT_TRUE(ExecuteScript(
+        root->current_frame_host(),
+        base::StringPrintf(
+            "document.getElementById('child-1').setAttribute("
+            "    'marginheight', '%d');", current_margin_height)));
+
+    NavigateFrameToURL(child, urls[i]);
+    // TODO(alexmos): This can be removed once TestFrameNavigationObserver is
+    // fixed to use DidFinishLoad.
+    EXPECT_TRUE(WaitForRenderFrameReady(child->current_frame_host()));
+
+    std::string actual_margin_width;
+    EXPECT_TRUE(ExecuteScriptAndExtractString(
+        child->current_frame_host(),
+        "window.domAutomationController.send("
+        "document.body.getAttribute('marginwidth'));",
+        &actual_margin_width));
+    EXPECT_EQ(base::IntToString(current_margin_width), actual_margin_width);
+
+    std::string actual_margin_height;
+    EXPECT_TRUE(ExecuteScriptAndExtractString(
+        child->current_frame_host(),
+        "window.domAutomationController.send("
+        "document.body.getAttribute('marginheight'));",
+        &actual_margin_height));
+    EXPECT_EQ(base::IntToString(current_margin_height), actual_margin_height);
+
+    current_margin_width += 5;
+    current_margin_height += 10;
+  }
+}
+
 // Verify origin replication with an A-embed-B-embed-C-embed-A hierarchy.
 IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, OriginReplication) {
   GURL main_url(embedded_test_server()->GetURL(