Convert webapplicationinfo messages to use mojo.

This patch mojofies the ChromeFrameMsg_GetWebApplicationInfo and ChromeFrameHostMsg_DidGetWebApplicationInfo message Bug: 789826 Change-Id: Iff07e66d41f56f85e9298fc8e2ba38ab00f3e734 Reviewed-on: https://chromium-review.googlesource.com/800130 Commit-Queue: Chandramouli Sanchi <cm.sanchi@samsung.com> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Dominick Ng <dominickn@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Cr-Commit-Position: refs/heads/master@{#524674}
FairyWorld · Dec 18, 2017 · 2b601c3 · 2b601c3
1 parent 9f78b66
commit 2b601c3
Show file tree

Hide file tree

Showing 15 changed files with 121 additions and 130 deletions.
diff --git a/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc b/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.cc
@@ -17,7 +17,6 @@
 #include "chrome/browser/installable/installable_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_constants.h"
-#include "chrome/common/render_messages.h"
 #include "chrome/common/web_application_info.h"
 #include "components/dom_distiller/core/url_utils.h"
 #include "components/favicon/core/favicon_service.h"
@@ -27,6 +26,7 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/manifest.h"
+#include "third_party/WebKit/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/WebKit/public/platform/modules/screen_orientation/WebScreenOrientationLockType.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/favicon_size.h"
@@ -111,22 +111,27 @@ AddToHomescreenDataFetcher::AddToHomescreenDataFetcher(
       shortcut_info_(GetShortcutUrl(web_contents)),
       data_timeout_ms_(base::TimeDelta::FromMilliseconds(data_timeout_ms)),
       check_webapk_compatibility_(check_webapk_compatibility),
-      is_waiting_for_web_application_info_(true),
       is_waiting_for_manifest_(true),
       weak_ptr_factory_(this) {
   DCHECK(shortcut_info_.url.is_valid());
 
   // Send a message to the renderer to retrieve information about the page.
-  content::RenderFrameHost* main_frame = web_contents->GetMainFrame();
-  main_frame->Send(
-      new ChromeFrameMsg_GetWebApplicationInfo(main_frame->GetRoutingID()));
+  chrome::mojom::ChromeRenderFrameAssociatedPtr chrome_render_frame;
+  web_contents->GetMainFrame()->GetRemoteAssociatedInterfaces()->GetInterface(
+      &chrome_render_frame);
+  // Bind the InterfacePtr into the callback so that it's kept alive
+  // until there's either a connection error or a response.
+  auto* web_app_info_proxy = chrome_render_frame.get();
+  web_app_info_proxy->GetWebApplicationInfo(
+      base::Bind(&AddToHomescreenDataFetcher::OnDidGetWebApplicationInfo,
+                 base::Unretained(this), base::Passed(&chrome_render_frame)));
 }
 
 AddToHomescreenDataFetcher::~AddToHomescreenDataFetcher() {}
 
 void AddToHomescreenDataFetcher::OnDidGetWebApplicationInfo(
+    chrome::mojom::ChromeRenderFrameAssociatedPtr chrome_render_frame,
     const WebApplicationInfo& received_web_app_info) {
-  is_waiting_for_web_application_info_ = false;
   if (!web_contents())
     return;
 
@@ -179,23 +184,6 @@ void AddToHomescreenDataFetcher::OnDidGetWebApplicationInfo(
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
-bool AddToHomescreenDataFetcher::OnMessageReceived(
-    const IPC::Message& message,
-    content::RenderFrameHost* sender) {
-  if (!is_waiting_for_web_application_info_)
-    return false;
-
-  bool handled = true;
-
-  IPC_BEGIN_MESSAGE_MAP(AddToHomescreenDataFetcher, message)
-    IPC_MESSAGE_HANDLER(ChromeFrameHostMsg_DidGetWebApplicationInfo,
-                        OnDidGetWebApplicationInfo)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-
-  return handled;
-}
-
 void AddToHomescreenDataFetcher::StopTimer() {
   data_timeout_timer_.Stop();
   RecordAddToHomescreenDialogDuration(base::TimeTicks::Now() - start_time_);

diff --git a/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.h b/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.h
@@ -14,6 +14,7 @@
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/android/shortcut_info.h"
+#include "chrome/common/chrome_render_frame.mojom.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
@@ -62,18 +63,16 @@ class AddToHomescreenDataFetcher : public content::WebContentsObserver {
   ~AddToHomescreenDataFetcher() override;
 
   // IPC message received when the initialization is finished.
-  void OnDidGetWebApplicationInfo(const WebApplicationInfo& web_app_info);
+  void OnDidGetWebApplicationInfo(
+      chrome::mojom::ChromeRenderFrameAssociatedPtr chrome_render_frame,
+      const WebApplicationInfo& web_app_info);
 
   // Accessors, etc.
   const SkBitmap& badge_icon() const { return badge_icon_; }
   const SkBitmap& primary_icon() const { return primary_icon_; }
   ShortcutInfo& shortcut_info() { return shortcut_info_; }
 
  private:
-  // WebContentsObserver:
-  bool OnMessageReceived(const IPC::Message& message,
-                         content::RenderFrameHost* sender) override;
-
   // Called to stop the timeout timer.
   void StopTimer();
 
@@ -114,7 +113,6 @@ class AddToHomescreenDataFetcher : public content::WebContentsObserver {
 
   // Indicates whether to check WebAPK compatibility.
   bool check_webapk_compatibility_;
-  bool is_waiting_for_web_application_info_;
   bool is_waiting_for_manifest_;
 
   base::WeakPtrFactory<AddToHomescreenDataFetcher> weak_ptr_factory_;

diff --git a/chrome/browser/android/webapps/add_to_homescreen_data_fetcher_unittest.cc b/chrome/browser/android/webapps/add_to_homescreen_data_fetcher_unittest.cc
@@ -267,7 +267,7 @@ class AddToHomescreenDataFetcherTest : public ChromeRenderViewHostTestHarness {
     WebApplicationInfo web_application_info;
     web_application_info.title = base::UTF8ToUTF16(kWebApplicationInfoTitle);
 
-    fetcher->OnDidGetWebApplicationInfo(web_application_info);
+    fetcher->OnDidGetWebApplicationInfo(nullptr, web_application_info);
     waiter.WaitForDataAvailable();
 
     EXPECT_EQ(check_webapk_compatibility(),

diff --git a/chrome/browser/extensions/bookmark_app_helper_browsertest.cc b/chrome/browser/extensions/bookmark_app_helper_browsertest.cc
@@ -13,23 +13,18 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/test/test_browser_dialog.h"
 #include "chrome/common/chrome_features.h"
-#include "chrome/common/render_messages.h"
+#include "chrome/common/chrome_render_frame.mojom.h"
 #include "content/public/browser/browser_message_filter.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
+#include "third_party/WebKit/common/associated_interfaces/associated_interface_provider.h"
 
 namespace extensions {
 namespace {
 
-content::RenderWidgetHost* GetActiveRenderWidgetHost(Browser* browser) {
-  return browser->tab_strip_model()
-      ->GetActiveWebContents()
-      ->GetRenderWidgetHostView()
-      ->GetRenderWidgetHost();
-}
-
 // Extends BookmarkAppHelper to see the call to OnIconsDownloaded.
 class TestBookmarkAppHelper : public BookmarkAppHelper {
  public:
@@ -54,20 +49,21 @@ class TestBookmarkAppHelper : public BookmarkAppHelper {
   DISALLOW_COPY_AND_ASSIGN(TestBookmarkAppHelper);
 };
 
-// Intercepts the ChromeFrameHostMsg_DidGetWebApplicationInfo that would usually
-// get sent to extensions::TabHelper to create a BookmarkAppHelper that lets us
-// detect when icons are downloaded and the dialog is ready to show.
-class WebAppReadyMsgWatcher : public content::BrowserMessageFilter {
+}  // namespace
+
+class BookmarkAppHelperTest : public DialogBrowserTest {
  public:
-  explicit WebAppReadyMsgWatcher(Browser* browser)
-      : BrowserMessageFilter(ChromeMsgStart), browser_(browser) {}
+  BookmarkAppHelperTest() {}
 
   content::WebContents* web_contents() {
-    return browser_->tab_strip_model()->GetActiveWebContents();
+    return browser()->tab_strip_model()->GetActiveWebContents();
   }
 
-  void OnDidGetWebApplicationInfo(const WebApplicationInfo& const_info) {
+  void OnDidGetWebApplicationInfo(
+      chrome::mojom::ChromeRenderFrameAssociatedPtr chrome_render_frame,
+      const WebApplicationInfo& const_info) {
     WebApplicationInfo info = const_info;
+
     // Mimic extensions::TabHelper for fields missing from the manifest.
     if (info.app_url.is_empty())
       info.app_url = web_contents()->GetURL();
@@ -77,9 +73,10 @@ class WebAppReadyMsgWatcher : public content::BrowserMessageFilter {
       info.title = base::UTF8ToUTF16(info.app_url.spec());
 
     bookmark_app_helper_ = base::MakeUnique<TestBookmarkAppHelper>(
-        browser_->profile(), info, web_contents(), quit_closure_);
+        browser()->profile(), info, web_contents(), quit_closure_);
     bookmark_app_helper_->Create(
-        base::Bind(&WebAppReadyMsgWatcher::FinishCreateBookmarkApp, this));
+        base::Bind(&BookmarkAppHelperTest::FinishCreateBookmarkApp,
+                   base::Unretained(this)));
   }
 
   void FinishCreateBookmarkApp(const Extension* extension,
@@ -94,40 +91,6 @@ class WebAppReadyMsgWatcher : public content::BrowserMessageFilter {
     quit_closure_ = run_loop.QuitClosure();
     run_loop.Run();
   }
-
-  // BrowserMessageFilter:
-  void OverrideThreadForMessage(const IPC::Message& message,
-                                content::BrowserThread::ID* thread) override {
-    if (message.type() == ChromeFrameHostMsg_DidGetWebApplicationInfo::ID)
-      *thread = content::BrowserThread::UI;
-  }
-
-  bool OnMessageReceived(const IPC::Message& message) override {
-    bool handled = true;
-    IPC_BEGIN_MESSAGE_MAP(WebAppReadyMsgWatcher, message)
-      IPC_MESSAGE_HANDLER(ChromeFrameHostMsg_DidGetWebApplicationInfo,
-                          OnDidGetWebApplicationInfo)
-      IPC_MESSAGE_UNHANDLED(handled = false)
-    IPC_END_MESSAGE_MAP()
-    return handled;
-  }
-
- private:
-  ~WebAppReadyMsgWatcher() override {}
-
-  Browser* browser_;
-  base::Closure quit_closure_;
-  std::unique_ptr<TestBookmarkAppHelper> bookmark_app_helper_;
-
-  DISALLOW_COPY_AND_ASSIGN(WebAppReadyMsgWatcher);
-};
-
-}  // namespace
-
-class BookmarkAppHelperTest : public DialogBrowserTest {
- public:
-  BookmarkAppHelperTest() {}
-
   // DialogBrowserTest:
   void ShowDialog(const std::string& name) override {
     ASSERT_TRUE(embedded_test_server()->Start());
@@ -138,14 +101,25 @@ class BookmarkAppHelperTest : public DialogBrowserTest {
     AddTabAtIndex(1, GURL(embedded_test_server()->GetURL(path)),
                   ui::PAGE_TRANSITION_LINK);
 
-    scoped_refptr<WebAppReadyMsgWatcher> filter =
-        new WebAppReadyMsgWatcher(browser());
-    GetActiveRenderWidgetHost(browser())->GetProcess()->AddFilter(filter.get());
-    chrome::ExecuteCommand(browser(), IDC_CREATE_HOSTED_APP);
-    filter->Wait();
+    chrome::mojom::ChromeRenderFrameAssociatedPtr chrome_render_frame;
+    browser()
+        ->tab_strip_model()
+        ->GetActiveWebContents()
+        ->GetMainFrame()
+        ->GetRemoteAssociatedInterfaces()
+        ->GetInterface(&chrome_render_frame);
+    // Bind the InterfacePtr into the callback so that it's kept alive
+    // until there's either a connection error or a response.
+    auto* web_app_info_proxy = chrome_render_frame.get();
+    web_app_info_proxy->GetWebApplicationInfo(
+        base::Bind(&BookmarkAppHelperTest::OnDidGetWebApplicationInfo,
+                   base::Unretained(this), base::Passed(&chrome_render_frame)));
+    Wait();
   }
 
  private:
+  base::Closure quit_closure_;
+  std::unique_ptr<TestBookmarkAppHelper> bookmark_app_helper_;
   DISALLOW_COPY_AND_ASSIGN(BookmarkAppHelperTest);
 };
 

diff --git a/chrome/browser/extensions/tab_helper.cc b/chrome/browser/extensions/tab_helper.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/extensions/tab_helper.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
@@ -61,6 +62,7 @@
 #include "extensions/common/feature_switch.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
+#include "third_party/WebKit/common/associated_interfaces/associated_interface_provider.h"
 #include "url/url_constants.h"
 
 #if defined(OS_WIN)
@@ -322,8 +324,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(ChromeFrameHostMsg_DidGetWebApplicationInfo,
-                        OnDidGetWebApplicationInfo)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_GetAppInstallState,
                         OnGetAppInstallState)
     IPC_MESSAGE_HANDLER(ExtensionHostMsg_ContentScriptsExecuting,
@@ -344,8 +344,9 @@ void TabHelper::DidCloneToNewWebContents(WebContents* old_web_contents,
   new_helper->extension_app_icon_ = extension_app_icon_;
 }
 
-void TabHelper::OnDidGetWebApplicationInfo(content::RenderFrameHost* sender,
-                                           const WebApplicationInfo& info) {
+void TabHelper::OnDidGetWebApplicationInfo(
+    chrome::mojom::ChromeRenderFrameAssociatedPtr chrome_render_frame,
+    const WebApplicationInfo& info) {
   web_app_info_ = info;
 
   NavigationEntry* entry =
@@ -591,12 +592,21 @@ void TabHelper::GetApplicationInfo(WebAppAction action) {
   if (!entry)
     return;
 
+  // This DCHECK added to ensure this function is called only once.
+  DCHECK_EQ(pending_web_app_action_, NONE);
+
   pending_web_app_action_ = action;
   last_committed_nav_entry_unique_id_ = entry->GetUniqueID();
 
-  content::RenderFrameHost* main_frame = web_contents()->GetMainFrame();
-  main_frame->Send(
-      new ChromeFrameMsg_GetWebApplicationInfo(main_frame->GetRoutingID()));
+  chrome::mojom::ChromeRenderFrameAssociatedPtr chrome_render_frame;
+  web_contents()->GetMainFrame()->GetRemoteAssociatedInterfaces()->GetInterface(
+      &chrome_render_frame);
+  // Bind the InterfacePtr into the callback so that it's kept alive
+  // until there's either a connection error or a response.
+  auto* web_app_info_proxy = chrome_render_frame.get();
+  web_app_info_proxy->GetWebApplicationInfo(
+      base::Bind(&TabHelper::OnDidGetWebApplicationInfo, base::Unretained(this),
+                 base::Passed(&chrome_render_frame)));
 }
 
 void TabHelper::SetTabId(content::RenderFrameHost* render_frame_host) {

diff --git a/chrome/browser/extensions/tab_helper.h b/chrome/browser/extensions/tab_helper.h
@@ -16,6 +16,7 @@
 #include "base/scoped_observer.h"
 #include "chrome/browser/extensions/active_tab_permission_granter.h"
 #include "chrome/browser/extensions/extension_reenabler.h"
+#include "chrome/common/chrome_render_frame.mojom.h"
 #include "chrome/common/extensions/mojom/inline_install.mojom.h"
 #include "chrome/common/extensions/webstore_install_result.h"
 #include "chrome/common/web_application_info.h"
@@ -154,8 +155,9 @@ class TabHelper : public content::WebContentsObserver,
       DoInlineInstallCallback callback) override;
 
   // Message handlers.
-  void OnDidGetWebApplicationInfo(content::RenderFrameHost* sender,
-                                  const WebApplicationInfo& info);
+  void OnDidGetWebApplicationInfo(
+      chrome::mojom::ChromeRenderFrameAssociatedPtr chrome_render_frame,
+      const WebApplicationInfo& info);
   void OnGetAppInstallState(content::RenderFrameHost* host,
                             const GURL& requestor_url,
                             int return_route_id,

diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc b/chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc
@@ -41,6 +41,7 @@
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_render_frame.mojom.h"
 #include "chrome/common/render_messages.h"
+#include "chrome/common/web_application_info.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/search_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"

diff --git a/chrome/common/BUILD.gn b/chrome/common/BUILD.gn
@@ -187,6 +187,7 @@ static_library("common") {
     "url_constants.h",
     "web_application_info.cc",
     "web_application_info.h",
+    "web_application_info_provider_param_traits.h",
     "webui_url_constants.cc",
     "webui_url_constants.h",
   ]

diff --git a/chrome/common/chrome_render_frame.mojom b/chrome/common/chrome_render_frame.mojom
@@ -13,6 +13,9 @@ enum ImageFormat {
   PNG,
 };
 
+[Native]
+struct WebApplicationInfo;
+
 // Messages sent from chrome to the render frame.
 interface ChromeRenderFrame {
 
@@ -41,4 +44,7 @@ interface ChromeRenderFrame {
 
   // Sent when the profile changes the kSafeBrowsingEnabled preference.
   SetClientSidePhishingDetection(bool enable_phishing_detection);
+
+  // Requests the web application info from the renderer.
+  GetWebApplicationInfo() => (WebApplicationInfo web_application_info);
 };