Fuchsia: Always attach WebContentsObserver to a Frame's WebContents.

Currently, a Frame doesn't process WebContentsObserver methods until
a FIDL NavigationEventObserver is registered. WebContentsObserver is
useful for more than just navigation events, though! In cases where a
Frame is created but never passed a N.E.O., observer events like
ReadyToCommitNavigation() were not being received, which interfered
with on-load script injection.

This CL fixes the issue by attaching the observer at Frame
creation time.

Bug: 903880
Change-Id: Ie3ab194fbaa60a5e18a4150be3ce4c6f7345845f
Reviewed-on: https://chromium-review.googlesource.com/c/1328147
Commit-Queue: Kevin Marshall <kmarshall@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606997}

webrunner/browser/frame_impl.cc

@@ -160,6 +160,7 @@ FrameImpl::FrameImpl(std::unique_ptr<content::WebContents> web_contents,
       context_(context),
       binding_(this, std::move(frame_request)) {
   web_contents_->SetDelegate(this);
+  Observe(web_contents_.get());
   binding_.set_error_handler([this]() { context_->DestroyFrame(this); });
 }
 
@@ -317,13 +318,10 @@ void FrameImpl::SetNavigationEventObserver(
   if (observer) {
     navigation_observer_.Bind(std::move(observer));
     navigation_observer_.set_error_handler([this]() {
-      // Stop observing on Observer connection loss.
       SetNavigationEventObserver(nullptr);
     });
-    Observe(web_contents_.get());
   } else {
     navigation_observer_.Unbind();
-    Observe(nullptr);  // Stop receiving WebContentsObserver events.
   }
 }
 
@@ -383,27 +381,28 @@ void FrameImpl::DidFinishLoad(content::RenderFrameHost* render_frame_host,
                              &pending_navigation_event_);
   cached_navigation_state_ = std::move(current_navigation_state);
 
-  if (pending_navigation_event_is_dirty_ &&
-      !waiting_for_navigation_event_ack_) {
   MaybeSendNavigationEvent();
-  }
 }
 
 void FrameImpl::MaybeSendNavigationEvent() {
-  if (pending_navigation_event_is_dirty_) {
+  if (!navigation_observer_)
+    return;
+
+  if (!pending_navigation_event_is_dirty_ ||
+      waiting_for_navigation_event_ack_) {
+    return;
+  }
+
   pending_navigation_event_is_dirty_ = false;
   waiting_for_navigation_event_ack_ = true;
 
   // Send the event to the observer and, upon acknowledgement, revisit this
   // function to send another update.
   navigation_observer_->OnNavigationStateChanged(
-        std::move(pending_navigation_event_),
-        [this]() { MaybeSendNavigationEvent(); });
-    return;
-  } else {
-    // No more changes to report.
+      std::move(pending_navigation_event_), [this]() {
         waiting_for_navigation_event_ack_ = false;
-  }
+        MaybeSendNavigationEvent();
+      });
 }
 
 void FrameImpl::ReadyToCommitNavigation(

webrunner/browser/frame_impl.h

@@ -118,8 +118,6 @@ class FrameImpl : public chromium::web::Frame,
       const GURL& target_url,
       const std::string& partition_id,
       content::SessionStorageNamespace* session_storage_namespace) override;
-  void ReadyToCommitNavigation(
-      content::NavigationHandle* navigation_handle) override;
   bool DidAddMessageToConsole(content::WebContents* source,
                               int32_t level,
                               const base::string16& message,
@@ -129,6 +127,8 @@ class FrameImpl : public chromium::web::Frame,
   // content::WebContentsObserver implementation.
   void DidFinishLoad(content::RenderFrameHost* render_frame_host,
                      const GURL& validated_url) override;
+  void ReadyToCommitNavigation(
+      content::NavigationHandle* navigation_handle) override;
 
   std::unique_ptr<aura::WindowTreeHost> window_tree_host_;
   std::unique_ptr<content::WebContents> web_contents_;