Replicate whether a frame is in a document or shadow tree.

content/browser/frame_host/frame_tree_unittest.cc

Index: content/browser/frame_host/frame_tree_unittest.cc
diff --git a/content/browser/frame_host/frame_tree_unittest.cc b/content/browser/frame_host/frame_tree_unittest.cc
index 96f6b4d9466f14b806183d8b548c774d5a350782..cf633e60307c44b36bbac11926834545025a320b 100644
--- a/content/browser/frame_host/frame_tree_unittest.cc
+++ b/content/browser/frame_host/frame_tree_unittest.cc
@@ -140,18 +140,21 @@ TEST_F(FrameTreeTest, DISABLED_Shape) {
   ASSERT_EQ("1: []", GetTreeState(frame_tree));
 
   // Simulate attaching a series of frames to build the frame tree.
-  frame_tree->AddFrame(root, process_id, 14, std::string(),
-                       SandboxFlags::NONE);
-  frame_tree->AddFrame(root, process_id, 15, std::string(),
-                       SandboxFlags::NONE);
-  frame_tree->AddFrame(root, process_id, 16, std::string(),
-                       SandboxFlags::NONE);
+  frame_tree->AddFrame(root, process_id, 14, blink::WebTreeScopeType::Document,

[Charlie Reis, 2015/05/19 23:17:10]

I agree that these changes are a bit unwieldy for every time a new item is added
to replication state.  Would it help for AddFrame to take in a
FrameReplicationState and just be able to pass a default one?

[dcheng, 2015/05/20 00:09:29]

That doesn't really help us a lot in tests though, and that's where most of the
churn is.

e.g. this line would just turn into:
frame_tree->AddFrame(root, process_id, 14, FrameTreeReplicationState(...));

IMO, passing structs of params around also makes calls and the function
definition itself harder to read: you just know you have a bunch of stuff in
"params" and then you have to go find the struct definition elsewhere.

[Charlie Reis, 2015/05/20 21:38:40]

That's why I said "pass in a default one."  Almost all of these tests just use
the default values for each of the replication state parameters, which the
zero-argument FrameReplicationState() would give you for free.  No changes to
most of the tests would be required each time we add a new flag.
That makes sense when there's just a few parameters, but it gets unwieldy fast. 
I think we're going to need something that scales for FrameReplicationState, but
Alex would have a better sense.

[dcheng, 2015/05/21 18:16:16]

Sure, but AddFrame() doesn't need the origin, which FrameReplicationState
specifies. It seems weird to pass the entire FrameReplicationState along when
not all fields are needed.
At most, I expect that we'll need to replicate one or two more things, so I
don't expect this to be a problem in practice.

[Charlie Reis, 2015/05/21 23:18:17]

Ah.  Yes, it would be unfortunate to have to pass in more than is needed just to
make it easier to call in tests.
That's seems overly optimistic to me, but I'll defer for now.  :)

+                       std::string(), SandboxFlags::NONE);
+  frame_tree->AddFrame(root, process_id, 15, blink::WebTreeScopeType::Document,
+                       std::string(), SandboxFlags::NONE);
+  frame_tree->AddFrame(root, process_id, 16, blink::WebTreeScopeType::Document,
+                       std::string(), SandboxFlags::NONE);
 
