Theme Preferences for Distilled Pages

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 26c650266d3d6f5e0bde7c13e774c7cef8c9cf95..ff0685e68dc4203f5cdab1d3c88601efa75285e5 100644
--- a/components/dom_distiller/content/dom_distiller_viewer_source.cc
+++ b/components/dom_distiller/content/dom_distiller_viewer_source.cc
@@ -12,6 +12,8 @@
 #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/dom_distiller_service.h"
+#include "components/dom_distiller/core/reader_mode_preferences.h"
 #include "components/dom_distiller/core/task_tracker.h"
 #include "components/dom_distiller/core/url_constants.h"
 #include "components/dom_distiller/core/viewer.h"
@@ -31,16 +33,18 @@ namespace dom_distiller {
 // the current main frame's page in the Viewer instance.
 class DomDistillerViewerSource::RequestViewerHandle
     : public ViewRequestDelegate,
-      public content::WebContentsObserver {
+      public content::WebContentsObserver,
+      public ReaderModePrefs::Observer {
  public:
   explicit RequestViewerHandle(
       content::WebContents* web_contents,
       const std::string& expected_scheme,
       const std::string& expected_request_path,
-      const content::URLDataSource::GotDataCallback& callback);
+      const content::URLDataSource::GotDataCallback& callback,
+      ReaderModePrefs* reader_mode_prefs);
   virtual ~RequestViewerHandle();
 
-  // ViewRequestDelegate implementation.
+  // ViewRequestDelegate implementation:
   virtual void OnArticleReady(
       const DistilledArticleProto* article_proto) OVERRIDE;
 
@@ -49,7 +53,7 @@ class DomDistillerViewerSource::RequestViewerHandle
 
   void TakeViewerHandle(scoped_ptr<ViewerHandle> viewer_handle);
 
-  // WebContentsObserver:
+  // content::WebContentsObserver implementation:
   virtual void DidNavigateMainFrame(
       const content::LoadCommittedDetails& details,
       const content::FrameNavigateParams& params) OVERRIDE;
@@ -61,6 +65,9 @@ class DomDistillerViewerSource::RequestViewerHandle
       bool is_main_frame,
       content::RenderViewHost* render_view_host) OVERRIDE;
 
+  // Needed by DomDistillerViewerSource.
+  const ReaderModePrefs* GetReaderModePrefs();

[nyquist, 2014/06/27 20:48:26]

Remove.

[smaslo, 2014/06/30 17:57:14]

It removed it, but I had to make the reader_mode_prefs_ variable public so that
the outer class DomDistillerViewerSource has access to it.

+
  private:
   // Sends JavaScript to the attached Viewer, buffering data if the viewer isn't
   // ready.
@@ -71,6 +78,12 @@ class DomDistillerViewerSource::RequestViewerHandle
   // cancelled.
   void Cancel();
 
+  // ReaderModePrefs::Observer implementation:
+  virtual void OnChangeTheme(ReaderModePrefs::Theme new_theme) OVERRIDE;
+
+  // Sets CSS class name for the webpage's HTML body element.

[nyquist, 2014/06/27 20:48:26]

Remove

[smaslo, 2014/06/30 17:57:14]

Done.

+  void SetBodyClassName();
+
   // 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_;
@@ -98,25 +111,32 @@ class DomDistillerViewerSource::RequestViewerHandle
   // Temporary store of pending JavaScript if the page isn't ready to receive
   // data from distillation.
   std::string buffer_;
+
+  // Interface for accessing preferences for reader mode.
+  ReaderModePrefs* reader_mode_prefs_;
 };
 
 DomDistillerViewerSource::RequestViewerHandle::RequestViewerHandle(
     content::WebContents* web_contents,
     const std::string& expected_scheme,
     const std::string& expected_request_path,
-    const content::URLDataSource::GotDataCallback& callback)
+    const content::URLDataSource::GotDataCallback& callback,
+    ReaderModePrefs* reader_mode_prefs)
     : web_contents_(web_contents),
       expected_scheme_(expected_scheme),
       expected_request_path_(expected_request_path),
       callback_(callback),
       page_count_(0),
