Theme Preferences for Distilled Pages

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>
 
 #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/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"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.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/base/url_util.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. Lifetime matches that of
 // 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.

[nyquist, 2014/06/26 20:39:57]

// ViewRequestDelegate implementation:

[smaslo, 2014/06/27 18:33:16]

Done.

   virtual void OnArticleReady(
       const DistilledArticleProto* article_proto) OVERRIDE;
 
   virtual void OnArticleUpdated(
       ArticleDistillationUpdate article_update) OVERRIDE;
 
   void TakeViewerHandle(scoped_ptr<ViewerHandle> viewer_handle);
 
   // WebContentsObserver:

[nyquist, 2014/06/26 20:39:57]

// content::WebContentsObserver implementation:

[smaslo, 2014/06/27 18:33:17]

Done.

   virtual void DidNavigateMainFrame(
       const content::LoadCommittedDetails& details,
       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;
 
+  // ReaderModePrefs::Observer

[nyquist, 2014/06/26 20:39:58]

// ReaderModePrefs::Observer implementation:

[smaslo, 2014/06/27 18:33:17]

Done.

+  virtual void OnChangeTheme(ReaderModePrefs* reader_mode_prefs) OVERRIDE;

[nyquist, 2014/06/26 20:39:58]

Also, is it possible to make all of these interface implementations private?

[smaslo, 2014/06/27 18:33:16]

Done.

+
+  void SetBodyClassName();

[nyquist, 2014/06/26 20:39:57]

How about SendBodyClassName()? And could it take the theme as an argument? Also,
could this method be private?

[smaslo, 2014/06/27 18:33:17]

Done.

+
  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();
 
@@ skipping 17 matching lines @@
   // 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_;
+
+  ReaderModePrefs* reader_mode_prefs_;

[nyquist, 2014/06/26 20:39:57]

Add a comment. For example, what can we get from this member that we use?

[smaslo, 2014/06/27 18:33:17]

Done.

 };
 
 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);

[nyquist, 2014/06/26 20:39:57]

Nit: Use reader_mode_prefs_

[smaslo, 2014/06/27 18:33:17]

Done.

 }
 
 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(
     const std::string& buffer) {
   if (waiting_for_page_ready_) {
     buffer_ += buffer;
   } else {
     if (web_contents_) {
       web_contents_->GetMainFrame()->ExecuteJavaScript(
           base::UTF8ToUTF16(buffer));
@@ skipping 69 matching lines @@
     // 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()));
     }
   }
+  SendJavaScript("alert(0);");

[nyquist, 2014/06/26 20:39:58]

You probably don't want this.

[smaslo, 2014/06/27 18:33:17]

Done.

+  SetBodyClassName();
   // 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_);
@@ skipping 52 matching lines @@
   content::WebContents* web_contents =
       content::WebContents::FromRenderFrameHost(
           content::RenderFrameHost::FromID(render_process_id,
                                            render_frame_id));
   DCHECK(web_contents);
   // An empty |path| is invalid, but guard against it. If not empty, assume
   // |path| starts with '?', which is stripped away.
   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);
 
   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 {
@@ skipping 26 matching lines @@
 void DomDistillerViewerSource::WillServiceRequest(
     const net::URLRequest* request,
     std::string* path) const {
 }
 
 std::string DomDistillerViewerSource::GetContentSecurityPolicyObjectSrc()
     const {
   return "object-src 'none'; style-src 'self';";
 }
 
+void DomDistillerViewerSource::RequestViewerHandle::OnChangeTheme(

[nyquist, 2014/06/26 20:39:57]

Could you move these methods up to the rest of the methods for
DomDistillerViewerSource::RequestViewerHandle?

[smaslo, 2014/06/27 18:33:17]

Done.

+    ReaderModePrefs* ReaderModePrefs) {
+  SetBodyClassName();
+}
+
+void DomDistillerViewerSource::RequestViewerHandle::SetBodyClassName() {
+  std::string s = "document.body.className='";

[nyquist, 2014/06/26 20:39:57]

Could you maybe use std::ostringstream?

std::ostringstream js;
js << "document.body.className='" << reader_mode_prefs_->GetBodyCssClass() <<
"';";
SendJavaScript(js.str());

If the middle line is too long, maybe just store the body CSS class as a
variable.

[smaslo, 2014/06/27 18:33:17]

Done.

+  s += reader_mode_prefs_->GetBodyCssClass();
+  s += "';";
+  SendJavaScript(s);
+}
+
 }  // namespace dom_distiller