Refactor feedback to use native JS object

components/dom_distiller/content/browser/distiller_javascript_service_impl.h

Index: components/dom_distiller/content/browser/distiller_javascript_service_impl.h
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 d21cd4a427dec8de8443191135021b9bc67d3923..a12603358b1f073d11129ab44fd5418df4523fa7 100644
--- a/components/dom_distiller/content/browser/distiller_javascript_service_impl.h
+++ b/components/dom_distiller/content/browser/distiller_javascript_service_impl.h
@@ -5,6 +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/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"
@@ -15,18 +16,25 @@ namespace dom_distiller {
 class DistillerJavaScriptServiceImpl : public DistillerJavaScriptService {
  public:
   DistillerJavaScriptServiceImpl(
-      mojo::InterfaceRequest<DistillerJavaScriptService> request);
+      mojo::InterfaceRequest<DistillerJavaScriptService> request,
+      content::RenderFrameHost* render_frame_host,
+      ExternalFeedbackReporter* external_feedback_reporter);
   ~DistillerJavaScriptServiceImpl() override;
 
   // Mojo DistillerJavaScriptService implementation.
   void HandleDistillerEchoCall(const mojo::String& message) override;
+  void HandleDistillerFeedbackCall(bool good) override;
 
  private:
   mojo::StrongBinding<DistillerJavaScriptService> binding_;

[wychen, 2015/09/09 21:12:24]

Where is this used?

[mdjones, 2015/09/09 22:35:16]

Mojo's pipe uses it. See:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/mojo/s...

+  content::RenderFrameHost* render_frame_host_;
+  ExternalFeedbackReporter* external_feedback_reporter_;
 };
 
 // static
 void CreateDistillerJavaScriptService(

[wychen, 2015/09/09 21:12:24]

Would it be more readable if the order of the params are the same as the
constructor?

[mdjones, 2015/09/09 22:35:16]

The way that this is used (with base::Bind) requires the request param be last
in the list. Instead I updated the constructor.

+    content::RenderFrameHost* render_frame_host,
+    ExternalFeedbackReporter* feedback_reporter,
     mojo::InterfaceRequest<DistillerJavaScriptService> request);
 
 }  // namespace dom_distiller