Consistent content placement method for dom-distiller

components/dom_distiller/content/dom_distiller_viewer_source.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/dom_distiller/content/dom_distiller_viewer_source.h"
 
 #include <sstream>
 #include <string>
 #include <vector>
 
@@ skipping 28 matching lines @@
       public DistilledPagePrefs::Observer {
  public:
   explicit RequestViewerHandle(
       content::WebContents* web_contents,
       const std::string& expected_scheme,
       const std::string& expected_request_path,
       const content::URLDataSource::GotDataCallback& callback,
       DistilledPagePrefs* distilled_page_prefs);
   ~RequestViewerHandle() override;
 
+  // Flag this request as an error and send the error page template.
+  void flagAsErrorPage();
+
   // ViewRequestDelegate implementation:
   void OnArticleReady(const DistilledArticleProto* article_proto) override;
 
   void OnArticleUpdated(ArticleDistillationUpdate article_update) override;
 
   void TakeViewerHandle(scoped_ptr<ViewerHandle> viewer_handle);
 
   // content::WebContentsObserver implementation:
   void DidNavigateMainFrame(
       const content::LoadCommittedDetails& details,
@@ skipping 38 matching lines @@
   // Interface for accessing preferences for distilled pages.
   DistilledPagePrefs* distilled_page_prefs_;
 
   // 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_;
+
+  // Flag to tell this observer that the web contents are in an error state.
+  bool is_error_page_;
 };
 
 DomDistillerViewerSource::RequestViewerHandle::RequestViewerHandle(
     content::WebContents* web_contents,
     const std::string& expected_scheme,
     const std::string& expected_request_path,
     const content::URLDataSource::GotDataCallback& callback,
     DistilledPagePrefs* distilled_page_prefs)
     : expected_scheme_(expected_scheme),
       expected_request_path_(expected_request_path),
       callback_(callback),
       page_count_(0),
       distilled_page_prefs_(distilled_page_prefs),
-      waiting_for_page_ready_(true) {
+      waiting_for_page_ready_(true),
+      is_error_page_(false) {
   content::WebContentsObserver::Observe(web_contents);
   distilled_page_prefs_->AddObserver(this);
 }
 
 DomDistillerViewerSource::RequestViewerHandle::~RequestViewerHandle() {
   distilled_page_prefs_->RemoveObserver(this);
 }
 
+void DomDistillerViewerSource::RequestViewerHandle::flagAsErrorPage() {
+  is_error_page_ = true;
+  std::string error_page_html = viewer::GetErrorPageHtml(
+      distilled_page_prefs_->GetTheme(),
+      distilled_page_prefs_->GetFontFamily());
+  callback_.Run(base::RefCountedString::TakeString(&error_page_html));
+}
+
 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) {
   const GURL& navigation = details.entry->GetURL();
   if (details.is_in_page || (
       navigation.SchemeIs(expected_scheme_.c_str()) &&
       expected_request_path_ == navigation.query())) {
     // In-page navigations, as well as the main view request can be ignored.
     return;
   }
 
   Cancel();
-
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::RenderProcessGone(
     base::TerminationStatus status) {
   Cancel();
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::WebContentsDestroyed() {
   Cancel();
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::Cancel() {
   // 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(
     content::RenderFrameHost* render_frame_host,
     const GURL& validated_url) {
+  if (is_error_page_) {
+    waiting_for_page_ready_ = false;
+    SendJavaScript(viewer::GetErrorPageJs());
+    Cancel(); // This will cause the object to clean itself up.
+    return;
+  }
+
   if (render_frame_host->GetParent()) {
     return;
   }
   waiting_for_page_ready_ = false;
   if (buffer_.empty()) {
     return;
   }
   web_contents()->GetMainFrame()->ExecuteJavaScript(base::UTF8ToUTF16(buffer_));
   buffer_.clear();
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::OnArticleReady(
     const DistilledArticleProto* article_proto) {
   if (page_count_ == 0) {
-    // This is a single-page article.
-    std::string unsafe_page_html =
-        viewer::GetUnsafeArticleHtml(
-            article_proto,
-            distilled_page_prefs_->GetTheme(),
-            distilled_page_prefs_->GetFontFamily());
+    std::string unsafe_page_html = viewer::GetUnsafeArticleTemplateHtml(
+        &article_proto->pages(0),
+        distilled_page_prefs_->GetTheme(),
+        distilled_page_prefs_->GetFontFamily());
     callback_.Run(base::RefCountedString::TakeString(&unsafe_page_html));
+    // Send first page to client.
+    SendJavaScript(viewer::GetUnsafeArticleContentJs(article_proto));
   } 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()));
     }
   }

[noyau (Ping after 24h), 2015/04/01 11:42:08]

This logic needs to be moved into a common superclass. Building the html and the
javascript should be completely agnostic of the platform and the call to the
underline content could be abstracted (They already mostly are). All the
javascript buffering should also be moved up.

The error page also can be moved up.

[mdjones, 2015/04/01 22:25:33]

I have marked this as a TODO for myself.

   // No need to hold on to the ViewerHandle now that distillation is complete.
   viewer_handle_.reset();
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::OnArticleUpdated(
     ArticleDistillationUpdate article_update) {
   for (;page_count_ < static_cast<int>(article_update.GetPagesSize());
        page_count_++) {
     const DistilledPageProto& page =
         article_update.GetDistilledPage(page_count_);
     if (page_count_ == 0) {
       // This is the first page, so send Viewer page scaffolding too.
-      std::string unsafe_page_html = viewer::GetUnsafePartialArticleHtml(
+      std::string unsafe_page_html = viewer::GetUnsafeArticleTemplateHtml(
           &page,
           distilled_page_prefs_->GetTheme(),
           distilled_page_prefs_->GetFontFamily());
       callback_.Run(base::RefCountedString::TakeString(&unsafe_page_html));
-    } else {
-      SendJavaScript(
-          viewer::GetUnsafeIncrementalDistilledPageJs(&page, false));
     }
+    // Send the page content to the client.
+    SendJavaScript(viewer::GetUnsafeIncrementalDistilledPageJs(&page, false));
   }
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::TakeViewerHandle(
     scoped_ptr<ViewerHandle> viewer_handle) {
   viewer_handle_ = viewer_handle.Pass();
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::OnChangeTheme(
     DistilledPagePrefs::Theme new_theme) {
@@ skipping 60 matching lines @@
       dom_distiller_service_, path, request_viewer_handle,
       web_contents->GetContainerBounds().size());
 
   if (viewer_handle) {
     // The service returned a |ViewerHandle| and guarantees it will call
     // the |RequestViewerHandle|, so passing ownership to it, to ensure the
     // request is not cancelled. The |RequestViewerHandle| will delete itself
     // after receiving the callback.
     request_viewer_handle->TakeViewerHandle(viewer_handle.Pass());
   } else {
-    // The service did not return a |ViewerHandle|, which means the
-    // |RequestViewerHandle| will never be called, so clean up now.
-    delete request_viewer_handle;
-
-    std::string error_page_html = viewer::GetErrorPageHtml(
-        dom_distiller_service_->GetDistilledPagePrefs()->GetTheme(),
-        dom_distiller_service_->GetDistilledPagePrefs()->GetFontFamily());
-    callback.Run(base::RefCountedString::TakeString(&error_page_html));
+    request_viewer_handle->flagAsErrorPage();
   }
 };
 
 std::string DomDistillerViewerSource::GetMimeType(
     const std::string& path) const {
   if (kViewerCssPath == path) {
     return "text/css";
   }
   if (kViewerJsPath == path) {
     return "text/javascript";
@@ skipping 11 matching lines @@
     const net::URLRequest* request,
     std::string* path) const {
 }
 
 std::string DomDistillerViewerSource::GetContentSecurityPolicyObjectSrc()
     const {
   return "object-src 'none'; style-src 'self' https://fonts.googleapis.com;";
 }
 
 }  // namespace dom_distiller