[dom_distiller] Add support for incremental viewer.

components/dom_distiller/content/dom_distiller_viewer_source.cc

Index: components/dom_distiller/content/dom_distiller_viewer_source.cc
diff --git a/components/dom_distiller/content/dom_distiller_viewer_source.cc b/components/dom_distiller/content/dom_distiller_viewer_source.cc
index c3351c2cc5383870384c95f89af27b1d1cd3c977..b14d40d0d4f801ae02284b8b34555fa557e9e66b 100644
--- a/components/dom_distiller/content/dom_distiller_viewer_source.cc
+++ b/components/dom_distiller/content/dom_distiller_viewer_source.cc
@@ -11,21 +11,28 @@
 #include "base/memory/ref_counted_memory.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
+#include "base/strings/utf_string_conversions.h"
 #include "components/dom_distiller/core/task_tracker.h"
 #include "components/dom_distiller/core/url_constants.h"
 #include "components/dom_distiller/core/viewer.h"
+#include "content/public/browser/navigation_details.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_view_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "net/url_request/url_request.h"
 
 namespace dom_distiller {
 
 // Handles receiving data asynchronously for a specific entry, and passing
-// it along to the data callback for the data source.
+// it along to the data callback for the data source. Lifetime matches that of
+// the RenderFrameHost for the Viewer instance.

[Charlie Reis, 2014/05/12 22:12:52]

nit: Lifetime matches that of the current main frame's page in the Viewer
instance.

[Yaron, 2014/05/14 17:42:33]

Done.

 class DomDistillerViewerSource::RequestViewerHandle
-    : public ViewRequestDelegate {
+    : public ViewRequestDelegate,
+      public content::WebContentsObserver {
  public:
   explicit RequestViewerHandle(
+      content::WebContents* web_contents,
       const content::URLDataSource::GotDataCallback& callback);
   virtual ~RequestViewerHandle();
 
@@ -38,33 +45,166 @@ class DomDistillerViewerSource::RequestViewerHandle
 
   void TakeViewerHandle(scoped_ptr<ViewerHandle> viewer_handle);
 
+  // WebContentsObserver:
+  virtual void DidNavigateMainFrame(
+      const content::LoadCommittedDetails& details,
+      const content::FrameNavigateParams& params) OVERRIDE;
+  virtual void RenderProcessGone(base::TerminationStatus status) OVERRIDE;
+  virtual void WebContentsDestroyed(content::WebContents* web_contents)
+       OVERRIDE;
+  virtual void DidFinishLoad(
+      int64 frame_id,
+      const GURL& validated_url,
+      bool is_main_frame,
+      content::RenderViewHost* render_view_host) OVERRIDE;
+
  private:
+  // Sends JavaScript to the attached Viewer, buffering data if the viewer isn't
+  // ready.
+  void SendJavaScript(const std::string& buffer);
+
+  // Cancels the current view request. Once called, no updates will be
+  // propagated to the view, and the request to DomDistillerService will be
+  // cancelled.
+  void Cancel();
+
   // The handle to the view request towards the DomDistillerService. It
   // needs to be kept around to ensure the distillation request finishes.
   scoped_ptr<ViewerHandle> viewer_handle_;
 
-  // This holds the callback to where the data retrieved is sent back.
+  // WebContents associated with the Viewer's render process.
+  content::WebContents* web_contents_;
+
+  // Holds the callback to where the data retrieved is sent back.
   content::URLDataSource::GotDataCallback callback_;
+
+  // Number of pages of the distilled article content that have been rendered by
+  // the viewer.
+  int page_count_;
+
+  // Whether the page is sufficiently initialized to handle updates from the
+  // distiller.
+  bool waiting_for_page_ready_;
+
+  // Temporary store of pending JavaScript if the page isn't ready to receive
+  // data from distillation.
+  std::string buffer_;
 };
 
 DomDistillerViewerSource::RequestViewerHandle::RequestViewerHandle(
+    content::WebContents* web_contents,
     const content::URLDataSource::GotDataCallback& callback)
-    : callback_(callback) {
+    : web_contents_(web_contents),
+      callback_(callback),
+      page_count_(0),
+      waiting_for_page_ready_(true) {
+  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(
+    const std::string& buffer) {
+  if (waiting_for_page_ready_) {
+    buffer_ += buffer;
+  } else {
+    if (web_contents_) {
+      web_contents_->GetMainFrame()->ExecuteJavaScript(
+          base::UTF8ToUTF16(buffer));
+    }
+  }
+}
+
+void DomDistillerViewerSource::RequestViewerHandle::DidNavigateMainFrame(
+    const content::LoadCommittedDetails& details,
+    const content::FrameNavigateParams& params) {
+  if (!details.is_in_page) {
+    Cancel();
+  }
+}
+
+void DomDistillerViewerSource::RequestViewerHandle::RenderProcessGone(
+    base::TerminationStatus status) {
+  Cancel();
+}
+
+void DomDistillerViewerSource::RequestViewerHandle::WebContentsDestroyed(
+    content::WebContents* web_contents) {
+  Cancel();
+}
+
+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);
+
+  // Schedule the Viewer for deletion. Ensures distillation is cancelled, and
+  // any pending data stored in |buffer_| is released.
+  base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
+}
+
+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) {
+    return;
+  }
+  waiting_for_page_ready_ = false;
+  if (buffer_.empty()) {
+    return;
+  }
+  if (web_contents_) {
+    web_contents_->GetMainFrame()->ExecuteJavaScript(
+        base::UTF8ToUTF16(buffer_));
+  }
+  buffer_.clear();
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::OnArticleReady(
     const DistilledArticleProto* article_proto) {
-  std::string unsafe_page_html = viewer::GetUnsafeHtml(article_proto);
-  callback_.Run(base::RefCountedString::TakeString(&unsafe_page_html));
-  base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
+  if (page_count_ == 0) {
+    // This is a single-page article.
+    std::string unsafe_page_html = viewer::GetUnsafeArticleHtml(article_proto);
+    callback_.Run(base::RefCountedString::TakeString(&unsafe_page_html));
+  } else if (page_count_ == article_proto->pages_size()) {
+    // We may still be showing the "Loading" indicator.
+    SendJavaScript(viewer::GetToggleLoadingIndicatorJs(true));
+  } else {
+    // It's possible that we didn't get some incremental updates from the
+    // distiller. Ensure all remaining pages are flushed to the viewer.
+    for (;page_count_ < article_proto->pages_size(); page_count_++) {
+      const DistilledPageProto& page = article_proto->pages(page_count_);
+      SendJavaScript(
+          viewer::GetUnsafeIncrementalDistilledPageJs(
+              &page,
+              page_count_ == article_proto->pages_size()));
+    }
+  }
+  // No need to hold on to the ViewerHandle now that distillation is complete.
+  viewer_handle_.reset();
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::OnArticleUpdated(
     ArticleDistillationUpdate article_update) {
-  // TODO(nyquist): Add support for displaying pages incrementally.
+  for (;page_count_ < static_cast<int>(article_update.GetPagesSize());
+       page_count_++) {
+    const DistilledPageProto& page =
+        article_update.GetDistilledPage(page_count_);
+    if (page_count_ == 0) {

[nyquist, 2014/05/13 05:01:15]

Nit: Add a comment like:
// This is the first page, so send web page scaffolding too.

[Yaron, 2014/05/14 17:42:33]

Done.

+      std::string unsafe_page_html = viewer::GetUnsafePartialArticleHtml(&page);
+      callback_.Run(base::RefCountedString::TakeString(&unsafe_page_html));
+    } else {
+      SendJavaScript(
+          viewer::GetUnsafeIncrementalDistilledPageJs(&page, false));
+    }
+  }
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::TakeViewerHandle(
@@ -98,14 +238,23 @@ void DomDistillerViewerSource::StartDataRequest(
   DCHECK(render_view_host);
   CHECK_EQ(0, render_view_host->GetEnabledBindings());
 
-  if (kCssPath == path) {
+  if (kViewerCssPath == path) {
     std::string css = viewer::GetCss();
     callback.Run(base::RefCountedString::TakeString(&css));
     return;
   }
-
+  if (kViewerJsPath == path) {
+    std::string js = viewer::GetJavaScript();
+    callback.Run(base::RefCountedString::TakeString(&js));
+    return;
+  }
+  content::WebContents* web_contents =
+      content::WebContents::FromRenderFrameHost(
+          content::RenderFrameHost::FromID(render_process_id,
+                                           render_frame_id));
+  DCHECK(web_contents);
   RequestViewerHandle* request_viewer_handle =
-      new RequestViewerHandle(callback);
+      new RequestViewerHandle(web_contents, callback);
   scoped_ptr<ViewerHandle> viewer_handle = viewer::CreateViewRequest(
       dom_distiller_service_, path, request_viewer_handle);
 
@@ -127,8 +276,10 @@ void DomDistillerViewerSource::StartDataRequest(
 
 std::string DomDistillerViewerSource::GetMimeType(
     const std::string& path) const {
-  if (path == kCssPath)
+  if (kViewerCssPath == path)

[nyquist, 2014/05/13 05:01:15]

Nit: Following the style of the rest of this CL, add {} in this function maybe?

[Yaron, 2014/05/14 17:42:33]

Done.

     return "text/css";
+  else if (kViewerJsPath == path)
+    return "text/javascript";
   return "text/html";
 }