Fixes timing issue with frame creation

This patch gates OnConnect() until the client has responsed back from
OnWillNavigate(). This is important for the case of the parent and
client being in the same process.

Consider the following. HTML Frame P creates a child, C, and navigates
C to a new url C' and the loading status of C' changes.

P creates C
C navigates to C':
  C::OnWillNavigate()
  C'::OnConnect()
C' updates loading state
  C'<server>::LoadingStateChanged
  P::OnFrameLoadingStateChanged()

Problem is, this may be processed as:

P creates C
C navigates to C':
  C'::OnConnect()
C' updates loading state
  C'<server>::LoadingStateChanged
  P::OnFrameLoadingStateChanged()
  C::OnWillNavigate()

If this order results (which telemetry triggers), then when P gets
OnFrameLoadingStateChanged it has C as still local and ignores the
loading state change.

By only calling OnConnect() after the client has hacked
OnWillNavigate() we ensure the order as first outlined.

BUG=none
TEST=none
R=yzshen@chromium.org

Review URL: https://codereview.chromium.org/1362983003

Cr-Commit-Position: refs/heads/master@{#350392}

1 files changed, 10 insertions, 0 deletions

diff --git a/components/html_viewer/html_frame_tree_manager.cc b/components/html_viewer/html_frame_tree_manager.cc
index e8b2de4..6d8d12e 100644
--- a/components/html_viewer/html_frame_tree_manager.cc
+++ b/components/html_viewer/html_frame_tree_manager.cc
@@ -94,6 +94,11 @@ HTMLFrame* HTMLFrameTreeManager::CreateFrameAndAttachToTree(
 
   DCHECK(!frame_tree || change_id <= frame_tree->change_id_);
 
+  DVLOG(2) << "HTMLFrameTreeManager::CreateFrameAndAttachToTree "
+           << " frame_tree=" << frame_tree << " use_existing="
+           << (view_connect_type ==
+               web_view::mojom::VIEW_CONNECT_TYPE_USE_EXISTING)
+           << " frame_id=" << view_id;
   if (view_connect_type == web_view::mojom::VIEW_CONNECT_TYPE_USE_EXISTING &&
       !frame_tree) {
     DVLOG(1) << "was told to use existing view but do not have frame tree";
@@ -300,6 +305,9 @@ void HTMLFrameTreeManager::ProcessOnFrameAdded(
   if (pending_remove_ids_.count(frame_data->frame_id))
     return;
 
+  DVLOG(2) << "OnFrameAdded this=" << this
+           << " frame_id=" << frame_data->frame_id;
+
   HTMLFrame::CreateParams params(this, parent, frame_data->frame_id, nullptr,
                                  frame_data->client_properties, nullptr);
   // |parent| takes ownership of created HTMLFrame.
@@ -335,6 +343,8 @@ void HTMLFrameTreeManager::ProcessOnFrameRemoved(HTMLFrame* source,
   if (frame->IsLocal())
     return;
 
+  DVLOG(2) << "OnFrameRemoved this=" << this << " frame_id=" << frame_id;
+
   DCHECK(!in_process_on_frame_removed_);
   in_process_on_frame_removed_ = true;
   base::WeakPtr<HTMLFrameTreeManager> ref(weak_factory_.GetWeakPtr());