0

Migrate ViewHostMsg_UpdateTargetURL/ViewMsg_UpdateTargetURL_ACK to mojo

This CL migrates these two legacy IPC messages out of view_messages.h
to the {Local,Remote}MainFrameHost mojo interfaces from Blink, by using
the newly defined method UpdateTargetURL() in both interfaces (required
to work both when the main frame in the renderer is local or remote).

Additionally, this CL moves the  UpdateTargetURL() method present in
RenderViewHostDelegate into RenderFrameHostDelegate, now that the IPC
message will be now received in the browser process by a RenderFrame,
instead of being handled by RenderViewHost as it was the case before.

Last, the UpdateTargetURLWithInvalidURL unit test has been moved out of
RenderViewBrowserTest and into WebViewTest, as that reflects better the
situation in the renderer side now, and also to get friend access to
the |target_url_| private attribute, which is now part of WebViewImpl.

Bug: 993189, 1047464
Change-Id: Ib97ee5270221efc48d50138cf3f075619430d063
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2387801
Reviewed-by: Aaron Colwell <acolwell@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Cr-Commit-Position: refs/heads/master@{#804735}
This commit is contained in:
Mario Sanchez Prada
2020-09-07 12:01:08 +00:00
committed by Commit Bot
parent 9477120724
commit dffbe66e98
22 changed files with 165 additions and 117 deletions

@ -226,6 +226,10 @@ class CONTENT_EXPORT RenderFrameHostDelegate {
const base::string16& title,
base::i18n::TextDirection title_direction) {}
// The destination URL has changed and should be updated.
virtual void UpdateTargetURL(RenderFrameHost* render_frame_host,
const GURL& url) {}
// Return this object cast to a WebContents, if it is one. If the object is
// not a WebContents, returns NULL.
virtual WebContents* GetAsWebContents();

@ -3402,6 +3402,13 @@ void RenderFrameHostImpl::FocusPage() {
render_view_host_->OnFocus();
}
void RenderFrameHostImpl::UpdateTargetURL(
const GURL& url,
blink::mojom::LocalMainFrameHost::UpdateTargetURLCallback callback) {
delegate_->UpdateTargetURL(this, url);
std::move(callback).Run();
}
void RenderFrameHostImpl::UpdateFaviconURL(
std::vector<blink::mojom::FaviconURLPtr> favicon_urls) {
delegate_->UpdateFaviconURL(this, std::move(favicon_urls));

@ -1661,6 +1661,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
void TextAutosizerPageInfoChanged(
blink::mojom::TextAutosizerPageInfoPtr page_info) override;
void FocusPage() override;
void UpdateTargetURL(const GURL& url,
blink::mojom::LocalMainFrameHost::UpdateTargetURLCallback
callback) override;
void ReportNoBinderForInterface(const std::string& error);

@ -591,6 +591,13 @@ void RenderFrameProxyHost::FocusPage() {
frame_tree_node_->current_frame_host()->FocusPage();
}
void RenderFrameProxyHost::UpdateTargetURL(
const GURL& url,
blink::mojom::RemoteMainFrameHost::UpdateTargetURLCallback callback) {
frame_tree_node_->current_frame_host()->UpdateTargetURL(url,
std::move(callback));
}
void RenderFrameProxyHost::RouteCloseEvent() {
// Tell the active RenderViewHost to run unload handlers and close, as long
// as the request came from a RenderViewHost in the same BrowsingInstance.

@ -180,6 +180,10 @@ class CONTENT_EXPORT RenderFrameProxyHost
// blink::mojom::RemoteMainFrameHost overrides:
void FocusPage() override;
void UpdateTargetURL(
const GURL& url,
blink::mojom::RemoteMainFrameHost::UpdateTargetURLCallback callback)
override;
void RouteCloseEvent() override;
// mojom::RenderFrameProxyHost:

@ -5,7 +5,6 @@
#include "content/browser/renderer_host/render_view_host_delegate.h"
#include "content/public/common/web_preferences.h"
#include "url/gurl.h"
namespace content {

@ -18,8 +18,6 @@
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "net/base/load_states.h"
class GURL;
namespace IPC {
class Message;
}
@ -91,10 +89,6 @@ class CONTENT_EXPORT RenderViewHostDelegate {
// RenderView is going to be destroyed
virtual void RenderViewDeleted(RenderViewHost* render_view_host) {}
// The destination URL has changed should be updated.
virtual void UpdateTargetURL(RenderViewHost* render_view_host,
const GURL& url) {}
// The page is trying to close the RenderView's representation in the client.
virtual void Close(RenderViewHost* render_view_host) {}

@ -699,7 +699,6 @@ bool RenderViewHostImpl::OnMessageReceived(const IPC::Message& msg) {
IPC_MESSAGE_HANDLER(ViewHostMsg_ShowWidget, OnShowWidget)
IPC_MESSAGE_HANDLER(ViewHostMsg_ShowFullscreenWidget,
OnShowFullscreenWidget)
IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateTargetURL, OnUpdateTargetURL)
IPC_MESSAGE_HANDLER(ViewHostMsg_TakeFocus, OnTakeFocus)
IPC_MESSAGE_UNHANDLED(handled = false)
IPC_END_MESSAGE_MAP()
@ -730,14 +729,6 @@ void RenderViewHostImpl::OnShowFullscreenWidget(int widget_route_id) {
Send(new WidgetMsg_SetBounds_ACK(widget_route_id));
}
void RenderViewHostImpl::OnUpdateTargetURL(const GURL& url) {
delegate_->UpdateTargetURL(this, url);
// Send a notification back to the renderer that we are ready to
// receive more target urls.
Send(new ViewMsg_UpdateTargetURL_ACK(GetRoutingID()));
}
void RenderViewHostImpl::OnDidContentsPreferredSizeChange(
const gfx::Size& new_size) {
delegate_->UpdatePreferredSize(new_size);

@ -308,7 +308,6 @@ class CONTENT_EXPORT RenderViewHostImpl
bool user_gesture);
void OnShowWidget(int widget_route_id, const gfx::Rect& initial_rect);
void OnShowFullscreenWidget(int widget_route_id);
void OnUpdateTargetURL(const GURL& url);
void OnDidContentsPreferredSizeChange(const gfx::Size& new_size);
void OnPasteFromSelectionClipboard();
void OnTakeFocus(bool reverse);

@ -6509,28 +6509,8 @@ void WebContentsImpl::RenderViewDeleted(RenderViewHost* rvh) {
[&](WebContentsObserver* observer) { observer->RenderViewDeleted(rvh); });
}
void WebContentsImpl::UpdateTargetURL(RenderViewHost* render_view_host,
const GURL& url) {
if (fullscreen_widget_routing_id_ != MSG_ROUTING_NONE) {
// If we're in flash fullscreen (i.e. Pepper plugin fullscreen) only update
// the url if it's from the fullscreen renderer.
RenderWidgetHostView* fs = GetFullscreenRenderWidgetHostView();
if (fs && fs->GetRenderWidgetHost() != render_view_host->GetWidget())
return;
}
// In case of racey updates from multiple RenderViewHosts, the last URL should
// be shown - see also some discussion in https://crbug.com/807776.
if (!url.is_valid() && render_view_host != view_that_set_last_target_url_)
return;
view_that_set_last_target_url_ = url.is_valid() ? render_view_host : nullptr;
if (delegate_)
delegate_->UpdateTargetURL(this, url);
}
void WebContentsImpl::ClearTargetURL() {
view_that_set_last_target_url_ = nullptr;
frame_that_set_last_target_url_ = nullptr;
if (delegate_)
delegate_->UpdateTargetURL(this, GURL());
}
@ -6827,6 +6807,28 @@ void WebContentsImpl::UpdateTitle(RenderFrameHost* render_frame_host,
UpdateTitleForEntry(entry, title);
}
void WebContentsImpl::UpdateTargetURL(RenderFrameHost* render_frame_host,
const GURL& url) {
if (fullscreen_widget_routing_id_ != MSG_ROUTING_NONE) {
// If we're in flash fullscreen (i.e. Pepper plugin fullscreen) only update
// the url if it's from the fullscreen renderer.
RenderWidgetHostView* fs = GetFullscreenRenderWidgetHostView();
RenderViewHost* render_view_host = render_frame_host->GetRenderViewHost();
if (fs && fs->GetRenderWidgetHost() != render_view_host->GetWidget())
return;
}
// In case of racey updates from multiple RenderViewHosts, the last URL should
// be shown - see also some discussion in https://crbug.com/807776.
if (!url.is_valid() && render_frame_host != frame_that_set_last_target_url_)
return;
frame_that_set_last_target_url_ =
url.is_valid() ? render_frame_host : nullptr;
if (delegate_)
delegate_->UpdateTargetURL(this, url);
}
bool WebContentsImpl::ShouldRouteMessageEvent(
RenderFrameHost* target_rfh,
SiteInstance* source_site_instance) const {

@ -596,6 +596,8 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
void UpdateTitle(RenderFrameHost* render_frame_host,
const base::string16& title,
base::i18n::TextDirection title_direction) override;
void UpdateTargetURL(RenderFrameHost* render_frame_host,
const GURL& url) override;
WebContents* GetAsWebContents() override;
bool IsNeverComposited() override;
ui::AXMode GetAccessibilityMode() override;
@ -783,8 +785,6 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
base::TerminationStatus status,
int error_code) override;
void RenderViewDeleted(RenderViewHost* render_view_host) override;
void UpdateTargetURL(RenderViewHost* render_view_host,
const GURL& url) override;
void Close(RenderViewHost* render_view_host) override;
void RequestSetBounds(const gfx::Rect& new_bounds) override;
bool DidAddMessageToConsole(blink::mojom::ConsoleMessageLevel log_level,
@ -2079,7 +2079,7 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
bool was_ever_audible_ = false;
// Helper variable for resolving races in UpdateTargetURL / ClearTargetURL.
RenderViewHost* view_that_set_last_target_url_ = nullptr;
RenderFrameHost* frame_that_set_last_target_url_ = nullptr;
// Whether we should override user agent in new tabs.
bool should_override_user_agent_in_new_tabs_ = false;

@ -98,10 +98,6 @@ IPC_STRUCT_TRAITS_END()
IPC_MESSAGE_ROUTED1(ViewMsg_UpdateWebPreferences,
content::WebPreferences)
// Used to notify the render-view that we have received a target URL. Used
// to prevent target URLs spamming the browser.
IPC_MESSAGE_ROUTED0(ViewMsg_UpdateTargetURL_ACK)
// Notification that a move or resize renderer's containing window has
// started.
IPC_MESSAGE_ROUTED0(ViewMsg_MoveOrResizeStarted)
@ -135,11 +131,6 @@ IPC_MESSAGE_ROUTED2(ViewHostMsg_ShowWidget,
IPC_MESSAGE_ROUTED1(ViewHostMsg_ShowFullscreenWidget,
int /* route_id */)
// Notifies the browser that we want to show a destination url for a potential
// action (e.g. when the user is hovering over a link).
IPC_MESSAGE_ROUTED1(ViewHostMsg_UpdateTargetURL,
GURL)
#if BUILDFLAG(ENABLE_PLUGINS)
// A renderer sends this to the browser process when it wants to access a PPAPI
// broker. In contrast to FrameHostMsg_OpenChannelToPpapiBroker, this is called

@ -2092,13 +2092,6 @@ TEST_F(RenderViewImplTest, DroppedNavigationStaysInViewSourceMode) {
EXPECT_TRUE(web_frame->IsViewSourceModeEnabled());
}
// Regression test for http://crbug.com/41562
TEST_F(RenderViewImplTest, UpdateTargetURLWithInvalidURL) {
const GURL invalid_gurl("http://");
view()->SetMouseOverURL(blink::WebURL(invalid_gurl));
EXPECT_EQ(invalid_gurl, view()->target_url_);
}
TEST_F(RenderViewImplTest, SetHistoryLengthAndOffset) {
// No history to merge; one committed page.
view()->OnSetHistoryOffsetAndLength(0, 1);

@ -1064,14 +1064,6 @@ void RenderViewImpl::ResizeWebWidgetForWidget(
// IPC message handlers -----------------------------------------
void RenderViewImpl::OnUpdateTargetURLAck() {
// Check if there is a targeturl waiting to be sent.
if (target_url_status_ == TARGET_PENDING)
Send(new ViewHostMsg_UpdateTargetURL(GetRoutingID(), pending_target_url_));
target_url_status_ = TARGET_NONE;
}
void RenderViewImpl::OnSetHistoryOffsetAndLength(int history_offset,
int history_length) {
// -1 <= history_offset < history_length <= kMaxSessionHistoryEntries(50).
@ -1130,7 +1122,6 @@ bool RenderViewImpl::OnMessageReceived(const IPC::Message& message) {
bool handled = true;
IPC_BEGIN_MESSAGE_MAP(RenderViewImpl, message)
IPC_MESSAGE_HANDLER(ViewMsg_UpdateTargetURL_ACK, OnUpdateTargetURLAck)
IPC_MESSAGE_HANDLER(ViewMsg_UpdateWebPreferences, OnUpdateWebPreferences)
IPC_MESSAGE_HANDLER(ViewMsg_MoveOrResizeStarted, OnMoveOrResizeStarted)
@ -1415,31 +1406,6 @@ void RenderViewImpl::SetValidationMessageDirection(
}
}
void RenderViewImpl::UpdateTargetURL(const GURL& url,
const GURL& fallback_url) {
GURL latest_url = url.is_empty() ? fallback_url : url;
if (latest_url == target_url_)
return;
// Tell the browser to display a destination link.
if (target_url_status_ == TARGET_INFLIGHT ||
target_url_status_ == TARGET_PENDING) {
// If we have a request in-flight, save the URL to be sent when we
// receive an ACK to the in-flight request. We can happily overwrite
// any existing pending sends.
pending_target_url_ = latest_url;
target_url_status_ = TARGET_PENDING;
} else {
// URLs larger than |kMaxURLChars| cannot be sent through IPC -
// see |ParamTraits<GURL>|.
if (latest_url.possibly_invalid_spec().size() > url::kMaxURLChars)
latest_url = GURL();
Send(new ViewHostMsg_UpdateTargetURL(GetRoutingID(), latest_url));
target_url_ = latest_url;
target_url_status_ = TARGET_INFLIGHT;
}
}
void RenderViewImpl::StartNavStateSyncTimerIfNecessary(RenderFrameImpl* frame) {
// Keep track of which frames have pending updates.
frames_with_pending_state_.insert(frame->GetRoutingID());
@ -1468,12 +1434,12 @@ void RenderViewImpl::StartNavStateSyncTimerIfNecessary(RenderFrameImpl* frame) {
void RenderViewImpl::SetMouseOverURL(const WebURL& url) {
mouse_over_url_ = GURL(url);
UpdateTargetURL(mouse_over_url_, focus_url_);
GetWebView()->UpdateTargetURL(mouse_over_url_, focus_url_);
}
void RenderViewImpl::SetKeyboardFocusURL(const WebURL& url) {
focus_url_ = GURL(url);
UpdateTargetURL(focus_url_, mouse_over_url_);
GetWebView()->UpdateTargetURL(focus_url_, mouse_over_url_);
}
bool RenderViewImpl::AcceptsLoadDrops() {

@ -299,7 +299,6 @@ class CONTENT_EXPORT RenderViewImpl : public blink::WebViewClient,
FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, StaleNavigationsIgnored);
FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest,
DontIgnoreBackAfterNavEntryLimit);
FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, UpdateTargetURLWithInvalidURL);
FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest,
GetCompositionCharacterBoundsTest);
FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, OnNavigationHttpPost);
@ -499,35 +498,12 @@ class CONTENT_EXPORT RenderViewImpl : public blink::WebViewClient,
// UI state ------------------------------------------------------------------
// The state of our target_url transmissions. When we receive a request to
// send a URL to the browser, we set this to TARGET_INFLIGHT until an ACK
// comes back - if a new request comes in before the ACK, we store the new
// URL in pending_target_url_ and set the status to TARGET_PENDING. If an
// ACK comes back and we are in TARGET_PENDING, we send the stored URL and
// revert to TARGET_INFLIGHT.
//
// We don't need a queue of URLs to send, as only the latest is useful.
enum {
TARGET_NONE,
TARGET_INFLIGHT, // We have a request in-flight, waiting for an ACK
TARGET_PENDING // INFLIGHT + we have a URL waiting to be sent
} target_url_status_ = TARGET_NONE;
// The URL we show the user in the status bar. We use this to determine if we
// want to send a new one (we do not need to send duplicates). It will be
// equal to either |mouse_over_url_| or |focus_url_|, depending on which was
// updated last.
GURL target_url_;
// The URL the user's mouse is hovering over.
GURL mouse_over_url_;
// The URL that has keyboard focus.
GURL focus_url_;
// The next target URL we want to send to the browser.
GURL pending_target_url_;
// View ----------------------------------------------------------------------
// This class owns this member, and is responsible for calling