Open distiller UI setting through JavaScript

This change adds support for opening the android view distiller
settings from JavaScript via "distiller.openSettings". To avoid
passing numerous objects into the DomDistillerViewerSource, the
ExternalFeedbackReporter has been refactored into a generic UI
handle that is responsible for any interaction between Chrome and
Android specific controls and the distiller component.

The actual UI in the page is not yet implemented.

BUG=

Review URL: https://codereview.chromium.org/1386043002

Cr-Commit-Position: refs/heads/master@{#354123}

9 files changed, 91 insertions, 41 deletions

diff --git a/components/dom_distiller/content/BUILD.gn b/components/dom_distiller/content/BUILD.gn
index cc9137b..ef7c32f 100644
--- a/components/dom_distiller/content/BUILD.gn
+++ b/components/dom_distiller/content/BUILD.gn
@@ -25,9 +25,9 @@ static_library("content_browser") {
     "browser/distiller_javascript_utils.h",
     "browser/distiller_page_web_contents.cc",
     "browser/distiller_page_web_contents.h",
+    "browser/distiller_ui_handle.h",
     "browser/dom_distiller_viewer_source.cc",
     "browser/dom_distiller_viewer_source.h",
-    "browser/external_feedback_reporter.h",
     "browser/web_contents_main_frame_observer.cc",
     "browser/web_contents_main_frame_observer.h",
   ]
diff --git a/components/dom_distiller/content/browser/distiller_javascript_service_impl.cc b/components/dom_distiller/content/browser/distiller_javascript_service_impl.cc
index 3f9a7a6..e60e3e2 100644
--- a/components/dom_distiller/content/browser/distiller_javascript_service_impl.cc
+++ b/components/dom_distiller/content/browser/distiller_javascript_service_impl.cc
@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 #include "components/dom_distiller/content/browser/distiller_javascript_service_impl.h"
-#include "components/dom_distiller/content/browser/external_feedback_reporter.h"
+#include "components/dom_distiller/content/browser/distiller_ui_handle.h"
 #include "components/dom_distiller/core/feedback_reporter.h"
 #include "content/public/browser/user_metrics.h"
 #include "third_party/mojo/src/mojo/public/cpp/bindings/string.h"
@@ -12,11 +12,11 @@ namespace dom_distiller {
 
 DistillerJavaScriptServiceImpl::DistillerJavaScriptServiceImpl(
     content::RenderFrameHost* render_frame_host,
-    ExternalFeedbackReporter* external_feedback_reporter,
+    DistillerUIHandle* distiller_ui_handle,
     mojo::InterfaceRequest<DistillerJavaScriptService> request)
     : binding_(this, request.Pass()),
       render_frame_host_(render_frame_host),
-      external_feedback_reporter_(external_feedback_reporter) {}
+      distiller_ui_handle_(distiller_ui_handle) {}
 
 DistillerJavaScriptServiceImpl::~DistillerJavaScriptServiceImpl() {}
 
@@ -31,12 +31,12 @@ void DistillerJavaScriptServiceImpl::HandleDistillerFeedbackCall(
   }
 
   // If feedback is bad try to start up external feedback.
-  if (!external_feedback_reporter_) {
+  if (!distiller_ui_handle_) {
     return;
   }
   content::WebContents* contents =
       content::WebContents::FromRenderFrameHost(render_frame_host_);
-  external_feedback_reporter_->ReportExternalFeedback(
+  distiller_ui_handle_->ReportExternalFeedback(
       contents, contents->GetURL(), false);
   return;
 }
@@ -45,12 +45,21 @@ void DistillerJavaScriptServiceImpl::HandleDistillerClosePanelCall() {
   content::RecordAction(base::UserMetricsAction("DomDistiller_ViewOriginal"));
 }
 
+void DistillerJavaScriptServiceImpl::HandleDistillerOpenSettingsCall() {
+  if (!distiller_ui_handle_) {
+    return;
+  }
+  content::WebContents* contents =
+      content::WebContents::FromRenderFrameHost(render_frame_host_);
+  distiller_ui_handle_->OpenSettings(contents);
+}
+
 void CreateDistillerJavaScriptService(
     content::RenderFrameHost* render_frame_host,
-    ExternalFeedbackReporter* feedback_reporter,
+    DistillerUIHandle* distiller_ui_handle,
     mojo::InterfaceRequest<DistillerJavaScriptService> request) {
   // This is strongly bound and owned by the pipe.
-  new DistillerJavaScriptServiceImpl(render_frame_host, feedback_reporter,
+  new DistillerJavaScriptServiceImpl(render_frame_host, distiller_ui_handle,
       request.Pass());
 }
 
diff --git a/components/dom_distiller/content/browser/distiller_javascript_service_impl.h b/components/dom_distiller/content/browser/distiller_javascript_service_impl.h
index fd58892..d1ffe10 100644
--- a/components/dom_distiller/content/browser/distiller_javascript_service_impl.h
+++ b/components/dom_distiller/content/browser/distiller_javascript_service_impl.h
@@ -5,7 +5,7 @@
 #ifndef COMPONENTS_DOM_DISTILLER_CONTENT_BROWSER_DISTILLER_JAVASCRIPT_SERVICE_IMPL_H_
 #define COMPONENTS_DOM_DISTILLER_CONTENT_BROWSER_DISTILLER_JAVASCRIPT_SERVICE_IMPL_H_
 
-#include "components/dom_distiller/content/browser/external_feedback_reporter.h"
+#include "components/dom_distiller/content/browser/distiller_ui_handle.h"
 #include "components/dom_distiller/content/common/distiller_javascript_service.mojom.h"
 #include "third_party/mojo/src/mojo/public/cpp/bindings/string.h"
 #include "third_party/mojo/src/mojo/public/cpp/bindings/strong_binding.h"
@@ -17,7 +17,7 @@ class DistillerJavaScriptServiceImpl : public DistillerJavaScriptService {
  public:
   DistillerJavaScriptServiceImpl(
       content::RenderFrameHost* render_frame_host,
-      ExternalFeedbackReporter* external_feedback_reporter,
+      DistillerUIHandle* distiller_ui_handle,
       mojo::InterfaceRequest<DistillerJavaScriptService> request);
   ~DistillerJavaScriptServiceImpl() override;
 
@@ -33,16 +33,19 @@ class DistillerJavaScriptServiceImpl : public DistillerJavaScriptService {
   // Make a call into Android to close the overlay panel containing reader mode.
   void HandleDistillerClosePanelCall() override;
 
+  // Show the Android view containing Reader Mode settings.
+  void HandleDistillerOpenSettingsCall() override;
+
  private:
   mojo::StrongBinding<DistillerJavaScriptService> binding_;
   content::RenderFrameHost* render_frame_host_;
-  ExternalFeedbackReporter* external_feedback_reporter_;
+  DistillerUIHandle* distiller_ui_handle_;
 };
 
 // static
 void CreateDistillerJavaScriptService(
     content::RenderFrameHost* render_frame_host,
-    ExternalFeedbackReporter* feedback_reporter,
+    DistillerUIHandle* distiller_ui_handle,
     mojo::InterfaceRequest<DistillerJavaScriptService> request);
 
 }  // namespace dom_distiller
diff --git a/components/dom_distiller/content/browser/distiller_ui_handle.h b/components/dom_distiller/content/browser/distiller_ui_handle.h
new file mode 100644
index 0000000..7908a58
--- /dev/null
+++ b/components/dom_distiller/content/browser/distiller_ui_handle.h
@@ -0,0 +1,35 @@
+// Copyright 2015 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.
+
+#ifndef COMPONENTS_DOM_DISTILLER_CONTENT_BROWSER_DISTILLER_UI_HANDLE_H_
+#define COMPONENTS_DOM_DISTILLER_CONTENT_BROWSER_DISTILLER_UI_HANDLE_H_
+
+#include "base/macros.h"
+#include "content/public/browser/web_contents.h"
+#include "url/gurl.h"
+
+namespace dom_distiller {
+
+// ExternalFeedbackReporter handles reporting distillation quality through an
+// external source.
+class DistillerUIHandle {
+ public:
+  DistillerUIHandle() {}
+  virtual ~DistillerUIHandle() {}
+
+  // Start an external form to record user feedback.
+  virtual void ReportExternalFeedback(content::WebContents* web_contents,
+                                      const GURL& url,
+                                      const bool good) = 0;
+
+  // Open the UI settings for dom distiller.
+  virtual void OpenSettings(content::WebContents* web_contents) = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DistillerUIHandle);
+};
+
+}  // namespace dom_distiller
+
+#endif  // COMPONENTS_DOM_DISTILLER_CONTENT_BROWSER_DISTILLER_UI_HANDLE_H_
diff --git a/components/dom_distiller/content/browser/dom_distiller_viewer_source.cc b/components/dom_distiller/content/browser/dom_distiller_viewer_source.cc
index 4a8e424..65a327d 100644
--- a/components/dom_distiller/content/browser/dom_distiller_viewer_source.cc
+++ b/components/dom_distiller/content/browser/dom_distiller_viewer_source.cc
@@ -16,7 +16,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "components/dom_distiller/content/browser/distiller_javascript_service_impl.h"
 #include "components/dom_distiller/content/browser/distiller_javascript_utils.h"
-#include "components/dom_distiller/content/browser/external_feedback_reporter.h"
+#include "components/dom_distiller/content/browser/distiller_ui_handle.h"
 #include "components/dom_distiller/content/common/distiller_page_notifier_service.mojom.h"
 #include "components/dom_distiller/core/distilled_page_prefs.h"
 #include "components/dom_distiller/core/dom_distiller_request_view_base.h"
@@ -168,10 +168,10 @@ void DomDistillerViewerSource::RequestViewerHandle::DidFinishLoad(
 DomDistillerViewerSource::DomDistillerViewerSource(
     DomDistillerServiceInterface* dom_distiller_service,
     const std::string& scheme,
-    scoped_ptr<ExternalFeedbackReporter> external_reporter)
+    scoped_ptr<DistillerUIHandle> ui_handle)
     : scheme_(scheme),
       dom_distiller_service_(dom_distiller_service),
-      external_feedback_reporter_(external_reporter.Pass()) {
+      distiller_ui_handle_(ui_handle.Pass()) {
 }
 
 DomDistillerViewerSource::~DomDistillerViewerSource() {
@@ -232,7 +232,7 @@ void DomDistillerViewerSource::StartDataRequest(
   render_frame_host->GetServiceRegistry()->AddService(
       base::Bind(&CreateDistillerJavaScriptService,
           render_frame_host,
-          external_feedback_reporter_.get()));
+          distiller_ui_handle_.get()));
 
   // Tell the renderer that this is currently a distilled page.
   DistillerPageNotifierServicePtr page_notifier_service;
diff --git a/components/dom_distiller/content/browser/dom_distiller_viewer_source.h b/components/dom_distiller/content/browser/dom_distiller_viewer_source.h
index 9cf77d7..fbd3d5b 100644
--- a/components/dom_distiller/content/browser/dom_distiller_viewer_source.h
+++ b/components/dom_distiller/content/browser/dom_distiller_viewer_source.h
@@ -10,7 +10,7 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
-#include "components/dom_distiller/content/browser/external_feedback_reporter.h"
+#include "components/dom_distiller/content/browser/distiller_ui_handle.h"
 #include "content/public/browser/url_data_source.h"
 
 namespace dom_distiller {
@@ -26,7 +26,7 @@ class DomDistillerViewerSource : public content::URLDataSource {
   DomDistillerViewerSource(
       DomDistillerServiceInterface* dom_distiller_service,
       const std::string& scheme,
-      scoped_ptr<ExternalFeedbackReporter> external_reporter);
+      scoped_ptr<DistillerUIHandle> ui_handle);
   ~DomDistillerViewerSource() override;
 
   class RequestViewerHandle;
@@ -55,8 +55,9 @@ class DomDistillerViewerSource : public content::URLDataSource {
   // the list of articles.
   DomDistillerServiceInterface* dom_distiller_service_;
 
-  // A means for starting/opening an external service for feedback reporting.
-  scoped_ptr<ExternalFeedbackReporter> external_feedback_reporter_;
+  // An object for accessing chrome-specific UI controls including external
+  // feedback and opening the distiller settings.
+  scoped_ptr<DistillerUIHandle> distiller_ui_handle_;
 
   DISALLOW_COPY_AND_ASSIGN(DomDistillerViewerSource);
 };
diff --git a/components/dom_distiller/content/common/distiller_javascript_service.mojom b/components/dom_distiller/content/common/distiller_javascript_service.mojom
index 3f0f89a..dbf55e8 100644
--- a/components/dom_distiller/content/common/distiller_javascript_service.mojom
+++ b/components/dom_distiller/content/common/distiller_javascript_service.mojom
@@ -17,4 +17,8 @@ interface DistillerJavaScriptService {
   // Handle closing the overlay panel that contains Reader Mode; the
   // "distiller.close" function.
   HandleDistillerClosePanelCall();
+
+  // Open the Android view containing settings for Reader Mode; the
+  // "distiller.openSettings" function.
+  HandleDistillerOpenSettingsCall();
 };
diff --git a/components/dom_distiller/content/renderer/distiller_native_javascript.cc b/components/dom_distiller/content/renderer/distiller_native_javascript.cc
index 02a14dd..14a3411 100644
--- a/components/dom_distiller/content/renderer/distiller_native_javascript.cc
+++ b/components/dom_distiller/content/renderer/distiller_native_javascript.cc
@@ -39,25 +39,39 @@ void DistillerNativeJavaScript::AddJavaScriptObjectToFrame(
   v8::Local<v8::Object> distiller_obj =
       GetOrCreateDistillerObject(isolate, context->Global());
 
+  EnsureServiceConnected();
+
+  // Some of the JavaScript functions require extra work to be done when it is
+  // called, so they have wrapper functions maintained in this class.
   BindFunctionToObject(
       distiller_obj,
       "echo",
       base::Bind(
           &DistillerNativeJavaScript::DistillerEcho, base::Unretained(this)));
 
+  // Many functions can simply call the Mojo interface directly and have no
+  // wrapper function for binding. Note that calling distiller_js_service.get()
+  // does not transfer ownership of the interface.
   BindFunctionToObject(
       distiller_obj,
       "sendFeedback",
       base::Bind(
-          &DistillerNativeJavaScript::DistillerSendFeedback,
-          base::Unretained(this)));
+          &DistillerJavaScriptService::HandleDistillerFeedbackCall,
+          base::Unretained(distiller_js_service_.get())));
 
   BindFunctionToObject(
       distiller_obj,
       "closePanel",
       base::Bind(
-          &DistillerNativeJavaScript::DistillerClosePanel,
-          base::Unretained(this)));
+          &DistillerJavaScriptService::HandleDistillerClosePanelCall,
+          base::Unretained(distiller_js_service_.get())));
+
+  BindFunctionToObject(
+      distiller_obj,
+      "openSettings",
+      base::Bind(
+          &DistillerJavaScriptService::HandleDistillerOpenSettingsCall,
+          base::Unretained(distiller_js_service_.get())));
 }
 
 template<typename Sig>
@@ -73,22 +87,12 @@ void DistillerNativeJavaScript::BindFunctionToObject(
 }
 
 void DistillerNativeJavaScript::EnsureServiceConnected() {
-  if (!distiller_js_service_) {
+  if (!distiller_js_service_ || !distiller_js_service_.is_bound()) {
     render_frame_->GetServiceRegistry()->ConnectToRemoteService(
         mojo::GetProxy(&distiller_js_service_));
   }
 }
 
-void DistillerNativeJavaScript::DistillerSendFeedback(bool good) {
-  EnsureServiceConnected();
-  distiller_js_service_->HandleDistillerFeedbackCall(good);
-}
-
-void DistillerNativeJavaScript::DistillerClosePanel() {
-  EnsureServiceConnected();
-  distiller_js_service_->HandleDistillerClosePanelCall();
-}
-
 std::string DistillerNativeJavaScript::DistillerEcho(
     const std::string& message) {
   EnsureServiceConnected();
diff --git a/components/dom_distiller/content/renderer/distiller_native_javascript.h b/components/dom_distiller/content/renderer/distiller_native_javascript.h
index 77f380b..c190478 100644
--- a/components/dom_distiller/content/renderer/distiller_native_javascript.h
+++ b/components/dom_distiller/content/renderer/distiller_native_javascript.h
@@ -37,12 +37,6 @@ class DistillerNativeJavaScript {
   // provided string.
   std::string DistillerEcho(const std::string& message);
 
-  // Send feedback about distillation quality.
-  void DistillerSendFeedback(bool good);
-
-  // Close the UI panel and record the action.
-  void DistillerClosePanel();
-
   content::RenderFrame* render_frame_;
   DistillerJavaScriptServicePtr distiller_js_service_;
 };