Convert Web Store Inline Install IPCs to mojo

chrome/renderer/extensions/webstore_bindings.cc

 // Copyright (c) 2012 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 "chrome/renderer/extensions/webstore_bindings.h"
 
 #include <stdint.h>
 
 #include "base/macros.h"
 #include "base/strings/string_util.h"
 #include "chrome/common/extensions/api/webstore/webstore_api_constants.h"
-#include "chrome/common/extensions/chrome_extension_messages.h"
 #include "components/crx_file/id_util.h"
+#include "content/public/common/associated_interface_provider.h"
 #include "content/public/renderer/render_frame.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_urls.h"
 #include "extensions/renderer/script_context.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
+#include "services/service_manager/public/cpp/interface_provider.h"
+#include "services/service_manager/public/cpp/interface_registry.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebElement.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebNode.h"
 #include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
 #include "url/gurl.h"
 #include "v8/include/v8.h"
 
 using blink::WebDocument;
 using blink::WebElement;
@@ skipping 10 matching lines @@
     "Chrome Web Store installations can only be started by the top frame.";
 const char kNotUserGestureError[] =
     "Chrome Web Store installations can only be initated by a user gesture.";
 const char kNoWebstoreItemLinkFoundError[] =
     "No Chrome Web Store item link found.";
 const char kInvalidWebstoreItemUrlError[] =
     "Invalid Chrome Web Store item URL.";
 
 // chrome.webstore.install() calls generate an install ID so that the install's
 // callbacks may be fired when the browser notifies us of install completion
-// (successful or not) via OnInlineWebstoreInstallResponse.
+// (successful or not) via InlineInstallResponse.
 int g_next_install_id = 0;
 
 } // anonymous namespace
 
+class WebstoreBindings::WebstoreBindingsHelper
+    : public ObjectBackedNativeHandler,
+      public mojom::InlineInstallStatus {
+ public:
+  static void Create(ScriptContext* context,
+                     mojom::InlineInstallStatusRequest request);
+
+ private:
+  WebstoreBindingsHelper(ScriptContext* context,
+                         mojom::InlineInstallStatusRequest request);
+  ~WebstoreBindingsHelper() override;
+
+  void InlineInstallStageChanged(api::webstore::InstallStage stage) override;
+  void InlineInstallDownloadProgress(int percent_downloaded) override;
+
+  mojo::Binding<mojom::InlineInstallStatus> binding_;
+};
+
+// static
+void WebstoreBindings::WebstoreBindingsHelper::Create(
+    ScriptContext* context,
+    mojom::InlineInstallStatusRequest request) {
+  new WebstoreBindingsHelper(context, std::move(request));
+}
+
+WebstoreBindings::WebstoreBindingsHelper::WebstoreBindingsHelper(
+    ScriptContext* context,
+    mojom::InlineInstallStatusRequest request)
+    : ObjectBackedNativeHandler(context), binding_(this, std::move(request)) {}
+
+WebstoreBindings::WebstoreBindingsHelper::~WebstoreBindingsHelper() = default;
+
+void WebstoreBindings::WebstoreBindingsHelper::InlineInstallStageChanged(
+    api::webstore::InstallStage stage) {
+  const char* stage_string = NULL;
+  api::webstore::InstallStage install_stage =
+      static_cast<api::webstore::InstallStage>(stage);
+  switch (install_stage) {
+    case api::webstore::INSTALL_STAGE_DOWNLOADING:
+      stage_string = api::webstore::kInstallStageDownloading;
+      break;
+    case api::webstore::INSTALL_STAGE_INSTALLING:
+      stage_string = api::webstore::kInstallStageInstalling;
+      break;
+  }
+  v8::Isolate* isolate = context()->isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Context::Scope context_scope(context()->v8_context());
+  v8::Local<v8::Value> argv[] = {
+      v8::String::NewFromUtf8(isolate, stage_string)};
+  context()->module_system()->CallModuleMethodSafe(
+      "webstore", "onInstallStageChanged", arraysize(argv), argv);
+}
+
+void WebstoreBindings::WebstoreBindingsHelper::InlineInstallDownloadProgress(
+    int percent_downloaded) {
+  v8::Isolate* isolate = context()->isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Context::Scope context_scope(context()->v8_context());
+  v8::Local<v8::Value> argv[] = {
+      v8::Number::New(isolate, percent_downloaded / 100.0)};
+  context()->module_system()->CallModuleMethodSafe(
+      "webstore", "onDownloadProgress", arraysize(argv), argv);
+}
+
+ScriptContext* WebstoreBindings::context_ = NULL;
+
 WebstoreBindings::WebstoreBindings(ScriptContext* context)
     : ObjectBackedNativeHandler(context) {
+  WebstoreBindings::SetContext(context);
   RouteFunction("Install", "webstore",
                 base::Bind(&WebstoreBindings::Install, base::Unretained(this)));
+  context->GetRenderFrame()->GetRemoteAssociatedInterfaces()->GetInterface(
+      &inline_install_);
+  context->GetRenderFrame()->GetInterfaceRegistry()->AddInterface(
+      base::Bind(&WebstoreBindingsHelper::Create, context));

[Devlin, 2017/04/24 20:08:32]

This isn't necessary what we want.  In particular, we don't necessarily want to
strictly bind all requests for a frame to a single access point.  Instead, we
want each context to be able to receive its own messages.

Previously, the way we accomplished this was by having WebstoreBindings inherit
ChromeV8ExtensionHandler - a poorly named class that let these receive IPC
messages directly.  Now that we're moving away from these IPCs, we need a
different approach.

The issue with this current way is that a) it relies too much on which contexts
may or may not be created/valid and the order in which they are, and b) it binds
all requests to a single context.

