Consistent content placement method for dom-distiller

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 a3ae496db86455a07c886dfe45e3b366ece85493..938366a8be1ba44e22f385c883e5f1b4899a9703 100644
--- a/components/dom_distiller/content/dom_distiller_viewer_source.cc
+++ b/components/dom_distiller/content/dom_distiller_viewer_source.cc
@@ -25,9 +25,40 @@
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_contents_user_data.h"
 #include "net/base/url_util.h"
 #include "net/url_request/url_request.h"
 
+namespace {
+
+// Handles sending conent to pages that are in an error state (i.e. the URL
+// was bad or could not be loaded).
+class WebContentsErrorObserver

[cjhopman, 2015/03/21 02:33:22]

If distillation returned really really fast, could a successful distillation run
into the same case of calling javascript too early?

[mdjones, 2015/03/24 01:32:42]

Yes, I have added a second observer to watch for the template to be loaded on
the page. This places the first page of content then stops observing.

+    : public content::WebContentsObserver,
+      public content::WebContentsUserData<WebContentsErrorObserver> {
+ public:
+  // Once the page has finished loading, send an error message.
+  void DocumentOnLoadCompletedInMainFrame() override {
+    web_contents()->GetMainFrame()->ExecuteJavaScript(
+        base::UTF8ToUTF16(dom_distiller::viewer::GetErrorPageJs()));
+  }
+
+ private:
+  friend class content::WebContentsUserData<WebContentsErrorObserver>;
+  explicit WebContentsErrorObserver(content::WebContents* web_contents)
+      : WebContentsObserver(web_contents) {}
+
+  ~WebContentsErrorObserver() override {
+    content::WebContentsObserver::Observe(NULL);
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(WebContentsErrorObserver);
+};
+
+}  // namespace
+
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(WebContentsErrorObserver);
+
 namespace dom_distiller {
 
 // Handles receiving data asynchronously for a specific entry, and passing
@@ -188,13 +219,14 @@ void DomDistillerViewerSource::RequestViewerHandle::DidFinishLoad(
 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 the actual article content to the page.
+    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));
@@ -221,15 +253,14 @@ void DomDistillerViewerSource::RequestViewerHandle::OnArticleUpdated(
         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 actual article content to the page.
+    SendJavaScript(viewer::GetUnsafeIncrementalDistilledPageJs(&page, false));
   }
 }
 
@@ -314,6 +345,8 @@ void DomDistillerViewerSource::StartDataRequest(
     // |RequestViewerHandle| will never be called, so clean up now.
     delete request_viewer_handle;
 
+    WebContentsErrorObserver::CreateForWebContents(web_contents);
+
     std::string error_page_html = viewer::GetErrorPageHtml(
         dom_distiller_service_->GetDistilledPagePrefs()->GetTheme(),
         dom_distiller_service_->GetDistilledPagePrefs()->GetFontFamily());