Convert AwViewHostMsg_ShouldOverrideUrlLoading to Mojo

This CL migrates AwViewHostMsg_ShouldOverrideUrlLoading
to FrameHost interface. This CL moves the handler of
the existing IPC message from AwContentsMessageFilter
to AwRenderViewHostExt.

Bug: 1157131
Change-Id: I29ab375d5502ab3af465ffcca769dfb3980d5843
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2592534Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
Cr-Commit-Position: refs/heads/master@{#838431}

android_webview/browser/aw_content_browser_client.cc

@@ -136,16 +136,8 @@ class AwContentsMessageFilter : public content::BrowserMessageFilter {
   explicit AwContentsMessageFilter(int process_id);
 
   // BrowserMessageFilter methods.
-  void OverrideThreadForMessage(const IPC::Message& message,
-                                BrowserThread::ID* thread) override;
   bool OnMessageReceived(const IPC::Message& message) override;
 
-  void OnShouldOverrideUrlLoading(int routing_id,
-                                  const base::string16& url,
-                                  bool has_user_gesture,
-                                  bool is_redirect,
-                                  bool is_main_frame,
-                                  bool* ignore_navigation);
   void OnSubFrameCreated(int parent_render_frame_id, int child_render_frame_id);
 
  private:
@@ -161,49 +153,15 @@ AwContentsMessageFilter::AwContentsMessageFilter(int process_id)
 
 AwContentsMessageFilter::~AwContentsMessageFilter() = default;
 
-void AwContentsMessageFilter::OverrideThreadForMessage(
-    const IPC::Message& message,
-    BrowserThread::ID* thread) {
-  if (message.type() == AwViewHostMsg_ShouldOverrideUrlLoading::ID) {
-    *thread = BrowserThread::UI;
-  }
-}
-
 bool AwContentsMessageFilter::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(AwContentsMessageFilter, message)
-    IPC_MESSAGE_HANDLER(AwViewHostMsg_ShouldOverrideUrlLoading,
-                        OnShouldOverrideUrlLoading)
     IPC_MESSAGE_HANDLER(AwViewHostMsg_SubFrameCreated, OnSubFrameCreated)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
-void AwContentsMessageFilter::OnShouldOverrideUrlLoading(
-    int render_frame_id,
-    const base::string16& url,
-    bool has_user_gesture,
-    bool is_redirect,
-    bool is_main_frame,
-    bool* ignore_navigation) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  *ignore_navigation = false;
-  AwContentsClientBridge* client =
-      AwContentsClientBridge::FromID(process_id_, render_frame_id);
-  if (client) {
-    if (!client->ShouldOverrideUrlLoading(url, has_user_gesture, is_redirect,
-                                          is_main_frame, ignore_navigation)) {
-      // If the shouldOverrideUrlLoading call caused a java exception we should
-      // always return immediately here!
-      return;
-    }
-  } else {
-    LOG(WARNING) << "Failed to find the associated render view host for url: "
-                 << url;
-  }
-}
-
 void AwContentsMessageFilter::OnSubFrameCreated(int parent_render_frame_id,
                                                 int child_render_frame_id) {
   AwContentsIoThreadClient::SubFrameCreated(process_id_, parent_render_frame_id,

android_webview/browser/renderer_host/aw_render_view_host_ext.cc

@@ -5,12 +5,15 @@
 #include "android_webview/browser/renderer_host/aw_render_view_host_ext.h"
 
 #include "android_webview/browser/aw_browser_context.h"
+#include "android_webview/browser/aw_contents_client_bridge.h"
 #include "android_webview/common/render_view_messages.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/logging.h"
+#include "content/public/browser/browser_task_traits.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
@@ -20,6 +23,36 @@
 
 namespace android_webview {
 
+namespace {
+
+void ShouldOverrideUrlLoadingOnUI(
+    content::WebContents* web_contents,
+    const base::string16& url,
+    bool has_user_gesture,
+    bool is_redirect,
+    bool is_main_frame,
+    mojom::FrameHost::ShouldOverrideUrlLoadingCallback callback) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  bool ignore_navigation = false;
+  AwContentsClientBridge* client =
+      AwContentsClientBridge::FromWebContents(web_contents);
+  if (client) {
+    if (!client->ShouldOverrideUrlLoading(url, has_user_gesture, is_redirect,
+                                          is_main_frame, &ignore_navigation)) {
+      // If the shouldOverrideUrlLoading call caused a java exception we should
+      // always return immediately here!
+      return;
+    }
+  } else {
+    LOG(WARNING) << "Failed to find the associated render view host for url: "
+                 << url;
+  }
+
+  std::move(callback).Run(ignore_navigation);
+}
+
+}  // namespace
+
 AwRenderViewHostExt::AwRenderViewHostExt(AwRenderViewHostExtClient* client,
                                          content::WebContents* contents)
     : content::WebContentsObserver(contents),
@@ -173,4 +206,17 @@ void AwRenderViewHostExt::ContentsSizeChanged(const gfx::Size& contents_size) {
   client_->OnWebLayoutContentsSizeChanged(contents_size);
 }
 
+void AwRenderViewHostExt::ShouldOverrideUrlLoading(
+    const base::string16& url,
+    bool has_user_gesture,
+    bool is_redirect,
+    bool is_main_frame,
+    ShouldOverrideUrlLoadingCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  content::GetUIThreadTaskRunner({})->PostTask(
+      FROM_HERE, base::BindOnce(&ShouldOverrideUrlLoadingOnUI, web_contents(),
+                                url, has_user_gesture, is_redirect,
+                                is_main_frame, std::move(callback)));
+}
+
 }  // namespace android_webview

android_webview/browser/renderer_host/aw_render_view_host_ext.h

@@ -89,6 +89,12 @@ class AwRenderViewHostExt : public content::WebContentsObserver,
   void UpdateHitTestData(
       android_webview::mojom::HitTestDataPtr hit_test_data) override;
   void ContentsSizeChanged(const gfx::Size& contents_size) override;
+  void ShouldOverrideUrlLoading(
+      const base::string16& url,
+      bool has_user_gesture,
+      bool is_redirect,
+      bool is_main_frame,
+      ShouldOverrideUrlLoadingCallback callback) override;
 
   bool IsRenderViewReady() const;
 

android_webview/common/mojom/frame.mojom

@@ -122,4 +122,16 @@ interface FrameHost {
 
   // Calls whenever the contents size (as seen by RenderView) is changed.
   ContentsSizeChanged(gfx.mojom.Size contents_size);
+
+  // Calls immediately before a top level navigation is initiated within Blink.
+  // There are some exlusions, the most important ones are it is not sent
+  // when creating a popup window, and not sent for application initiated
+  // navigations. See AwContentRendererClient::HandleNavigation for all
+  // cornercases. This is sent before updating the NavigationController state
+  // or creating a URLRequest for the main frame resource.
+  [Sync]
+  ShouldOverrideUrlLoading(mojo_base.mojom.String16 url,
+                           bool has_user_gesture, bool is_redirect,
+                           bool is_main_frame)
+      => (bool result);
 };

android_webview/common/render_view_messages.h

@@ -24,20 +24,6 @@
 // RenderView messages
 // These are messages sent from the renderer to the browser process.
 
-// Sent immediately before a top level navigation is initiated within Blink.
-// There are some exlusions, the most important ones are it is not sent
-// when creating a popup window, and not sent for application initiated
-// navigations. See AwContentRendererClient::HandleNavigation for all
-// cornercases. This is sent before updating the NavigationController state
-// or creating a URLRequest for the main frame resource.
-IPC_SYNC_MESSAGE_CONTROL5_1(AwViewHostMsg_ShouldOverrideUrlLoading,
-                            int /* render_frame_id id */,
-                            base::string16 /* in - url */,
-                            bool /* in - has_user_gesture */,
-                            bool /* in - is_redirect */,
-                            bool /* in - is_main_frame */,
-                            bool /* out - result */)
-
 // Sent when a subframe is created.
 IPC_MESSAGE_CONTROL2(AwViewHostMsg_SubFrameCreated,
                      int /* parent_render_frame_id */,

android_webview/renderer/aw_content_renderer_client.cc

@@ -35,6 +35,7 @@
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
 #include "mojo/public/cpp/bindings/binder_map.h"
+#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/public/platform/web_string.h"
 #include "third_party/blink/public/platform/web_url.h"
@@ -137,10 +138,12 @@ bool AwContentRendererClient::HandleNavigation(
   base::string16 url = request.Url().GetString().Utf16();
   bool has_user_gesture = request.HasUserGesture();
 
-  int render_frame_id = render_frame->GetRoutingID();
-  RenderThread::Get()->Send(new AwViewHostMsg_ShouldOverrideUrlLoading(
-      render_frame_id, url, has_user_gesture, is_redirect, is_main_frame,
-      &ignore_navigation));
+  mojo::AssociatedRemote<mojom::FrameHost> frame_host_remote;
+  render_frame->GetRemoteAssociatedInterfaces()->GetInterface(
+      &frame_host_remote);
+  frame_host_remote->ShouldOverrideUrlLoading(
+      url, has_user_gesture, is_redirect, is_main_frame, &ignore_navigation);
+
   return ignore_navigation;
 }
 

android_webview/renderer/aw_content_renderer_client.h

@@ -8,11 +8,13 @@
 #include <memory>
 #include <string>
 
+#include "android_webview/common/mojom/frame.mojom.h"
 #include "android_webview/renderer/aw_render_thread_observer.h"
 #include "base/compiler_specific.h"
 #include "base/memory/weak_ptr.h"
 #include "components/spellcheck/spellcheck_buildflags.h"
 #include "content/public/renderer/content_renderer_client.h"
+#include "mojo/public/cpp/bindings/associated_remote.h"
 #include "services/service_manager/public/cpp/local_interface_provider.h"
 #include "third_party/blink/public/common/thread_safe_browser_interface_broker_proxy.h"