My suggestion to fix this would be to have the DoInlineInstall() message take an
interface to an InlineInstallStatus:

DoInlineInstall(
    int32 install_id, string webstore_item_id,
    int32 listeners_mask, InlineInstallStatus install_status) =>
       (int32 install_id, bool success, string error,
        WebstoreInstallResult result);

(Ken pointed me to
https://chromium.googlesource.com/chromium/src/+/master/mojo/public/cpp/bindi...
for references on passing interfaces)

Then, the TabHelper on the browser side uses that passed interface to call these
update methods.

On the renderer side, we'd have the WebstoreBindingsHelper inherit from
InlineInstallStatus just as it does now, but be owned by the WebstoreBindings
and passed as an argument to the DoInlineInstall() method.

This allows us to have a separate InlineInstallStatus for each context in the
frame, similar to what we do now, and couples the lifetime of the
InlineInstallStatus to the WebstoreBindings (which is good from a deterministic
point of view).

Does that make sense?

(Separately - maybe we should rename InlineInstallStatus to
InlineInstallProgressListener or something?)

[catmullings, 2017/04/26 20:36:34]

Done.

 }
 
+WebstoreBindings::~WebstoreBindings() {}
+
 void WebstoreBindings::Install(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
   content::RenderFrame* render_frame = context()->GetRenderFrame();
   if (!render_frame)
     return;
 
   // The first two arguments indicate whether or not there are install stage
   // or download progress listeners.
   int listener_mask = 0;
   CHECK(args[0]->IsBoolean());
@@ skipping 15 matching lines @@
 
   if (!GetWebstoreItemIdFromFrame(
       frame, preferred_store_link_url, &webstore_item_id, &error)) {
     args.GetIsolate()->ThrowException(
         v8::String::NewFromUtf8(args.GetIsolate(), error.c_str()));
     return;
   }
 
   int install_id = g_next_install_id++;
 
-  Send(new ExtensionHostMsg_InlineWebstoreInstall(
-      render_frame->GetRoutingID(), install_id, GetRoutingID(),
-      webstore_item_id, listener_mask));
+  inline_install_->DoInlineInstall(
+      install_id, webstore_item_id, listener_mask,
+      base::Bind(&WebstoreBindings::InlineInstallResponse));
 
   args.GetReturnValue().Set(static_cast<int32_t>(install_id));
 }
 
 // static
 bool WebstoreBindings::GetWebstoreItemIdFromFrame(
     blink::WebLocalFrame* frame,
     const std::string& preferred_store_link_url,
     std::string* webstore_item_id,
     std::string* error) {
@@ skipping 73 matching lines @@
 
     *webstore_item_id = candidate_webstore_item_id;
     return true;
   }
 
   *error = kNoWebstoreItemLinkFoundError;
   return false;
 }
 
 bool WebstoreBindings::OnMessageReceived(const IPC::Message& message) {
-  IPC_BEGIN_MESSAGE_MAP(WebstoreBindings, message)
-    IPC_MESSAGE_HANDLER(ExtensionMsg_InlineWebstoreInstallResponse,
-                        OnInlineWebstoreInstallResponse)
-    IPC_MESSAGE_HANDLER(ExtensionMsg_InlineInstallStageChanged,
-                        OnInlineInstallStageChanged)
-    IPC_MESSAGE_HANDLER(ExtensionMsg_InlineInstallDownloadProgress,
-                        OnInlineInstallDownloadProgress)
-    IPC_MESSAGE_UNHANDLED(CHECK(false) << "Unhandled IPC message")
-  IPC_END_MESSAGE_MAP()
   return true;
 }
 
