Convert ExtensionHostMsg_GetAppInstallState to a LocalFrameHost message

This CL converts ExtensionHostMsg_GetAppInstallState to a
extensions::mojom::LocalFrameHost message and it replies back
to the renderer with the callback instead of
ExtensionMsg_GetAppInstallStateResponse.

It cleans up //extensions/chrome_v8_extension_handler.{cc,h}
and AppHooksDelegate::IPCHelper.

Bug: 1185297
Change-Id: Ib72618426e54b1061abe6bb72f3a8479875149d6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2797148
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Julie Kim <jkim@igalia.com>
Cr-Commit-Position: refs/heads/master@{#870823}

chrome/browser/extensions/chrome_extension_frame_host.cc

@@ -5,6 +5,10 @@
 #include "chrome/browser/extensions/chrome_extension_frame_host.h"
 
 #include "chrome/browser/extensions/extension_action_runner.h"
+#include "chrome/common/extensions/extension_constants.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/common/extension_set.h"
+#include "url/gurl.h"
 
 namespace extensions {
 
@@ -29,4 +33,23 @@ void ChromeExtensionFrameHost::RequestScriptInjectionPermission(
                                              run_location, std::move(callback));
 }
 
+void ChromeExtensionFrameHost::GetAppInstallState(
+    const GURL& requestor_url,
+    GetAppInstallStateCallback callback) {
+  ExtensionRegistry* registry =
+      ExtensionRegistry::Get(web_contents_->GetBrowserContext());
+  const ExtensionSet& extensions = registry->enabled_extensions();
+  const ExtensionSet& disabled_extensions = registry->disabled_extensions();
+
+  std::string state;
+  if (extensions.GetHostedAppByURL(requestor_url))
+    state = extension_misc::kAppStateInstalled;
+  else if (disabled_extensions.GetHostedAppByURL(requestor_url))
+    state = extension_misc::kAppStateDisabled;
+  else
+    state = extension_misc::kAppStateNotInstalled;
+
+  std::move(callback).Run(state);
+}
+
 }  // namespace extensions

chrome/browser/extensions/chrome_extension_frame_host.h

@@ -14,6 +14,8 @@ namespace content {
 class WebContents;
 }
 
+class GURL;
+
 namespace extensions {
 
 class ChromeExtensionFrameHost : public ExtensionFrameHost {
@@ -29,6 +31,8 @@ class ChromeExtensionFrameHost : public ExtensionFrameHost {
       mojom::InjectionType script_type,
       mojom::RunLocation run_location,
       RequestScriptInjectionPermissionCallback callback) override;
+  void GetAppInstallState(const GURL& url,
+                          GetAppInstallStateCallback callback) override;
 
  private:
   // This raw pointer is safe to use because ExtensionWebContentsObserver whose

chrome/browser/extensions/tab_helper.cc

@@ -302,8 +302,6 @@ bool TabHelper::OnMessageReceived(const IPC::Message& message,
                                   content::RenderFrameHost* sender) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(TabHelper, message, sender)
-    IPC_MESSAGE_HANDLER(ExtensionHostMsg_GetAppInstallState,
-                        OnGetAppInstallState)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_ContentScriptsExecuting,
                         OnContentScriptsExecuting)
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -328,30 +326,6 @@ void TabHelper::WebContentsDestroyed() {
   });
 }
 
-void TabHelper::OnGetAppInstallState(content::RenderFrameHost* host,
-                                     const GURL& requestor_url,
-                                     int return_route_id,
-                                     int callback_id) {
-  ExtensionRegistry* registry =
-      ExtensionRegistry::Get(web_contents()->GetBrowserContext());
-  const ExtensionSet& extensions = registry->enabled_extensions();
-  const ExtensionSet& disabled_extensions = registry->disabled_extensions();
-
-  std::string state;
-  if (extensions.GetHostedAppByURL(requestor_url))
-    state = extension_misc::kAppStateInstalled;
-  else if (disabled_extensions.GetHostedAppByURL(requestor_url))
-    state = extension_misc::kAppStateDisabled;
-  else
-    state = extension_misc::kAppStateNotInstalled;
-
-  // We use the |host| to send the message because using
-  // WebContentsObserver::Send() defaults to using the main RenderView, which
-  // might be in a different process if the request came from a frame.
-  host->Send(new ExtensionMsg_GetAppInstallStateResponse(return_route_id, state,
-                                                         callback_id));
-}
-
 void TabHelper::OnContentScriptsExecuting(
     content::RenderFrameHost* host,
     const ExecutingScriptsMap& executing_scripts_map,

chrome/browser/extensions/tab_helper.h

@@ -120,10 +120,6 @@ class TabHelper : public content::WebContentsObserver,
                            UnloadedExtensionReason reason) override;
 
   // Message handlers.
