Convert Web Store Inline Install IPCs to mojo

chrome/browser/extensions/tab_helper.cc

Index: chrome/browser/extensions/tab_helper.cc
diff --git a/chrome/browser/extensions/tab_helper.cc b/chrome/browser/extensions/tab_helper.cc
index f85e854b63c31cfd75b15b6b227605ca4877b864..adbdab762927959c44bb1894d4a0985c31a9bb69 100644
--- a/chrome/browser/extensions/tab_helper.cc
+++ b/chrome/browser/extensions/tab_helper.cc
@@ -33,7 +33,6 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/extensions/api/webstore/webstore_api_constants.h"
-#include "chrome/common/extensions/chrome_extension_messages.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/render_messages.h"
@@ -64,6 +63,7 @@
 #include "extensions/common/extension_urls.h"
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
+#include "services/service_manager/public/cpp/interface_provider.h"
 #include "url/url_constants.h"
 
 #if defined(OS_WIN)
@@ -89,7 +89,6 @@ class TabHelper::InlineInstallObserver : public InstallObserver {
                         bool observe_download_progress,
                         bool observe_install_stage)
       : tab_helper_(tab_helper),
-        routing_id_(routing_id),
         extension_id_(extension_id),
         observe_download_progress_(observe_download_progress),
         observe_install_stage_(observe_install_stage),