-      waiting_for_page_ready_(true) {
+      waiting_for_page_ready_(true),
+      reader_mode_prefs_(reader_mode_prefs) {
   content::WebContentsObserver::Observe(web_contents_);
+  reader_mode_prefs_->AddObserver(this);
 }
 
 DomDistillerViewerSource::RequestViewerHandle::~RequestViewerHandle() {
   // Balanced with constructor although can be a no-op if frame navigated away.
   content::WebContentsObserver::Observe(NULL);
+  reader_mode_prefs_->RemoveObserver(this);
 }
 
 void DomDistillerViewerSource::RequestViewerHandle::SendJavaScript(
@@ -190,7 +210,8 @@ 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);
+    std::string unsafe_page_html = viewer::GetUnsafeArticleHtml(article_proto,
+        reader_mode_prefs_->GetThemePref());
     callback_.Run(base::RefCountedString::TakeString(&unsafe_page_html));
   } else if (page_count_ == article_proto->pages_size()) {
     // We may still be showing the "Loading" indicator.
@@ -218,7 +239,8 @@ 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(&page);
+      std::string unsafe_page_html = viewer::GetUnsafePartialArticleHtml(&page,
+          reader_mode_prefs_->GetThemePref());
       callback_.Run(base::RefCountedString::TakeString(&unsafe_page_html));
     } else {
       SendJavaScript(
@@ -232,6 +254,23 @@ void DomDistillerViewerSource::RequestViewerHandle::TakeViewerHandle(
   viewer_handle_ = viewer_handle.Pass();
 }
 
+void DomDistillerViewerSource::RequestViewerHandle::OnChangeTheme(
+    ReaderModePrefs::Theme theme) {

[nyquist, 2014/06/27 20:48:26]

Nit: new_theme

[smaslo, 2014/06/30 17:57:14]

Done.

+  SetBodyClassName();

[nyquist, 2014/06/27 20:48:26]

This is the only place that method is used, so just inline it here.

[smaslo, 2014/06/30 17:57:14]

Done.

+}
+
+void DomDistillerViewerSource::RequestViewerHandle::SetBodyClassName() {

[nyquist, 2014/06/27 20:48:26]

Require theme as argument:
ReaderModePrefs::Theme theme
Then use that in the string stream

[smaslo, 2014/06/30 17:57:14]

Done.

+  std::ostringstream js;
+  js << "document.body.className='" <<
+    viewer::GetBodyCssClass(reader_mode_prefs_->GetThemePref()) << "';";
+  SendJavaScript(js.str());
+}
+
+const ReaderModePrefs* DomDistillerViewerSource::RequestViewerHandle::

[nyquist, 2014/06/27 20:48:26]

Remove.

[smaslo, 2014/06/30 17:57:14]

Done.

+    GetReaderModePrefs() {
+  return reader_mode_prefs_;
+}
+
 DomDistillerViewerSource::DomDistillerViewerSource(
     DomDistillerServiceInterface* dom_distiller_service,
     const std::string& scheme)
@@ -278,7 +317,8 @@ void DomDistillerViewerSource::StartDataRequest(
   const std::string path_after_query_separator =
       path.size() > 0 ? path.substr(1) : "";
   RequestViewerHandle* request_viewer_handle = new RequestViewerHandle(
-      web_contents, scheme_, path_after_query_separator, callback);
+      web_contents, scheme_, path_after_query_separator, callback,
+      dom_distiller_service_->GetReaderModePrefs());
   scoped_ptr<ViewerHandle> viewer_handle = viewer::CreateViewRequest(
       dom_distiller_service_, path, request_viewer_handle);
 
@@ -289,11 +329,11 @@ void DomDistillerViewerSource::StartDataRequest(
     // after receiving the callback.
     request_viewer_handle->TakeViewerHandle(viewer_handle.Pass());
   } else {
+    std::string error_page_html = viewer::GetErrorPageHtml(
+        request_viewer_handle->GetReaderModePrefs()->GetThemePref());
     // The service did not return a |ViewerHandle|, which means the

[nyquist, 2014/06/27 20:48:26]

Just get it from the service like you do above. That way you don't have to add
the accessor method.
Then move this code down to where it was before. 

[smaslo, 2014/06/30 17:57:14]

Done.

     // |RequestViewerHandle| will never be called, so clean up now.
     delete request_viewer_handle;
-
-    std::string error_page_html = viewer::GetErrorPageHtml();
     callback.Run(base::RefCountedString::TakeString(&error_page_html));
   }
 };