-  frame_tree->AddFrame(root->child_at(0), process_id, 244, std::string(),
+  frame_tree->AddFrame(root->child_at(0), process_id, 244,
+                       blink::WebTreeScopeType::Document, std::string(),
                        SandboxFlags::NONE);
-  frame_tree->AddFrame(root->child_at(1), process_id, 255, no_children_node,
+  frame_tree->AddFrame(root->child_at(1), process_id, 255,
+                       blink::WebTreeScopeType::Document, no_children_node,
                        SandboxFlags::NONE);
-  frame_tree->AddFrame(root->child_at(0), process_id, 245, std::string(),
+  frame_tree->AddFrame(root->child_at(0), process_id, 245,
+                       blink::WebTreeScopeType::Document, std::string(),
                        SandboxFlags::NONE);
 
   ASSERT_EQ("1: [14: [244: [], 245: []], "
@@ -160,26 +163,35 @@ TEST_F(FrameTreeTest, DISABLED_Shape) {
             GetTreeState(frame_tree));
 
   FrameTreeNode* child_16 = root->child_at(2);
-  frame_tree->AddFrame(child_16, process_id, 264, std::string(),
+  frame_tree->AddFrame(child_16, process_id, 264,
+                       blink::WebTreeScopeType::Document, std::string(),
                        SandboxFlags::NONE);
-  frame_tree->AddFrame(child_16, process_id, 265, std::string(),
+  frame_tree->AddFrame(child_16, process_id, 265,
+                       blink::WebTreeScopeType::Document, std::string(),
                        SandboxFlags::NONE);
-  frame_tree->AddFrame(child_16, process_id, 266, std::string(),
+  frame_tree->AddFrame(child_16, process_id, 266,
+                       blink::WebTreeScopeType::Document, std::string(),
                        SandboxFlags::NONE);
-  frame_tree->AddFrame(child_16, process_id, 267, deep_subtree,
+  frame_tree->AddFrame(child_16, process_id, 267,
+                       blink::WebTreeScopeType::Document, deep_subtree,
                        SandboxFlags::NONE);
-  frame_tree->AddFrame(child_16, process_id, 268, std::string(),
+  frame_tree->AddFrame(child_16, process_id, 268,
+                       blink::WebTreeScopeType::Document, std::string(),
                        SandboxFlags::NONE);
 
   FrameTreeNode* child_267 = child_16->child_at(3);
-  frame_tree->AddFrame(child_267, process_id, 365, std::string(),
+  frame_tree->AddFrame(child_267, process_id, 365,
+                       blink::WebTreeScopeType::Document, std::string(),
                        SandboxFlags::NONE);
-  frame_tree->AddFrame(child_267->child_at(0), process_id, 455, std::string(),
+  frame_tree->AddFrame(child_267->child_at(0), process_id, 455,
+                       blink::WebTreeScopeType::Document, std::string(),
                        SandboxFlags::NONE);
   frame_tree->AddFrame(child_267->child_at(0)->child_at(0), process_id, 555,
-                       std::string(), SandboxFlags::NONE);
+                       blink::WebTreeScopeType::Document, std::string(),
+                       SandboxFlags::NONE);
   frame_tree->AddFrame(child_267->child_at(0)->child_at(0)->child_at(0),
-                       process_id, 655, std::string(), SandboxFlags::NONE);
+                       process_id, 655, blink::WebTreeScopeType::Document,
+                       std::string(), SandboxFlags::NONE);
 
   // Now that's it's fully built, verify the tree structure is as expected.
   ASSERT_EQ("1: [14: [244: [], 245: []], "
@@ -219,16 +231,20 @@ TEST_F(FrameTreeTest, FindFrames) {
   // Add a few child frames to the main frame.
   FrameTree* frame_tree = contents()->GetFrameTree();
   FrameTreeNode* root = frame_tree->root();
-  main_test_rfh()->OnCreateChildFrame(22, "child0", SandboxFlags::NONE);
-  main_test_rfh()->OnCreateChildFrame(23, "child1", SandboxFlags::NONE);
-  main_test_rfh()->OnCreateChildFrame(24, std::string(), SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(22, blink::WebTreeScopeType::Document,
+                                      "child0", SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(23, blink::WebTreeScopeType::Document,
+                                      "child1", SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(24, blink::WebTreeScopeType::Document,
+                                      std::string(), SandboxFlags::NONE);
   FrameTreeNode* child0 = root->child_at(0);
   FrameTreeNode* child1 = root->child_at(1);
+
   FrameTreeNode* child2 = root->child_at(2);
 
   // Add one grandchild frame.
-  child1->current_frame_host()->OnCreateChildFrame(33, "grandchild",
-                                                   SandboxFlags::NONE);
+  child1->current_frame_host()->OnCreateChildFrame(
+      33, blink::WebTreeScopeType::Document, "grandchild", SandboxFlags::NONE);
   FrameTreeNode* grandchild = child1->child_at(0);
 
   // Ensure they can be found by FTN id.
@@ -262,16 +278,19 @@ TEST_F(FrameTreeTest, PreviousSibling) {
   // Add a few child frames to the main frame.
   FrameTree* frame_tree = contents()->GetFrameTree();
   FrameTreeNode* root = frame_tree->root();
-  main_test_rfh()->OnCreateChildFrame(22, "child0", SandboxFlags::NONE);
-  main_test_rfh()->OnCreateChildFrame(23, "child1", SandboxFlags::NONE);
-  main_test_rfh()->OnCreateChildFrame(24, "child2", SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(22, blink::WebTreeScopeType::Document,
+                                      "child0", SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(23, blink::WebTreeScopeType::Document,
+                                      "child1", SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(24, blink::WebTreeScopeType::Document,
+                                      "child2", SandboxFlags::NONE);
   FrameTreeNode* child0 = root->child_at(0);
   FrameTreeNode* child1 = root->child_at(1);
   FrameTreeNode* child2 = root->child_at(2);
 
   // Add one grandchild frame.
-  child1->current_frame_host()->OnCreateChildFrame(33, "grandchild",
-                                                   SandboxFlags::NONE);
+  child1->current_frame_host()->OnCreateChildFrame(
+      33, blink::WebTreeScopeType::Document, "grandchild", SandboxFlags::NONE);
   FrameTreeNode* grandchild = child1->child_at(0);
 
   EXPECT_EQ(nullptr, root->PreviousSibling());
@@ -292,12 +311,14 @@ TEST_F(FrameTreeTest, ObserverWalksTreeDuringFrameCreation) {
   FrameTreeNode* root = frame_tree->root();
 
   // Simulate attaching a series of frames to build the frame tree.
-  main_test_rfh()->OnCreateChildFrame(14, std::string(), SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(14, blink::WebTreeScopeType::Document,
+                                      std::string(), SandboxFlags::NONE);
   EXPECT_EQ(
       "RenderFrameHostChanged(new)(14) -> 1: []\n"
       "RenderFrameCreated(14) -> 1: [14: []]",
       activity.GetLog());
-  main_test_rfh()->OnCreateChildFrame(18, std::string(), SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(18, blink::WebTreeScopeType::Document,
+                                      std::string(), SandboxFlags::NONE);
   EXPECT_EQ(
       "RenderFrameHostChanged(new)(18) -> 1: [14: []]\n"
       "RenderFrameCreated(18) -> 1: [14: [], 18: []]",
@@ -315,12 +336,14 @@ TEST_F(FrameTreeTest, ObserverWalksTreeAfterCrash) {
   contents()->NavigateAndCommit(GURL("http://www.google.com"));
   EXPECT_EQ("", activity.GetLog());
 
-  main_test_rfh()->OnCreateChildFrame(22, std::string(), SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(22, blink::WebTreeScopeType::Document,
+                                      std::string(), SandboxFlags::NONE);
   EXPECT_EQ(
       "RenderFrameHostChanged(new)(22) -> 1: []\n"
       "RenderFrameCreated(22) -> 1: [22: []]",
       activity.GetLog());
-  main_test_rfh()->OnCreateChildFrame(23, std::string(), SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(23, blink::WebTreeScopeType::Document,
+                                      std::string(), SandboxFlags::NONE);
   EXPECT_EQ(
       "RenderFrameHostChanged(new)(23) -> 1: [22: []]\n"
       "RenderFrameCreated(23) -> 1: [22: [], 23: []]",
@@ -347,8 +370,9 @@ TEST_F(FrameTreeTest, FailAddFrameWithWrongProcessId) {
   ASSERT_EQ("1: []", GetTreeState(frame_tree));
 
   // Simulate attaching a frame from mismatched process id.
-  ASSERT_FALSE(frame_tree->AddFrame(root, process_id + 1, 1, std::string(),
-                                    SandboxFlags::NONE));
+  ASSERT_FALSE(frame_tree->AddFrame(root, process_id + 1, 1,
+                                    blink::WebTreeScopeType::Document,
+                                    std::string(), SandboxFlags::NONE));
   ASSERT_EQ("1: []", GetTreeState(frame_tree));
 }
 
@@ -358,12 +382,15 @@ TEST_F(FrameTreeTest, ProcessCrashClearsGlobalMap) {
   // Add a couple child frames to the main frame.
   FrameTreeNode* root = contents()->GetFrameTree()->root();
 
-  main_test_rfh()->OnCreateChildFrame(22, std::string(), SandboxFlags::NONE);
-  main_test_rfh()->OnCreateChildFrame(23, std::string(), SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(22, blink::WebTreeScopeType::Document,
+                                      std::string(), SandboxFlags::NONE);
+  main_test_rfh()->OnCreateChildFrame(23, blink::WebTreeScopeType::Document,
+                                      std::string(), SandboxFlags::NONE);
 
   // Add one grandchild frame.
   RenderFrameHostImpl* child1_rfh = root->child_at(0)->current_frame_host();
-  child1_rfh->OnCreateChildFrame(33, std::string(), SandboxFlags::NONE);
+  child1_rfh->OnCreateChildFrame(33, blink::WebTreeScopeType::Document,
+                                 std::string(), SandboxFlags::NONE);
 
   // Ensure they can be found by id.
   int id1 = root->child_at(0)->frame_tree_node_id();