-  void OnGetAppInstallState(content::RenderFrameHost* host,
-                            const GURL& requestor_url,
-                            int return_route_id,
-                            int callback_id);
   void OnContentScriptsExecuting(content::RenderFrameHost* host,
                                  const ExecutingScriptsMap& extension_ids,
                                  const GURL& on_url);

chrome/renderer/BUILD.gn

@@ -294,8 +294,6 @@ static_library("renderer") {
       "extensions/chrome_extensions_dispatcher_delegate.h",
       "extensions/chrome_extensions_renderer_client.cc",
       "extensions/chrome_extensions_renderer_client.h",
-      "extensions/chrome_v8_extension_handler.cc",
-      "extensions/chrome_v8_extension_handler.h",
       "extensions/extension_hooks_delegate.cc",
       "extensions/extension_hooks_delegate.h",
       "extensions/extension_localization_peer.cc",

chrome/renderer/extensions/app_hooks_delegate.cc

@@ -18,6 +18,7 @@
 #include "extensions/renderer/bindings/api_request_handler.h"
 #include "extensions/renderer/bindings/api_signature.h"
 #include "extensions/renderer/dispatcher.h"
+#include "extensions/renderer/extension_frame_helper.h"
 #include "extensions/renderer/renderer_extension_registry.h"
 #include "extensions/renderer/script_context.h"
 #include "extensions/renderer/script_context_set.h"
@@ -52,40 +53,10 @@ void IsInstalledGetterCallback(
 
 }  // namespace
 
-AppHooksDelegate::IPCHelper::IPCHelper(AppHooksDelegate* owner)
-    : owner_(owner) {}
-AppHooksDelegate::IPCHelper::~IPCHelper() = default;
-
-void AppHooksDelegate::IPCHelper::SendGetAppInstallStateMessage(
-    content::RenderFrame* render_frame,
-    const GURL& url,
-    int request_id) {
-  Send(new ExtensionHostMsg_GetAppInstallState(
-      render_frame->GetRoutingID(), url, GetRoutingID(), request_id));
-}
-
-bool AppHooksDelegate::IPCHelper::OnMessageReceived(
-    const IPC::Message& message) {
-  IPC_BEGIN_MESSAGE_MAP(AppHooksDelegate::IPCHelper, message)
-    IPC_MESSAGE_HANDLER(ExtensionMsg_GetAppInstallStateResponse,
-                        OnAppInstallStateResponse)
-    IPC_MESSAGE_UNHANDLED(CHECK(false) << "Unhandled IPC message")
-  IPC_END_MESSAGE_MAP()
-  return true;
-}
-
-void AppHooksDelegate::IPCHelper::OnAppInstallStateResponse(
-    const std::string& state,
-    int request_id) {
-  owner_->OnAppInstallStateResponse(state, request_id);
-}
-
 AppHooksDelegate::AppHooksDelegate(Dispatcher* dispatcher,
                                    APIRequestHandler* request_handler)
-    : dispatcher_(dispatcher),
+    : dispatcher_(dispatcher), request_handler_(request_handler) {}
-      request_handler_(request_handler),
+AppHooksDelegate::~AppHooksDelegate() = default;
-      ipc_helper_(this) {}
-AppHooksDelegate::~AppHooksDelegate() {}
 
 bool AppHooksDelegate::GetIsInstalled(ScriptContext* script_context) const {
   const Extension* extension = script_context->extension();
@@ -186,9 +157,12 @@ void AppHooksDelegate::GetInstallState(ScriptContext* script_context,
   content::RenderFrame* render_frame = script_context->GetRenderFrame();
   CHECK(render_frame);
 
-  ipc_helper_.SendGetAppInstallStateMessage(
+  ExtensionFrameHelper::Get(render_frame)
-      render_frame, script_context->web_frame()->GetDocument().Url(),
+      ->GetLocalFrameHost()
-      request_id);
+      ->GetAppInstallState(
+          script_context->web_frame()->GetDocument().Url(),
+          base::BindOnce(&AppHooksDelegate::OnAppInstallStateResponse,
+                         weak_factory_.GetWeakPtr(), request_id));
 }
 
 const char* AppHooksDelegate::GetRunningState(
@@ -225,8 +199,8 @@ const char* AppHooksDelegate::GetRunningState(
   return state;
 }
 
-void AppHooksDelegate::OnAppInstallStateResponse(const std::string& state,
+void AppHooksDelegate::OnAppInstallStateResponse(int request_id,
-                                                 int request_id) {
+                                                 const std::string& state) {
   // Note: it's kind of lame that we serialize the install state to a
   // base::Value here when we're just going to later convert it to v8, but it's
   // not worth the specialization on APIRequestHandler for this oddball API.

chrome/renderer/extensions/app_hooks_delegate.h

@@ -8,16 +8,10 @@
 #include <string>
 
 #include "base/macros.h"
-#include "chrome/renderer/extensions/chrome_v8_extension_handler.h"
+#include "base/memory/weak_ptr.h"
 #include "extensions/renderer/bindings/api_binding_hooks_delegate.h"
 #include "v8/include/v8.h"
 
-class GURL;
-
-namespace content {
-class RenderFrame;
-}
-
 namespace extensions {
 class APIRequestHandler;
 class Dispatcher;
@@ -46,32 +40,6 @@ class AppHooksDelegate : public APIBindingHooksDelegate {
   bool GetIsInstalled(ScriptContext* script_context) const;
 
  private:
-  // A helper class to handle IPC message sending/receiving. Isolated from
-  // AppHooksDelegate to avoid multiple inheritence.
-  class IPCHelper : public ChromeV8ExtensionHandler {
-   public:
-    explicit IPCHelper(AppHooksDelegate* owner);
-    ~IPCHelper() override;
-
-    // Sends the IPC message to the browser to get the install state of the
-    // app.
-    void SendGetAppInstallStateMessage(content::RenderFrame* render_frame,
-                                       const GURL& url,
-                                       int request_id);
-
-   private:
-    // IPC::Listener:
-    bool OnMessageReceived(const IPC::Message& message) override;
-
-    // Handle for ExtensionMsg_GetAppInstallStateResponse; just forwards to
-    // AppHooksDelegate.
-    void OnAppInstallStateResponse(const std::string& state, int request_id);
-
-    AppHooksDelegate* owner_ = nullptr;
-
-    DISALLOW_COPY_AND_ASSIGN(IPCHelper);
-  };
-
   // Returns the manifest of the extension associated with the frame.
   v8::Local<v8::Value> GetDetails(ScriptContext* script_context) const;
 
@@ -85,15 +53,15 @@ class AppHooksDelegate : public APIBindingHooksDelegate {
   // for the extension associated with the frame of the script context.
   const char* GetRunningState(ScriptContext* script_context) const;
 
-  // Handle for ExtensionMsg_GetAppInstallStateResponse.
+  // Handles the reply from GetInstallState().
-  void OnAppInstallStateResponse(const std::string& state, int request_id);
+  void OnAppInstallStateResponse(int request_id, const std::string& state);
 
   // Dispatcher handle. Not owned.
   Dispatcher* dispatcher_ = nullptr;
 
   APIRequestHandler* request_handler_ = nullptr;
 
-  IPCHelper ipc_helper_;
+  base::WeakPtrFactory<AppHooksDelegate> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(AppHooksDelegate);
 };

chrome/renderer/extensions/chrome_v8_extension_handler.cc

@@ -1,35 +0,0 @@
-// 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/chrome_v8_extension_handler.h"
-
-#include "content/public/renderer/render_thread.h"
-
-using content::RenderThread;
-
-namespace extensions {
-
-ChromeV8ExtensionHandler::ChromeV8ExtensionHandler()
-    : routing_id_(MSG_ROUTING_NONE) {
-}
-
-ChromeV8ExtensionHandler::~ChromeV8ExtensionHandler() {
-  if (routing_id_ != MSG_ROUTING_NONE)
-    RenderThread::Get()->RemoveRoute(routing_id_);
-}
-
-int ChromeV8ExtensionHandler::GetRoutingID() {
-  if (routing_id_ == MSG_ROUTING_NONE) {
-    routing_id_ = RenderThread::Get()->GenerateRoutingID();
-    RenderThread::Get()->AddRoute(routing_id_, this);
-  }
-
-  return routing_id_;
-}
-
-void ChromeV8ExtensionHandler::Send(IPC::Message* message) {
-  RenderThread::Get()->Send(message);
-}
-
-}  // namespace extensions

chrome/renderer/extensions/chrome_v8_extension_handler.h

@@ -1,38 +0,0 @@
-// 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.
-
-#ifndef CHROME_RENDERER_EXTENSIONS_CHROME_V8_EXTENSION_HANDLER_H_
-#define CHROME_RENDERER_EXTENSIONS_CHROME_V8_EXTENSION_HANDLER_H_
-
-#include <string>
-
-#include "base/macros.h"
-#include "ipc/ipc_listener.h"
-#include "v8/include/v8.h"
-
-namespace extensions {
-
-// Base class for context-scoped handlers used with ChromeV8Extension.
-// TODO(koz): Rename/refactor this somehow. Maybe just pull it into
-// ChromeV8Extension.
-class ChromeV8ExtensionHandler : public IPC::Listener {
- public:
-  ~ChromeV8ExtensionHandler() override;
-
-  // IPC::Listener
-  bool OnMessageReceived(const IPC::Message& message) override = 0;
-
- protected:
-  ChromeV8ExtensionHandler();
-  int GetRoutingID();
-  void Send(IPC::Message* message);
-
- private:
-  int routing_id_;
-  DISALLOW_COPY_AND_ASSIGN(ChromeV8ExtensionHandler);
-};
-
-}  // namespace extensions
-
-#endif  // CHROME_RENDERER_EXTENSIONS_CHROME_V8_EXTENSION_HANDLER_H_

extensions/browser/extension_frame_host.cc

@@ -4,6 +4,8 @@
 
 #include "extensions/browser/extension_frame_host.h"
 
+#include <string>
+
 namespace extensions {
 
 ExtensionFrameHost::ExtensionFrameHost(content::WebContents* web_contents)
@@ -19,4 +21,10 @@ void ExtensionFrameHost::RequestScriptInjectionPermission(
   std::move(callback).Run(false);
 }
 
+void ExtensionFrameHost::GetAppInstallState(
+    const GURL& requestor_url,
+    GetAppInstallStateCallback callback) {
+  std::move(callback).Run(std::string());
+}
+
 }  // namespace extensions

extensions/browser/extension_frame_host.h

@@ -32,6 +32,8 @@ class ExtensionFrameHost : public mojom::LocalFrameHost {
       mojom::InjectionType script_type,
       mojom::RunLocation run_location,
       RequestScriptInjectionPermissionCallback callback) override;
+  void GetAppInstallState(const GURL& requestor_url,
+                          GetAppInstallStateCallback callback) override;
 
  private:
   content::WebContentsFrameReceiverSet<mojom::LocalFrameHost> receivers_;

extensions/common/extension_messages.h

@@ -509,11 +509,6 @@ IPC_MESSAGE_CONTROL2(ExtensionMsg_WakeEventPageResponse,
                      int /* request_id */,
                      bool /* success */)
 
-// Response to the renderer for ExtensionHostMsg_GetAppInstallState.
-IPC_MESSAGE_ROUTED2(ExtensionMsg_GetAppInstallStateResponse,
-                    std::string /* state */,
-                    int32_t /* callback_id */)
-
 // Check whether the Port for extension messaging exists in a frame or a Service
 // Worker. If the port ID is unknown, the frame replies with
 // ExtensionHostMsg_CloseMessagePort.
@@ -676,12 +671,6 @@ IPC_MESSAGE_ROUTED2(ExtensionHostMsg_ContentScriptsExecuting,
                     ExecutingScriptsMap,
                     GURL /* url of the _topmost_ frame */)
 
-// Sent by the renderer when a web page is checking if its app is installed.
-IPC_MESSAGE_ROUTED3(ExtensionHostMsg_GetAppInstallState,
-                    GURL /* requestor_url */,
-                    int32_t /* return_route_id */,
-                    int32_t /* callback_id */)
-
 // Optional Ack message sent to the browser to notify that the response to a
 // function has been processed.
 IPC_MESSAGE_ROUTED1(ExtensionHostMsg_ResponseAck,

extensions/common/mojom/frame.mojom

@@ -127,4 +127,8 @@ interface LocalFrameHost {
   RequestScriptInjectionPermission(string extension_id,
                       InjectionType script_type,
                       RunLocation run_location) => (bool granted);
+
+  // Gets the install state for the app when a web page is checking if its app
+  // is installed. The reply is sent back to the renderer with |state|.
+  GetAppInstallState(url.mojom.Url url) => (string state);
 };