Remove WebContentsImpl::ShowPopupMenu

This method doesn't actually show a popup menu but does decide whether
one should be shown by RenderFrameHostImpl, and it doesn't access
anything that RenderFrameHostImpl doesn't already have, so I am removing
the method as inlining it into RenderFrameHostImpl::ShowPopupMenu.

Fixed: 328100331
Change-Id: I18c821edaf241d280ebacf22d4348be3a17312c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5346416
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1289644}

content/browser/renderer_host/render_frame_host_delegate.cc

@@ -161,12 +161,6 @@ RenderWidgetHostImpl* RenderFrameHostDelegate::CreateNewPopupWidget(
   return nullptr;
 }
 
-bool RenderFrameHostDelegate::ShowPopupMenu(
-    RenderFrameHostImpl* render_frame_host,
-    const gfx::Rect& bounds) {
-  return false;
-}
-
 std::vector<RenderFrameHostImpl*>
 RenderFrameHostDelegate::GetActiveTopLevelDocumentsInBrowsingContextGroup(
     RenderFrameHostImpl* render_frame_host) {

content/browser/renderer_host/render_frame_host_delegate.h

@@ -600,13 +600,6 @@ class CONTENT_EXPORT RenderFrameHostDelegate {
           blink_widget_host,
       mojo::PendingAssociatedRemote<blink::mojom::Widget> blink_widget);
 
-  // Returns true if the popup is shown through WebContentsObserver. Else, the
-  // Android / Mac flavors of `RenderViewHostDelegateView` will show the popup
-  // menu correspondingly, and `WebContentsViewChildFrame` will show the popup
-  // for Mac's GuestView.
-  virtual bool ShowPopupMenu(RenderFrameHostImpl* render_frame_host,
-                             const gfx::Rect& bounds);
-
   virtual void DidLoadResourceFromMemoryCache(
       RenderFrameHostImpl* source,
       const GURL& url,

content/browser/renderer_host/render_frame_host_impl.cc

@@ -7869,7 +7869,14 @@ void RenderFrameHostImpl::ShowPopupMenu(
     return;
   }
 
-  if (delegate()->ShowPopupMenu(this, bounds)) {
+  if (delegate()->GetVisibility() != Visibility::VISIBLE) {
+    // Don't create popups for hidden tabs. https://crbug.com/1521345
+    send_did_cancel(std::move(popup_client));
+    return;
+  }
+
+  if (show_popup_menu_callback_for_testing_) {
+    std::move(show_popup_menu_callback_for_testing_).Run(bounds);
     send_did_cancel(std::move(popup_client));
     return;
   }

content/browser/renderer_host/render_frame_host_impl.h

@@ -2372,6 +2372,10 @@ class CONTENT_EXPORT RenderFrameHostImpl
   void TextSelectionChanged(const std::u16string& text,
                             uint32_t offset,
                             const gfx::Range& range) override;
+  void set_show_popup_menu_callback_for_testing(
+      base::OnceCallback<void(const gfx::Rect&)> callback) {
+    show_popup_menu_callback_for_testing_ = std::move(callback);
+  }
   void ShowPopupMenu(
       mojo::PendingRemote<blink::mojom::PopupMenuClient> popup_client,
       const gfx::Rect& bounds,
@@ -5173,6 +5177,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // nonce returned is unique.
   base::Uuid base_auction_nonce_;
 
+  base::OnceCallback<void(const gfx::Rect&)>
+      show_popup_menu_callback_for_testing_;
+
   // WeakPtrFactories are the last members, to ensure they are destroyed before
   // all other fields of `this`.
   base::WeakPtrFactory<RenderFrameHostImpl> weak_ptr_factory_{this};

content/browser/web_contents/web_contents_impl.cc

@@ -10193,28 +10193,6 @@ void WebContentsImpl::DidChangeScreenOrientation() {
   last_screen_orientation_change_time_ = ui::EventTimeForNow();
 }
 
-// TODO(crbug.com/328100331): This method should be renamed since it doesn't
-// actually show a popup menu.
-bool WebContentsImpl::ShowPopupMenu(RenderFrameHostImpl* render_frame_host,
-                                    const gfx::Rect& bounds) {
-  OPTIONAL_TRACE_EVENT1("content", "WebContentsImpl::ShowPopupMenu",
-                        "render_frame_host", render_frame_host);
-
-  if (visibility_ != Visibility::VISIBLE) {
-    // Don't create popups for hidden tabs. http://crbug.com/1521345
-    // Returning true here makes RenderFrameHostImpl::ShowPopupMenu abort
-    // before showing a popup.
-    return true;
-  }
-
-  DCHECK(render_frame_host->IsActive());
-  if (show_popup_menu_callback_) {
-    std::move(show_popup_menu_callback_).Run(bounds);
-    return true;
-  }
-  return false;
-}
-
 void WebContentsImpl::UpdateWebContentsVisibility(Visibility visibility) {
   OPTIONAL_TRACE_EVENT1("content",
                         "WebContentsImpl::UpdateWebContentsVisibility",

content/browser/web_contents/web_contents_impl.h

@@ -821,8 +821,6 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
           blink_widget_host,
       mojo::PendingAssociatedRemote<blink::mojom::Widget> blink_widget)
       override;
-  bool ShowPopupMenu(RenderFrameHostImpl* render_frame_host,
-                     const gfx::Rect& bounds) override;
   void DidLoadResourceFromMemoryCache(
       RenderFrameHostImpl* source,
       const GURL& url,
@@ -1401,11 +1399,6 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
   };
   CaptureTarget GetCaptureTarget();
 
-  void set_show_popup_menu_callback_for_testing(
-      base::OnceCallback<void(const gfx::Rect&)> callback) {
-    show_popup_menu_callback_ = std::move(callback);
-  }
-
   // Sets the value in tests to ensure expected ordering and correctness.
   void set_minimum_delay_between_loading_updates_for_testing(
       base::TimeDelta duration) {
@@ -2429,8 +2422,6 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
 
   viz::FrameSinkId xr_render_target_;
 
-  base::OnceCallback<void(const gfx::Rect&)> show_popup_menu_callback_;
-
   // Allows the app in the current WebContents to opt-in to exposing
   // information to apps that capture it.
   blink::mojom::CaptureHandleConfig capture_handle_config_;

content/test/content_browser_test_utils_internal.cc

@@ -582,8 +582,7 @@ ShowPopupWidgetWaiter::ShowPopupWidgetWaiter(WebContentsImpl* web_contents,
                                              RenderFrameHostImpl* frame_host)
     : frame_host_(frame_host) {
 #if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_ANDROID)
-  web_contents_ = web_contents;
-  web_contents_->set_show_popup_menu_callback_for_testing(base::BindOnce(
+  frame_host->set_show_popup_menu_callback_for_testing(base::BindOnce(
       &ShowPopupWidgetWaiter::ShowPopupMenu, base::Unretained(this)));
 #endif
   frame_host_->SetCreateNewPopupCallbackForTesting(base::BindRepeating(
@@ -605,7 +604,7 @@ void ShowPopupWidgetWaiter::Wait() {
 
 void ShowPopupWidgetWaiter::Stop() {
 #if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_ANDROID)
-  web_contents_->set_show_popup_menu_callback_for_testing(base::NullCallback());
+  frame_host_->set_show_popup_menu_callback_for_testing(base::NullCallback());
 #endif
   frame_host_->SetCreateNewPopupCallbackForTesting(base::NullCallback());
   frame_host_ = nullptr;

content/test/content_browser_test_utils_internal.h

@@ -326,9 +326,6 @@ class ShowPopupWidgetWaiter
   int32_t routing_id_ = MSG_ROUTING_NONE;
   int32_t process_id_ = 0;
   raw_ptr<RenderFrameHostImpl> frame_host_;
-#if BUILDFLAG(IS_MAC) || BUILDFLAG(IS_ANDROID)
-  raw_ptr<WebContentsImpl> web_contents_;
-#endif
 };
 
 // This observer waits until WebContentsObserver::OnRendererUnresponsive