-void WebstoreBindings::OnInlineWebstoreInstallResponse(
-    int install_id,
-    bool success,
-    const std::string& error,
-    webstore_install::Result result) {
-  v8::Isolate* isolate = context()->isolate();
+// static
+void WebstoreBindings::InlineInstallResponse(int install_id,
+                                             bool success,
+                                             const std::string& error,
+                                             webstore_install::Result result) {
+  v8::Isolate* isolate = WebstoreBindings::GetContext()->isolate();
   v8::HandleScope handle_scope(isolate);
-  v8::Context::Scope context_scope(context()->v8_context());
+  v8::Context::Scope context_scope(
+      WebstoreBindings::GetContext()->v8_context());
   v8::Local<v8::Value> argv[] = {
     v8::Integer::New(isolate, install_id),
     v8::Boolean::New(isolate, success),
     v8::String::NewFromUtf8(isolate, error.c_str()),
     v8::String::NewFromUtf8(
         isolate, api::webstore::kInstallResultCodes[static_cast<int>(result)])
   };
-  context()->module_system()->CallModuleMethodSafe(
+  WebstoreBindings::GetContext()->module_system()->CallModuleMethodSafe(
       "webstore", "onInstallResponse", arraysize(argv), argv);
 }
 
-void WebstoreBindings::OnInlineInstallStageChanged(int stage) {
-  const char* stage_string = NULL;
-  api::webstore::InstallStage install_stage =
-      static_cast<api::webstore::InstallStage>(stage);
-  switch (install_stage) {
-    case api::webstore::INSTALL_STAGE_DOWNLOADING:
-      stage_string = api::webstore::kInstallStageDownloading;
-      break;
-    case api::webstore::INSTALL_STAGE_INSTALLING:
-      stage_string = api::webstore::kInstallStageInstalling;
-      break;
-  }
-  v8::Isolate* isolate = context()->isolate();
-  v8::HandleScope handle_scope(isolate);
-  v8::Context::Scope context_scope(context()->v8_context());
-  v8::Local<v8::Value> argv[] = {
-      v8::String::NewFromUtf8(isolate, stage_string)};
-  context()->module_system()->CallModuleMethodSafe(
-      "webstore", "onInstallStageChanged", arraysize(argv), argv);
-}
-
-void WebstoreBindings::OnInlineInstallDownloadProgress(int percent_downloaded) {
-  v8::Isolate* isolate = context()->isolate();
-  v8::HandleScope handle_scope(isolate);
-  v8::Context::Scope context_scope(context()->v8_context());
-  v8::Local<v8::Value> argv[] = {
-      v8::Number::New(isolate, percent_downloaded / 100.0)};
-  context()->module_system()->CallModuleMethodSafe(
-      "webstore", "onDownloadProgress", arraysize(argv), argv);
-}
-
 }  // namespace extensions