Refactor feedback to use native JS object

components/dom_distiller/content/browser/distiller_javascript_service_impl.h

 // 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_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"
 
 namespace dom_distiller {
 
 // This is the receiving end of "distiller" JavaScript object calls.
 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
 
 #endif  // COMPONENTS_DOM_DISTILLER_CONTENT_BROWSER_DISTILLER_JAVASCRIPT_SERVICE_IMPL_H_