Dispatch WCO::RenderFrameCreated before WCO::RenderFrameHostChanged for iframes

Currently, WebContentsObserver::RenderFrameHostChanged callback is
being dispatched before WebContentsObserver::RenderFrameCreated callback
for child frames.

This is problematic as embedders use
WebContentsObserver::RenderFrameCreated callback to initialize
per-RenderFrameHost state. This CL fixes this ordering.

BUG=1097143

Change-Id: Iced632ee3369b3dbf3822dd57620b687ed47cb5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2614467
Commit-Queue: Sreeja Kamishetty <sreejakshetty@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846079}

content/browser/renderer_host/frame_tree.cc

@@ -254,11 +254,19 @@ FrameTreeNode* FrameTree::AddFrame(
   // Now that the new node is part of the FrameTree and has a RenderFrameHost,
   // we can announce the creation of the initial RenderFrame which already
   // exists in the renderer process.
+  // For consistency with navigating to a new RenderFrameHost case, we dispatch
+  // RenderFrameCreated before RenderFrameHostChanged.
   if (added_node->frame_owner_element_type() !=
       blink::mojom::FrameOwnerElementType::kPortal) {
     // Portals do not have a live RenderFrame in the renderer process.
     added_node->current_frame_host()->RenderFrameCreated();
   }
+
+  // Notify the delegate of the creation of the current RenderFrameHost.
+  // This is only for subframes, as the main frame case is taken care of by
+  // WebContentsImpl::Init.
+  manager_delegate_->NotifySwappedFromRenderManager(
+      nullptr, added_node->current_frame_host(), false /* is_main_frame */);
   return added_node;
 }
 

content/browser/renderer_host/frame_tree_unittest.cc

@@ -506,8 +506,8 @@ TEST_F(FrameTreeTest, ObserverWalksTreeDuringFrameCreation) {
       false, base::UnguessableToken::Create(), base::UnguessableToken::Create(),
       blink::FramePolicy(), blink::mojom::FrameOwnerProperties(), kOwnerType);
   EXPECT_EQ(
-      "RenderFrameHostChanged(new)(14) -> 1: []\n"
-      "RenderFrameCreated(14) -> 1: [14: []]",
+      "RenderFrameCreated(14) -> 1: [14: []]\n"
+      "RenderFrameHostChanged(new)(14) -> 1: [14: []]",
       activity.GetLog());
   main_test_rfh()->OnCreateChildFrame(
       18, CreateStubFrameRemote(), CreateStubBrowserInterfaceBrokerReceiver(),
@@ -516,8 +516,8 @@ TEST_F(FrameTreeTest, ObserverWalksTreeDuringFrameCreation) {
       false, base::UnguessableToken::Create(), base::UnguessableToken::Create(),
       blink::FramePolicy(), blink::mojom::FrameOwnerProperties(), kOwnerType);
   EXPECT_EQ(
-      "RenderFrameHostChanged(new)(18) -> 1: [14: []]\n"
-      "RenderFrameCreated(18) -> 1: [14: [], 18: []]",
+      "RenderFrameCreated(18) -> 1: [14: [], 18: []]\n"
+      "RenderFrameHostChanged(new)(18) -> 1: [14: [], 18: []]",
       activity.GetLog());
   frame_tree->RemoveFrame(root->child_at(0));
   EXPECT_EQ("RenderFrameDeleted(14) -> 1: [18: []]", activity.GetLog());
@@ -540,8 +540,8 @@ TEST_F(FrameTreeTest, ObserverWalksTreeAfterCrash) {
       false, base::UnguessableToken::Create(), base::UnguessableToken::Create(),
       blink::FramePolicy(), blink::mojom::FrameOwnerProperties(), kOwnerType);
   EXPECT_EQ(
-      "RenderFrameHostChanged(new)(22) -> 1: []\n"
-      "RenderFrameCreated(22) -> 1: [22: []]",
+      "RenderFrameCreated(22) -> 1: [22: []]\n"
+      "RenderFrameHostChanged(new)(22) -> 1: [22: []]",
       activity.GetLog());
   main_test_rfh()->OnCreateChildFrame(
       23, CreateStubFrameRemote(), CreateStubBrowserInterfaceBrokerReceiver(),
@@ -550,8 +550,8 @@ TEST_F(FrameTreeTest, ObserverWalksTreeAfterCrash) {
       false, base::UnguessableToken::Create(), base::UnguessableToken::Create(),
       blink::FramePolicy(), blink::mojom::FrameOwnerProperties(), kOwnerType);
   EXPECT_EQ(
-      "RenderFrameHostChanged(new)(23) -> 1: [22: []]\n"
-      "RenderFrameCreated(23) -> 1: [22: [], 23: []]",
+      "RenderFrameCreated(23) -> 1: [22: [], 23: []]\n"
+      "RenderFrameHostChanged(new)(23) -> 1: [22: [], 23: []]",
       activity.GetLog());
 
   // Crash the renderer

content/browser/renderer_host/render_frame_host_manager.cc

@@ -233,11 +233,6 @@ void RenderFrameHostManager::InitChild(
       CreateFrameCase::kInitChild, site_instance, frame_routing_id,
       std::move(frame_remote), frame_token,
       /*renderer_initiated_creation=*/false));
-  // Notify the delegate of the creation of the current RenderFrameHost.
-  // Do this only for subframes, as the main frame case is taken care of by
-  // WebContentsImpl::Init.
-  delegate_->NotifySwappedFromRenderManager(nullptr, render_frame_host_.get(),
-                                            false);
 }
 
 RenderViewHostImpl* RenderFrameHostManager::current_host() const {

content/public/browser/render_frame_host.h

@@ -431,7 +431,7 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener,
   // of a frame are defined in Blink.
   virtual PageVisibilityState GetVisibilityState() = 0;
 
-  // Returns true if WebContentsObserver::RenderFrameCreate notification has
+  // Returns true if WebContentsObserver::RenderFrameCreated notification has
   // been dispatched for this frame, and so a RenderFrameDeleted notification
   // will later be dispatched for this frame.
   virtual bool IsRenderFrameCreated() = 0;