Fixes possible race in new connection setup

The frame tree uses change_ids to track the tree status. When a
connection is made to a frame it is entirely possible for the new
connection to have a change_id > change_id seen by tree. To deal with
this HTMLFrameTreeManager gets an observer. DocumentResourceWaiter, as
necessary, adds itself as an observer and waits for the change ids to
match. Once they match, loading continues.

BUG=479172,490221
TEST=none
R=fsamuel@chromium.org

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

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

1 files changed, 47 insertions, 7 deletions

diff --git a/components/html_viewer/html_frame_tree_manager.cc b/components/html_viewer/html_frame_tree_manager.cc
index ad9e351..7de88a1 100644
--- a/components/html_viewer/html_frame_tree_manager.cc
+++ b/components/html_viewer/html_frame_tree_manager.cc
@@ -15,6 +15,7 @@
 #include "components/html_viewer/html_factory.h"
 #include "components/html_viewer/html_frame.h"
 #include "components/html_viewer/html_frame_delegate.h"
+#include "components/html_viewer/html_frame_tree_manager_observer.h"
 #include "components/view_manager/public/cpp/view_tree_connection.h"
 #include "components/web_view/web_view_switches.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
@@ -43,6 +44,24 @@ bool FindFrameDataIndex(const mojo::Array<web_view::FrameDataPtr>& frame_data,
 
 }  // namespace
 
+// Object that calls OnHTMLFrameTreeManagerChangeIdAdvanced() from the
+// destructor.
+class HTMLFrameTreeManager::ChangeIdAdvancedNotifier {
+ public:
+  explicit ChangeIdAdvancedNotifier(
+      base::ObserverList<HTMLFrameTreeManagerObserver>* observers)
+      : observers_(observers) {}
+  ~ChangeIdAdvancedNotifier() {
+    FOR_EACH_OBSERVER(HTMLFrameTreeManagerObserver, *observers_,
+                      OnHTMLFrameTreeManagerChangeIdAdvanced());
+  }
+
+ private:
+  base::ObserverList<HTMLFrameTreeManagerObserver>* observers_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChangeIdAdvancedNotifier);
+};
+
 // static
 HTMLFrameTreeManager::TreeMap* HTMLFrameTreeManager::instances_ = nullptr;
 
@@ -69,13 +88,8 @@ HTMLFrame* HTMLFrameTreeManager::CreateFrameAndAttachToTree(
 
   on_connect_callback.Run();
 
-  HTMLFrameTreeManager* frame_tree = nullptr;
-
-  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
-          web_view::switches::kOOPIFAlwaysCreateNewFrameTree) &&
-      instances_->count(frame_data[0]->frame_id)) {
-    frame_tree = (*instances_)[frame_data[0]->frame_id];
-  }
+  HTMLFrameTreeManager* frame_tree =
+      FindFrameTreeWithRoot(frame_data[0]->frame_id);
 
   if (view_connect_type == web_view::VIEW_CONNECT_TYPE_USE_EXISTING &&
       !frame_tree) {
@@ -140,10 +154,29 @@ HTMLFrame* HTMLFrameTreeManager::CreateFrameAndAttachToTree(
   return frame;
 }
 
+// static
+HTMLFrameTreeManager* HTMLFrameTreeManager::FindFrameTreeWithRoot(
+    uint32_t root_frame_id) {
+  return (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+              web_view::switches::kOOPIFAlwaysCreateNewFrameTree) &&
+          instances_ && instances_->count(root_frame_id))
+             ? (*instances_)[root_frame_id]
+             : nullptr;
+}
+
 blink::WebView* HTMLFrameTreeManager::GetWebView() {
   return root_->web_view();
 }
 
+void HTMLFrameTreeManager::AddObserver(HTMLFrameTreeManagerObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void HTMLFrameTreeManager::RemoveObserver(
+    HTMLFrameTreeManagerObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 void HTMLFrameTreeManager::OnFrameDestroyed(HTMLFrame* frame) {
   if (frame == root_)
     root_ = nullptr;
@@ -169,6 +202,9 @@ HTMLFrameTreeManager::HTMLFrameTreeManager(GlobalState* global_state)
 HTMLFrameTreeManager::~HTMLFrameTreeManager() {
   DCHECK(!root_ || !local_root_);
   RemoveFromInstances();
+
+  FOR_EACH_OBSERVER(HTMLFrameTreeManagerObserver, observers_,
+                    OnHTMLFrameTreeManagerDestroyed());
 }
 
 void HTMLFrameTreeManager::Init(
@@ -249,6 +285,8 @@ void HTMLFrameTreeManager::ProcessOnFrameAdded(
   if (!PrepareForStructureChange(source, change_id))
     return;
 
+  ChangeIdAdvancedNotifier notifier(&observers_);
+
   HTMLFrame* parent = root_->FindFrame(frame_data->parent_id);
   if (!parent) {
     DVLOG(1) << "Received invalid parent in OnFrameAdded "
@@ -279,6 +317,8 @@ void HTMLFrameTreeManager::ProcessOnFrameRemoved(HTMLFrame* source,
   if (!PrepareForStructureChange(source, change_id))
     return;
 
+  ChangeIdAdvancedNotifier notifier(&observers_);
+
   pending_remove_ids_.erase(frame_id);
 
   HTMLFrame* frame = root_->FindFrame(frame_id);