Convert remaining WebContentsObservers loading callbacks to use RFH.

Fix several WebContentsObserver implementations that are incorrectly
caching only frame routing IDs in order to act only on that frame in
later callbacks. Frame routing IDs by themselves are not suitable for
identity comparisons, since they are only unique within a given
RenderProcessHost.

Also clean up WebContentsObservers with a redundant WebContents
field. By definition, a WebContentsObserver must know which
WebContents it's observing, so there's no need for derived classes
to maintain their own WebContents field.

BUG=none

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282043 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 27 insertions, 75 deletions

diff --git a/components/dom_distiller/content/distiller_page_web_contents.cc b/components/dom_distiller/content/distiller_page_web_contents.cc
index 527c0eb..41187eb 100644
--- a/components/dom_distiller/content/distiller_page_web_contents.cc
+++ b/components/dom_distiller/content/distiller_page_web_contents.cc
@@ -108,21 +108,18 @@ void DistillerPageWebContents::CreateNewWebContents(const GURL& url) {
 }
 
 void DistillerPageWebContents::DocumentLoadedInFrame(
-    int64 frame_id,
-    RenderViewHost* render_view_host) {
-  if (frame_id == web_contents_->GetMainFrame()->GetRoutingID()) {
+    content::RenderFrameHost* render_frame_host) {
+  if (render_frame_host == web_contents_->GetMainFrame()) {
     ExecuteJavaScript();
   }
 }
 
 void DistillerPageWebContents::DidFailLoad(
-    int64 frame_id,
+    content::RenderFrameHost* render_frame_host,
     const GURL& validated_url,
-    bool is_main_frame,
     int error_code,
-    const base::string16& error_description,
-    RenderViewHost* render_view_host) {
-  if (is_main_frame) {
+    const base::string16& error_description) {
+  if (!render_frame_host->GetParent()) {
     content::WebContentsObserver::Observe(NULL);
     DCHECK(state_ == LOADING_PAGE || state_ == EXECUTING_JAVASCRIPT);
     state_ = PAGELOAD_FAILED;
diff --git a/components/dom_distiller/content/distiller_page_web_contents.h b/components/dom_distiller/content/distiller_page_web_contents.h
index aed9789..d738854 100644
--- a/components/dom_distiller/content/distiller_page_web_contents.h
+++ b/components/dom_distiller/content/distiller_page_web_contents.h
@@ -13,12 +13,6 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "url/gurl.h"
 
-namespace content {
-class RenderViewHost;
-}
-
-using content::RenderViewHost;
-
 namespace dom_distiller {
 
 class SourcePageHandleWebContents : public SourcePageHandle {
@@ -58,15 +52,13 @@ class DistillerPageWebContents : public DistillerPage,
   virtual ~DistillerPageWebContents();
 
   // content::WebContentsObserver implementation.
-  virtual void DocumentLoadedInFrame(int64 frame_id,
-                                     RenderViewHost* render_view_host) OVERRIDE;
+  virtual void DocumentLoadedInFrame(
+      content::RenderFrameHost* render_frame_host) OVERRIDE;
 
-  virtual void DidFailLoad(int64 frame_id,
+  virtual void DidFailLoad(content::RenderFrameHost* render_frame_host,
                            const GURL& validated_url,
-                           bool is_main_frame,
                            int error_code,
-                           const base::string16& error_description,
-                           RenderViewHost* render_view_host) OVERRIDE;
+                           const base::string16& error_description) OVERRIDE;
 
  protected:
   virtual void DistillPageImpl(const GURL& url,
diff --git a/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc b/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc
index 884a751..7e7aa02 100644
--- a/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc
+++ b/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc
@@ -131,11 +131,9 @@ class WebContentsMainFrameHelper : public content::WebContentsObserver {
   WebContentsMainFrameHelper(content::WebContents* web_contents,
                              const base::Closure& callback,
                              bool wait_for_document_loaded)
-      : web_contents_(web_contents),
+      : WebContentsObserver(web_contents),
         callback_(callback),
-        wait_for_document_loaded_(wait_for_document_loaded) {
-    content::WebContentsObserver::Observe(web_contents);
-  }
+        wait_for_document_loaded_(wait_for_document_loaded) {}
 
   virtual void DidCommitProvisionalLoadForFrame(
       content::RenderFrameHost* render_frame_host,
@@ -148,18 +146,14 @@ class WebContentsMainFrameHelper : public content::WebContentsObserver {
   }
 
   virtual void DocumentLoadedInFrame(
-      int64 frame_id,
-      content::RenderViewHost* render_view_host) OVERRIDE {
+      content::RenderFrameHost* render_frame_host) OVERRIDE {
     if (wait_for_document_loaded_) {
-      if (web_contents_ &&
-          frame_id == web_contents_->GetMainFrame()->GetRoutingID()) {
+      if (!render_frame_host->GetParent())
         callback_.Run();
-      }
     }
   }
 
  private:
-  content::WebContents* web_contents_;
   base::Closure callback_;
   bool wait_for_document_loaded_;
 };
diff --git a/components/dom_distiller/content/dom_distiller_viewer_source.cc b/components/dom_distiller/content/dom_distiller_viewer_source.cc
index 26c6502..8f475cc 100644
--- a/components/dom_distiller/content/dom_distiller_viewer_source.cc
+++ b/components/dom_distiller/content/dom_distiller_viewer_source.cc
@@ -55,11 +55,8 @@ class DomDistillerViewerSource::RequestViewerHandle
       const content::FrameNavigateParams& params) OVERRIDE;
   virtual void RenderProcessGone(base::TerminationStatus status) OVERRIDE;
   virtual void WebContentsDestroyed() OVERRIDE;
-  virtual void DidFinishLoad(
-      int64 frame_id,
-      const GURL& validated_url,
-      bool is_main_frame,
-      content::RenderViewHost* render_view_host) OVERRIDE;
+  virtual void DidFinishLoad(content::RenderFrameHost* render_frame_host,
+                             const GURL& validated_url) OVERRIDE;
 
  private:
   // Sends JavaScript to the attached Viewer, buffering data if the viewer isn't
@@ -75,9 +72,6 @@ class DomDistillerViewerSource::RequestViewerHandle
   // needs to be kept around to ensure the distillation request finishes.
   scoped_ptr<ViewerHandle> viewer_handle_;
 
-  // WebContents associated with the Viewer's render process.
-  content::WebContents* web_contents_;
-
   // The scheme hosting the current view request;
   std::string expected_scheme_;
 
@@ -105,18 +99,15 @@ DomDistillerViewerSource::RequestViewerHandle::RequestViewerHandle(
     const std::string& expected_scheme,
     const std::string& expected_request_path,
     const content::URLDataSource::GotDataCallback& callback)
-    : web_contents_(web_contents),
-      expected_scheme_(expected_scheme),
+    : expected_scheme_(expected_scheme),
       expected_request_path_(expected_request_path),
       callback_(callback),
       page_count_(0),
       waiting_for_page_ready_(true) {
-  content::WebContentsObserver::Observe(web_contents_);
+  content::WebContentsObserver::Observe(web_contents);
 }
 
 DomDistillerViewerSource::RequestViewerHandle::~RequestViewerHandle() {
-  // Balanced with constructor although can be a no-op if frame navigated away.
-  content::WebContentsObserver::Observe(NULL);
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::SendJavaScript(
@@ -124,8 +115,8 @@ void DomDistillerViewerSource::RequestViewerHandle::SendJavaScript(
   if (waiting_for_page_ready_) {
     buffer_ += buffer;
   } else {
-    if (web_contents_) {
-      web_contents_->GetMainFrame()->ExecuteJavaScript(
+    if (web_contents()) {
+      web_contents()->GetMainFrame()->ExecuteJavaScript(
           base::UTF8ToUTF16(buffer));
     }
   }
@@ -156,9 +147,6 @@ void DomDistillerViewerSource::RequestViewerHandle::WebContentsDestroyed() {
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::Cancel() {
-  // Ensure we don't send any incremental updates to the Viewer.
-  web_contents_ = NULL;
-
   // No need to listen for notifications.
   content::WebContentsObserver::Observe(NULL);
 
@@ -168,21 +156,16 @@ void DomDistillerViewerSource::RequestViewerHandle::Cancel() {
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::DidFinishLoad(
-     int64 frame_id,
-     const GURL& validated_url,
-     bool is_main_frame,
-     content::RenderViewHost* render_view_host)  {
-  if (!is_main_frame || web_contents_ == NULL) {
+    content::RenderFrameHost* render_frame_host,
+    const GURL& validated_url) {
+  if (render_frame_host->GetParent()) {
     return;
   }
   waiting_for_page_ready_ = false;
   if (buffer_.empty()) {
     return;
   }
-  if (web_contents_) {
-    web_contents_->GetMainFrame()->ExecuteJavaScript(
-        base::UTF8ToUTF16(buffer_));
-  }
+  web_contents()->GetMainFrame()->ExecuteJavaScript(base::UTF8ToUTF16(buffer_));
   buffer_.clear();
 }
 
diff --git a/components/dom_distiller/content/web_contents_main_frame_observer.cc b/components/dom_distiller/content/web_contents_main_frame_observer.cc
index ed1bbab..92ea4b9c 100644
--- a/components/dom_distiller/content/web_contents_main_frame_observer.cc
+++ b/components/dom_distiller/content/web_contents_main_frame_observer.cc
@@ -16,9 +16,7 @@ namespace dom_distiller {
 
 WebContentsMainFrameObserver::WebContentsMainFrameObserver(
     content::WebContents* web_contents)
-    : is_document_loaded_in_main_frame_(false),
-      is_initialized_(false),
-      web_contents_(web_contents) {
+    : is_document_loaded_in_main_frame_(false), is_initialized_(false) {
   content::WebContentsObserver::Observe(web_contents);
 }
 
@@ -27,10 +25,8 @@ WebContentsMainFrameObserver::~WebContentsMainFrameObserver() {
 }
 
 void WebContentsMainFrameObserver::DocumentLoadedInFrame(
-    int64 frame_id,
-    content::RenderViewHost* render_view_host) {
-  if (web_contents_ &&
-      frame_id == web_contents_->GetMainFrame()->GetRoutingID()) {
+    content::RenderFrameHost* render_frame_host) {
+  if (!render_frame_host->GetParent()) {
     is_document_loaded_in_main_frame_ = true;
   }
 }
@@ -49,13 +45,8 @@ void WebContentsMainFrameObserver::RenderProcessGone(
   CleanUp();
 }
 
-void WebContentsMainFrameObserver::WebContentsDestroyed() {
-  CleanUp();
-}
-
 void WebContentsMainFrameObserver::CleanUp() {
   content::WebContentsObserver::Observe(NULL);
-  web_contents_ = NULL;
 }
 
 }  // namespace dom_distiller
diff --git a/components/dom_distiller/content/web_contents_main_frame_observer.h b/components/dom_distiller/content/web_contents_main_frame_observer.h
index 7343dba..ff24452 100644
--- a/components/dom_distiller/content/web_contents_main_frame_observer.h
+++ b/components/dom_distiller/content/web_contents_main_frame_observer.h
@@ -28,13 +28,11 @@ class WebContentsMainFrameObserver
 
   // content::WebContentsObserver implementation.
   virtual void DocumentLoadedInFrame(
-      int64 frame_id,
-      content::RenderViewHost* render_view_host) OVERRIDE;
+      content::RenderFrameHost* render_frame_host) OVERRIDE;
   virtual void DidNavigateMainFrame(
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) OVERRIDE;
   virtual void RenderProcessGone(base::TerminationStatus status) OVERRIDE;
-  virtual void WebContentsDestroyed() OVERRIDE;
 
  private:
   explicit WebContentsMainFrameObserver(content::WebContents* web_contents);
@@ -52,9 +50,6 @@ class WebContentsMainFrameObserver
   // at least one call to DidNavigateMainFrame has happened.
   bool is_initialized_;
 
-  // The WebContents this class is tracking.
-  content::WebContents* web_contents_;
-
   DISALLOW_COPY_AND_ASSIGN(WebContentsMainFrameObserver);
 };