@@ -112,8 +111,8 @@ class TabHelper::InlineInstallObserver : public InstallObserver {
   void OnDownloadProgress(const ExtensionId& extension_id,
                           int percent_downloaded) override {
     if (observe_download_progress_ && extension_id == extension_id_) {
-      tab_helper_->Send(new ExtensionMsg_InlineInstallDownloadProgress(
-          routing_id_, percent_downloaded));
+      tab_helper_->inline_install_status_->InlineInstallDownloadProgress(
+          percent_downloaded);
     }
   }
   void OnBeginCrxInstall(const ExtensionId& extension_id) override {
@@ -124,18 +123,13 @@ class TabHelper::InlineInstallObserver : public InstallObserver {
 
   void SendInstallStageChangedMessage(const ExtensionId& extension_id,
                                       api::webstore::InstallStage stage) {
-    if (observe_install_stage_ && extension_id == extension_id_) {
-      tab_helper_->Send(
-          new ExtensionMsg_InlineInstallStageChanged(routing_id_, stage));
-    }
+    if (observe_install_stage_ && extension_id == extension_id_)
+      tab_helper_->inline_install_status_->InlineInstallStageChanged(stage);
   }
 
   // The owning TabHelper (guaranteed to be valid).
   TabHelper* const tab_helper_;
 
-  // The routing id to use in sending IPC updates.
-  int routing_id_;
-
   // The id of the extension to observe.
   ExtensionId extension_id_;
 
@@ -148,6 +142,10 @@ class TabHelper::InlineInstallObserver : public InstallObserver {
   DISALLOW_COPY_AND_ASSIGN(InlineInstallObserver);
 };
 
+TabHelper::~TabHelper() {
+  RemoveScriptExecutionObserver(ActivityLog::GetInstance(profile_));
+}
+
 TabHelper::TabHelper(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       profile_(Profile::FromBrowserContext(web_contents->GetBrowserContext())),
@@ -161,6 +159,7 @@ TabHelper::TabHelper(content::WebContents* web_contents)
       extension_action_runner_(new ExtensionActionRunner(web_contents)),
       webstore_inline_installer_factory_(new WebstoreInlineInstallerFactory()),
       registry_observer_(this),
+      bindings_(web_contents, this),
       image_loader_ptr_factory_(this),
       weak_ptr_factory_(this) {
   // The ActiveTabPermissionManager requires a session ID; ensure this
@@ -192,10 +191,10 @@ TabHelper::TabHelper(content::WebContents* web_contents)
                  content::NOTIFICATION_LOAD_STOP,
                  content::Source<NavigationController>(
                      &web_contents->GetController()));
-}
 
-TabHelper::~TabHelper() {
-  RemoveScriptExecutionObserver(ActivityLog::GetInstance(profile_));
+  content::RenderFrameHost* host = web_contents->GetMainFrame();
+  if (host)
+    host->GetRemoteInterfaces()->GetInterface(&inline_install_status_);
 }
 
 void TabHelper::CreateApplicationShortcuts() {
@@ -366,8 +365,6 @@ bool TabHelper::OnMessageReceived(const IPC::Message& message,
                                   content::RenderFrameHost* render_frame_host) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(TabHelper, message, render_frame_host)
-    IPC_MESSAGE_HANDLER(ExtensionHostMsg_InlineWebstoreInstall,
-                        OnInlineWebstoreInstall)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_GetAppInstallState,
                         OnGetAppInstallState)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_ContentScriptsExecuting,
@@ -401,8 +398,7 @@ void TabHelper::OnDidGetWebApplicationInfo(const WebApplicationInfo& info) {
 #if !defined(OS_MACOSX)
     case CREATE_SHORTCUT: {
       chrome::ShowCreateWebAppShortcutsDialog(
-          web_contents()->GetTopLevelNativeWindow(),
-          web_contents());
+          web_contents()->GetTopLevelNativeWindow(), web_contents());
       break;
     }
 #endif
@@ -436,39 +432,39 @@ void TabHelper::OnDidGetWebApplicationInfo(const WebApplicationInfo& info) {
     pending_web_app_action_ = NONE;
 }
 
-void TabHelper::OnInlineWebstoreInstall(content::RenderFrameHost* host,
-                                        int install_id,
-                                        int return_route_id,
-                                        const std::string& webstore_item_id,
-                                        int listeners_mask) {
-  GURL requestor_url(host->GetLastCommittedURL());
-  // Check that the listener is reasonable. We should never get anything other
-  // than an install stage listener, a download listener, or both.
-  // The requestor_url should also be valid, and the renderer should disallow
-  // child frames from sending the IPC.
-  if ((listeners_mask & ~(api::webstore::INSTALL_STAGE_LISTENER |
-                          api::webstore::DOWNLOAD_PROGRESS_LISTENER)) != 0 ||
-      !requestor_url.is_valid() || requestor_url == url::kAboutBlankURL ||
-      host->GetParent()) {
-    NOTREACHED();
-    return;
-  }
+void TabHelper::DoInlineInstall(int install_id,
+                                int return_route_id,
+                                const std::string& webstore_item_id,
+                                int listeners_mask,
+                                const DoInlineInstallCallback& callback) {
+  content::RenderFrameHost* host = web_contents()->GetMainFrame();
+  if (bindings_.GetCurrentTargetFrame() == host) {

[Devlin, 2017/04/12 01:16:07]

We should handle the case of the message not being from the main frame (probably
with a notreached, since there should be renderer-side checks for this).

Also, when the full body of a method needs to be behind some check like this,
it's often cleaner to do:
if (!some_condition)
  return;
// lots of code here

rather than
if (some_condition) {
  // lots of code here
}

[catmullings, 2017/04/14 20:59:40]

Done.

+    GURL requestor_url(host->GetLastCommittedURL());
+    // Check that the listener is reasonable. We should never get anything other
+    // than an install stage listener, a download listener, or both.
+    // The requestor_url should also be valid, and the renderer should disallow
+    // child frames from sending the IPC.
+    if ((listeners_mask & ~(api::webstore::INSTALL_STAGE_LISTENER |
+                            api::webstore::DOWNLOAD_PROGRESS_LISTENER)) != 0 ||
+        !requestor_url.is_valid() || requestor_url == url::kAboutBlankURL ||
+        host->GetParent()) {

[Devlin, 2017/04/12 01:16:07]

if host is the main frame, it can't have a parent.

[catmullings, 2017/04/14 20:59:40]

Done.

+      NOTREACHED();
+      return;
+    }
 
-  if (pending_inline_installations_.count(webstore_item_id) != 0) {
-    Send(new ExtensionMsg_InlineWebstoreInstallResponse(
-        return_route_id, install_id, false,
-        webstore_install::kInstallInProgressError,
-        webstore_install::INSTALL_IN_PROGRESS));
-    return;
-  }
+    if (pending_inline_installations_.count(webstore_item_id) != 0) {
+      callback.Run(install_id, false, webstore_install::kInstallInProgressError,
+                   webstore_install::INSTALL_IN_PROGRESS);
+      return;
+    }
 
-  pending_inline_installations_.insert(webstore_item_id);
-  // Inform the Webstore API that an inline install is happening, in case the
-  // page requested status updates.
-  ExtensionRegistry* registry = ExtensionRegistry::Get(profile_);
-  if (registry->disabled_extensions().Contains(webstore_item_id) &&
-      (ExtensionPrefs::Get(profile_)->GetDisableReasons(webstore_item_id) &
-           Extension::DISABLE_PERMISSIONS_INCREASE) != 0) {
+    pending_inline_installations_.insert(webstore_item_id);
+    // Inform the Webstore API that an inline install is happening, in case the
+    // page requested status updates.
+    ExtensionRegistry* registry = ExtensionRegistry::Get(profile_);
+    if (registry->disabled_extensions().Contains(webstore_item_id) &&
+        (ExtensionPrefs::Get(profile_)->GetDisableReasons(webstore_item_id) &
+         Extension::DISABLE_PERMISSIONS_INCREASE) != 0) {
       // The extension was disabled due to permissions increase. Prompt for
       // re-enable.
       // TODO(devlin): We should also prompt for re-enable for other reasons,
@@ -480,30 +476,32 @@ void TabHelper::OnInlineWebstoreInstall(content::RenderFrameHost* host,
           web_contents(), requestor_url,
           base::Bind(&TabHelper::OnReenableComplete,
                      weak_ptr_factory_.GetWeakPtr(), install_id,
-                     return_route_id, webstore_item_id));
-  } else {
-    // TODO(devlin): We should adddress the case of the extension already
-    // being installed and enabled.
-    bool observe_download_progress =
-        (listeners_mask & api::webstore::DOWNLOAD_PROGRESS_LISTENER) != 0;
-    bool observe_install_stage =
-        (listeners_mask & api::webstore::INSTALL_STAGE_LISTENER) != 0;
-    if (observe_install_stage || observe_download_progress) {
-      DCHECK_EQ(0u, install_observers_.count(webstore_item_id));
-      install_observers_[webstore_item_id] =
-          base::MakeUnique<InlineInstallObserver>(
-              this, web_contents()->GetBrowserContext(), return_route_id,
-              webstore_item_id, observe_download_progress,
-              observe_install_stage);
+                     return_route_id, webstore_item_id, callback));
+    } else {
+      // TODO(devlin): We should adddress the case of the extension already
+      // being installed and enabled.
+      bool observe_download_progress =
+          (listeners_mask & api::webstore::DOWNLOAD_PROGRESS_LISTENER) != 0;
+      bool observe_install_stage =
+          (listeners_mask & api::webstore::INSTALL_STAGE_LISTENER) != 0;
+      if (observe_install_stage || observe_download_progress) {
+        DCHECK_EQ(0u, install_observers_.count(webstore_item_id));
+        install_observers_[webstore_item_id] =
+            base::MakeUnique<InlineInstallObserver>(
+                this, web_contents()->GetBrowserContext(), return_route_id,

[Devlin, 2017/04/12 01:16:07]

Does this compile?  InlineInstallObserver doesn't take a return_route_id param
anymore.

[catmullings, 2017/04/14 20:59:40]

It does compile. According to the constructor definition, it takes a routing_id
https://cs.chromium.org/chromium/src/chrome/browser/extensions/tab_helper.cc?...

I've removed the return_route_id from the InlineInstallObserver class, since
it's no longer needed.

+                webstore_item_id, observe_download_progress,
+                observe_install_stage);
+      }
+
+      WebstoreStandaloneInstaller::Callback completion_callback = base::Bind(
+          &TabHelper::OnInlineInstallComplete, weak_ptr_factory_.GetWeakPtr(),
+          callback, install_id, return_route_id, webstore_item_id);
+      scoped_refptr<WebstoreInlineInstaller> installer(
+          webstore_inline_installer_factory_->CreateInstaller(
+              web_contents(), host, webstore_item_id, requestor_url,
+              completion_callback));
+      installer->BeginInstall();
     }
-
-    WebstoreStandaloneInstaller::Callback callback = base::Bind(
-        &TabHelper::OnInlineInstallComplete, weak_ptr_factory_.GetWeakPtr(),
-        install_id, return_route_id, webstore_item_id);
-    scoped_refptr<WebstoreInlineInstaller> installer(
-        webstore_inline_installer_factory_->CreateInstaller(
-            web_contents(), host, webstore_item_id, requestor_url, callback));
-    installer->BeginInstall();
   }
 }
 
@@ -592,6 +590,7 @@ WindowController* TabHelper::GetExtensionWindowController() const  {
 void TabHelper::OnReenableComplete(int install_id,
                                    int return_route_id,
                                    const ExtensionId& extension_id,
+                                   const DoInlineInstallCallback& callback,
                                    ExtensionReenabler::ReenableResult result) {
   // Map the re-enable results to webstore-install results.
   webstore_install::Result webstore_result = webstore_install::SUCCESS;
@@ -613,7 +612,7 @@ void TabHelper::OnReenableComplete(int install_id,
       break;
   }
 
-  OnInlineInstallComplete(install_id, return_route_id, extension_id,
+  OnInlineInstallComplete(callback, install_id, return_route_id, extension_id,
                           result == ExtensionReenabler::REENABLE_SUCCESS, error,
                           webstore_result);
   // Note: ExtensionReenabler contained the callback with the curried-in
@@ -621,7 +620,8 @@ void TabHelper::OnReenableComplete(int install_id,
   extension_reenabler_.reset();
 }
 
-void TabHelper::OnInlineInstallComplete(int install_id,
+void TabHelper::OnInlineInstallComplete(const DoInlineInstallCallback& callback,
+                                        int install_id,
                                         int return_route_id,
                                         const ExtensionId& extension_id,
                                         bool success,
@@ -630,12 +630,7 @@ void TabHelper::OnInlineInstallComplete(int install_id,
   DCHECK_EQ(1u, pending_inline_installations_.count(extension_id));
   pending_inline_installations_.erase(extension_id);
   install_observers_.erase(extension_id);
-  Send(new ExtensionMsg_InlineWebstoreInstallResponse(
-      return_route_id,
-      install_id,
-      success,
-      success ? std::string() : error,
-      result));
+  callback.Run(install_id, success, success ? std::string() : error, result);
 }
 
 WebContents* TabHelper::GetAssociatedWebContents() const {