Convert Web Store Inline Install IPCs to mojo

chrome/renderer/extensions/webstore_bindings.h

Index: chrome/renderer/extensions/webstore_bindings.h
diff --git a/chrome/renderer/extensions/webstore_bindings.h b/chrome/renderer/extensions/webstore_bindings.h
index 641fddd50888c3bbfd34d38102484fdbb2814111..d801e37b1ce7be32d3c746e78877def51a8e951a 100644
--- a/chrome/renderer/extensions/webstore_bindings.h
+++ b/chrome/renderer/extensions/webstore_bindings.h
@@ -7,38 +7,46 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "chrome/common/extensions/mojo/inline_install.mojom.h"
 #include "chrome/common/extensions/webstore_install_result.h"
 #include "chrome/renderer/extensions/chrome_v8_extension_handler.h"
 #include "extensions/renderer/object_backed_native_handler.h"
+#include "mojo/public/cpp/bindings/binding_set.h"
 
 namespace blink {
 class WebLocalFrame;
 }
 
 namespace extensions {
+class ScriptContext;
+class WebstoreBindingsHelper;

[Devlin, 2017/04/28 18:33:02]

not needed

[catmullings, 2017/05/02 19:02:15]

Done.

 
 // A V8 extension that creates an object at window.chrome.webstore. This object
 // allows JavaScript to initiate inline installs of apps that are listed in the
 // Chrome Web Store (CWS).
 class WebstoreBindings : public ObjectBackedNativeHandler,
-                         public ChromeV8ExtensionHandler {
+                         public ChromeV8ExtensionHandler,
+                         public mojom::InlineInstallProgressListener {
  public:
   explicit WebstoreBindings(ScriptContext* context);
+  ~WebstoreBindings() override;
 
-  // IPC::Listener
+  // IPC::Listener:
   bool OnMessageReceived(const IPC::Message& message) override;
 
- private:
-  void Install(const v8::FunctionCallbackInfo<v8::Value>& args);
+  void InlineInstallResponse(int install_id,

[Devlin, 2017/04/28 18:33:02]

I think this can be private, right?

[catmullings, 2017/05/02 19:02:15]

With InlineInstallResponse as an mojo msg instead of a callback, it should
remain public.

+                             bool success,
+                             const std::string& error,
+                             webstore_install::Result result);
 
-  void OnInlineWebstoreInstallResponse(int install_id,
-                                       bool success,
-                                       const std::string& error,
-                                       webstore_install::Result result);
+  // mojom::InlineInstallProgressListener
+  void InlineInstallStageChanged(api::webstore::InstallStage stage) override;
+  void InlineInstallDownloadProgress(int percent_downloaded) override;
 
-  void OnInlineInstallStageChanged(int stage);
+ private:
+  void OnInstallProgressListenerBindingError();
 
-  void OnInlineInstallDownloadProgress(int percent_downloaded);
+  void Install(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   // Extracts a Web Store item ID from a <link rel="chrome-webstore-item"
   // href="https://chrome.google.com/webstore/detail/id"> node found in the
@@ -51,6 +59,11 @@ class WebstoreBindings : public ObjectBackedNativeHandler,
       std::string* webstore_item_id,
       std::string* error);
 
+  extensions::mojom::InlineInstallAssociatedPtr inline_install_;

[Devlin, 2017/04/28 18:33:02]

no need for extensions:: namespace

[catmullings, 2017/05/02 19:02:15]

Done.

+
+  mojo::BindingSet<mojom::InlineInstallProgressListener>

[catmullings, 2017/04/28 03:07:30]

A (weak) binding set does not own the object that implements the corresponding
interface -- only a strong binding set would. Using a (weak) binding set would
prevent double deletion, in the case that the interface implementation were to
own the binding and the binding also owns the implementation (through passing
the object via |this|). 

However, by default, a (weak) binding set doesn't treat its own destruction as a
signal to unregister all the current bindings. I've had to handle this manually
via set_connection_error (see
https://codereview.chromium.org/2791533002/diff/180001/chrome/renderer/extens...
and #newcode66). 

Finally, a (weak) binding set may not do the following: "When the pipe a binding
is bound to is disconnected, the binding is automatically destroyed and removed
from the set, and the interface implementation is deleted." Would this be a
problem in context of this CL? If it is, lmk and I'll try to determine a
workaround. 

[Devlin, 2017/04/28 18:33:02]

I think that's desired - otherwise, we'd end up deleting this object
artificially soon.

[catmullings, 2017/05/02 19:02:15]

Confirming that indeed that a regular binding set does destroy and remove a
binding if the binding's corresponding pipe becomes disconnected.

+      install_progress_listener_bindings_;
+
   DISALLOW_COPY_AND_ASSIGN(WebstoreBindings);
 };