Commit 9b890006 authored by Dmitry Gozman's avatar Dmitry Gozman Committed by Commit Bot

Remove mojom::Frame::CommitNavigation IPC

After PerNavigationMojoInterface has been launched, most commits
used NavigationClient::CommitNavigation instead. This IPC was
only used by non-committed interstitials.

After r766516, even interstitials do not use this IPC, and all
commits now go through NavigationClient.

Bug: 1020175, 1077074
Change-Id: Iea23c7bd2e22a8adf8023f2fae8fcc3d8e60c404
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2214626
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: default avatarArthur Hemery <ahemery@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Reviewed-by: default avatarCarlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772251}
parent 731b2a85
......@@ -5628,11 +5628,11 @@ void RenderFrameHostImpl::CommitNavigation(
"frame_tree_node", frame_tree_node_->frame_tree_node_id(), "url",
common_params->url.possibly_invalid_spec());
DCHECK(!IsRendererDebugURL(common_params->url));
DCHECK(navigation_request);
bool is_same_document =
NavigationTypeUtils::IsSameDocument(common_params->navigation_type);
bool is_mhtml_iframe =
navigation_request && navigation_request->IsForMhtmlSubframe();
bool is_mhtml_iframe = navigation_request->IsForMhtmlSubframe();
// A |response| and a |url_loader_client_endpoints| must always be provided,
// except for edge cases, where another way to load the document exist.
......@@ -5745,7 +5745,7 @@ void RenderFrameHostImpl::CommitNavigation(
}
DCHECK(!isolation_info_.IsEmpty());
if (navigation_request && navigation_request->appcache_handle()) {
if (navigation_request->appcache_handle()) {
// AppCache may create a subresource URLLoaderFactory later, so make sure it
// has the correct origin to use when calling
// ContentBrowserClient::WillCreateURLLoaderFactory().
......@@ -5844,7 +5844,7 @@ void RenderFrameHostImpl::CommitNavigation(
// NavigationRequest in this function.
recreate_default_url_loader_factory_after_network_service_crash_ = true;
CrossOriginEmbedderPolicyReporter* const coep_reporter =
navigation_request ? navigation_request->coep_reporter() : nullptr;
navigation_request->coep_reporter();
mojo::PendingRemote<network::mojom::CrossOriginEmbedderPolicyReporter>
coep_reporter_remote;
if (coep_reporter) {
......@@ -5856,9 +5856,7 @@ void RenderFrameHostImpl::CommitNavigation(
CreateNetworkServiceDefaultFactoryAndObserve(
CreateURLLoaderFactoryParamsForMainWorld(
main_world_origin_for_url_loader_factory,
navigation_request
? mojo::Clone(navigation_request->client_security_state())
: network::mojom::ClientSecurityState::New(),
mojo::Clone(navigation_request->client_security_state()),
std::move(coep_reporter_remote),
DetermineWhetherToForbidTrustTokenRedemption(
GetParent(), *commit_params,
......@@ -5982,9 +5980,7 @@ void RenderFrameHostImpl::CommitNavigation(
CreateURLLoaderFactoriesForIsolatedWorlds(
main_world_origin_for_url_loader_factory,
isolated_worlds_requiring_separate_url_loader_factory_,
navigation_request
? mojo::Clone(navigation_request->client_security_state())
: network::mojom::ClientSecurityState::New(),
mojo::Clone(navigation_request->client_security_state()),
DetermineWhetherToForbidTrustTokenRedemption(
GetParent(), *commit_params,
main_world_origin_for_url_loader_factory));
......@@ -6057,9 +6053,8 @@ void RenderFrameHostImpl::CommitNavigation(
EnsurePrefetchedSignedExchangeCache());
}
mojom::NavigationClient* navigation_client = nullptr;
if (navigation_request)
navigation_client = navigation_request->GetCommitNavigationClient();
mojom::NavigationClient* navigation_client =
navigation_request->GetCommitNavigationClient();
// Record the metrics about the state of the old main frame at the moment
// when we navigate away from it as it matters for whether the page
......@@ -8139,32 +8134,14 @@ void RenderFrameHostImpl::SendCommitNavigation(
mojo::PendingRemote<network::mojom::URLLoaderFactory>
prefetch_loader_factory,
const base::UnguessableToken& devtools_navigation_token) {
// For committed interstitials, we do not have a NavigationRequest and use the
// old NavigationControl mojo interface. For anything else we should have a
// NavigationRequest, containing a NavigationClient, and commit through it.
// TODO(ahemery): Update when https://crbug.com/448486 is done.
if (navigation_client) {
navigation_client->CommitNavigation(
std::move(common_params), std::move(commit_params),
std::move(response_head), std::move(response_body),
std::move(url_loader_client_endpoints),
std::move(subresource_loader_factories),
std::move(subresource_overrides), std::move(controller),
std::move(container_info), std::move(prefetch_loader_factory),
devtools_navigation_token,
BuildCommitNavigationCallback(navigation_request));
} else {
DCHECK(!navigation_request);
GetNavigationControl()->CommitNavigation(
std::move(common_params), std::move(commit_params),
std::move(response_head), std::move(response_body),
std::move(url_loader_client_endpoints),
std::move(subresource_loader_factories),
std::move(subresource_overrides), std::move(controller),
std::move(container_info), std::move(prefetch_loader_factory),
devtools_navigation_token,
mojom::FrameNavigationControl::CommitNavigationCallback());
}
navigation_client->CommitNavigation(
std::move(common_params), std::move(commit_params),
std::move(response_head), std::move(response_body),
std::move(url_loader_client_endpoints),
std::move(subresource_loader_factories), std::move(subresource_overrides),
std::move(controller), std::move(container_info),
std::move(prefetch_loader_factory), devtools_navigation_token,
BuildCommitNavigationCallback(navigation_request));
}
void RenderFrameHostImpl::SendCommitFailedNavigation(
......
......@@ -1669,7 +1669,7 @@ class CONTENT_EXPORT RenderFrameHostImpl
LifecycleState lifecycle_state);
// The SendCommit* functions below are wrappers for commit calls
// made to mojom::FrameNavigationControl and mojom::NavigationClient.
// made to mojom::NavigationClient.
// These exist to be overridden in tests to retain mojo callbacks.
// Note: |navigation_id| is used in test overrides, but is unused otherwise.
virtual void SendCommitNavigation(
......@@ -2216,6 +2216,8 @@ class CONTENT_EXPORT RenderFrameHostImpl
// Called by the renderer process when it is done processing a cross-document
// commit request.
// TODO(https://crbug.com/1020175): this is only called with
// blink::mojom::CommitResult::Aborted.
void OnCrossDocumentCommitProcessed(NavigationRequest* navigation_request,
blink::mojom::CommitResult result);
......
......@@ -99,52 +99,6 @@ interface Frame {
// KEEP THE COMMIT FUNCTIONS IN SYNC in content/common/navigation_client.mojom.
// These will eventually be removed from FrameNavigationControl.
interface FrameNavigationControl {
// Tells the renderer that a navigation is ready to commit.
//
// The renderer should bind the |url_loader_client_endpoints| to an
// URLLoaderClient implementation to continue loading the document that will
// be the result of the committed navigation.
//
// Note: |url_loader_client_endpoints| will be empty iff the navigation URL
// wasn't handled by the network stack (i.e. about:blank, ...)
//
// When the Network Service is enabled, |subresource_loader_factories| may
// also be provided by the browser as a a means for the renderer to load
// subresources where applicable.
//
// |controller_service_worker_info| may also be provided by the browser if the
// frame that is being navigated is supposed to be controlled by a Service
// Worker.
// |container_info| may also be provided if the browser has created a
// ServiceWorkerContainerHost for this navigation.
// |prefetch_loader_factory| is populated only when Network Service is
// enabled. The pointer is used to start a prefetch loading via the browser
// process.
//
// For automation driver-initiated navigations over the devtools protocol,
// |devtools_navigation_token_| is used to tag the navigation. This navigation
// token is then sent into the renderer and lands on the DocumentLoader. That
// way subsequent Blink-level frame lifecycle events can be associated with
// the concrete navigation.
// - The value should not be sent back to the browser.
// - The value on DocumentLoader may be generated in the renderer in some
// cases, and thus shouldn't be trusted.
// TODO(crbug.com/783506): Replace devtools navigation token with the generic
// navigation token that can be passed from renderer to the browser.
CommitNavigation(
CommonNavigationParams common_params,
CommitNavigationParams request_params,
network.mojom.URLResponseHead response_head,
handle<data_pipe_consumer>? response_body,
network.mojom.URLLoaderClientEndpoints? url_loader_client_endpoints,
blink.mojom.URLLoaderFactoryBundle? subresource_loader_factories,
array<TransferrableURLLoader>? subresource_overrides,
blink.mojom.ControllerServiceWorkerInfo? controller_service_worker_info,
blink.mojom.ServiceWorkerContainerInfoForClient? container_info,
pending_remote<network.mojom.URLLoaderFactory>? prefetch_loader_factory,
mojo_base.mojom.UnguessableToken devtools_navigation_token)
=> (blink.mojom.CommitResult commit_result);
// Tells the renderer that a same-document navigation should be committed.
// The renderer will return a status value indicating whether the commit
// could proceed as expected or not. In particular, it might be necessary to
......
......@@ -35,7 +35,7 @@ void NavigationClient::CommitNavigation(
// race conditions leading to the early deletion of NavigationRequest would
// unexpectedly abort the ongoing navigation. Remove when the races are fixed.
ResetDisconnectionHandler();
render_frame_->CommitPerNavigationMojoInterfaceNavigation(
render_frame_->CommitNavigation(
std::move(common_params), std::move(commit_params),
std::move(response_head), std::move(response_body),
std::move(url_loader_client_endpoints), std::move(subresource_loaders),
......
......@@ -16,7 +16,6 @@
namespace content {
NavigationState::~NavigationState() {
RunCommitNavigationCallback(blink::mojom::CommitResult::Aborted);
navigation_client_.reset();
}
......@@ -24,22 +23,19 @@ NavigationState::~NavigationState() {
std::unique_ptr<NavigationState> NavigationState::CreateBrowserInitiated(
mojom::CommonNavigationParamsPtr common_params,
mojom::CommitNavigationParamsPtr commit_params,
mojom::FrameNavigationControl::CommitNavigationCallback callback,
mojom::NavigationClient::CommitNavigationCallback
per_navigation_mojo_interface_callback,
mojom::NavigationClient::CommitNavigationCallback commit_callback,
std::unique_ptr<NavigationClient> navigation_client,
bool was_initiated_in_this_frame) {
return base::WrapUnique(new NavigationState(
std::move(common_params), std::move(commit_params), false,
std::move(callback), std::move(per_navigation_mojo_interface_callback),
std::move(navigation_client), was_initiated_in_this_frame));
std::move(commit_callback), std::move(navigation_client),
was_initiated_in_this_frame));
}
// static
std::unique_ptr<NavigationState> NavigationState::CreateContentInitiated() {
return base::WrapUnique(new NavigationState(
CreateCommonNavigationParams(), CreateCommitNavigationParams(), true,
content::mojom::FrameNavigationControl::CommitNavigationCallback(),
content::mojom::NavigationClient::CommitNavigationCallback(), nullptr,
true));
}
......@@ -60,17 +56,12 @@ bool NavigationState::IsContentInitiated() {
}
void NavigationState::RunCommitNavigationCallback(
blink::mojom::CommitResult result) {
if (commit_callback_)
std::move(commit_callback_).Run(result);
}
void NavigationState::RunPerNavigationInterfaceCommitNavigationCallback(
std::unique_ptr<::FrameHostMsg_DidCommitProvisionalLoad_Params> params,
mojom::DidCommitProvisionalLoadInterfaceParamsPtr interface_params) {
if (per_navigation_mojo_interface_commit_callback_)
std::move(per_navigation_mojo_interface_commit_callback_)
if (commit_callback_) {
std::move(commit_callback_)
.Run(std::move(params), std::move(interface_params));
}
navigation_client_.reset();
}
......@@ -78,9 +69,7 @@ NavigationState::NavigationState(
mojom::CommonNavigationParamsPtr common_params,
mojom::CommitNavigationParamsPtr commit_params,
bool is_content_initiated,
mojom::FrameNavigationControl::CommitNavigationCallback callback,
mojom::NavigationClient::CommitNavigationCallback
per_navigation_mojo_interface_commit_callback,
mojom::NavigationClient::CommitNavigationCallback commit_callback,
std::unique_ptr<NavigationClient> navigation_client,
bool was_initiated_in_this_frame)
: was_within_same_document_(false),
......@@ -89,7 +78,5 @@ NavigationState::NavigationState(
common_params_(std::move(common_params)),
commit_params_(std::move(commit_params)),
navigation_client_(std::move(navigation_client)),
commit_callback_(std::move(callback)),
per_navigation_mojo_interface_commit_callback_(
std::move(per_navigation_mojo_interface_commit_callback)) {}
commit_callback_(std::move(commit_callback)) {}
} // namespace content
......@@ -33,7 +33,6 @@ class CONTENT_EXPORT NavigationState {
static std::unique_ptr<NavigationState> CreateBrowserInitiated(
mojom::CommonNavigationParamsPtr common_params,
mojom::CommitNavigationParamsPtr commit_params,
mojom::FrameNavigationControl::CommitNavigationCallback callback,
mojom::NavigationClient::CommitNavigationCallback
per_navigation_mojo_interface_callback,
std::unique_ptr<NavigationClient> navigation_client,
......@@ -56,9 +55,7 @@ class CONTENT_EXPORT NavigationState {
const mojom::CommitNavigationParams& commit_params() const {
return *commit_params_;
}
bool uses_per_navigation_mojo_interface() const {
return navigation_client_.get();
}
bool has_navigation_client() const { return navigation_client_.get(); }
void set_was_within_same_document(bool value) {
was_within_same_document_ = value;
}
......@@ -71,28 +68,18 @@ class CONTENT_EXPORT NavigationState {
common_params_->transition = transition;
}
// Only used when PerNavigationMojoInterface is enabled.
void set_navigation_client(
std::unique_ptr<NavigationClient> navigation_client_impl) {
navigation_client_ = std::move(navigation_client_impl);
}
void RunCommitNavigationCallback(blink::mojom::CommitResult result);
void RunPerNavigationInterfaceCommitNavigationCallback(
void RunCommitNavigationCallback(
std::unique_ptr<::FrameHostMsg_DidCommitProvisionalLoad_Params> params,
mojom::DidCommitProvisionalLoadInterfaceParamsPtr interface_params);
private:
NavigationState(
mojom::CommonNavigationParamsPtr common_params,
mojom::CommitNavigationParamsPtr commit_params,
bool is_content_initiated,
content::mojom::FrameNavigationControl::CommitNavigationCallback callback,
content::mojom::NavigationClient::CommitNavigationCallback
per_navigation_mojo_interface_callback,
std::unique_ptr<NavigationClient> navigation_client,
bool was_initiated_in_this_frame);
NavigationState(mojom::CommonNavigationParamsPtr common_params,
mojom::CommitNavigationParamsPtr commit_params,
bool is_content_initiated,
content::mojom::NavigationClient::CommitNavigationCallback
commit_callback,
std::unique_ptr<NavigationClient> navigation_client,
bool was_initiated_in_this_frame);
bool was_within_same_document_;
......@@ -129,13 +116,7 @@ class CONTENT_EXPORT NavigationState {
// Used to notify whether a commit request from the browser process was
// successful or not.
mojom::FrameNavigationControl::CommitNavigationCallback commit_callback_;
// Temporary member meant to be used in place of |commit_callback_| when
// PerNavigationMojoInterface is enabled. Should eventually replace it
// completely.
mojom::NavigationClient::CommitNavigationCallback
per_navigation_mojo_interface_commit_callback_;
mojom::NavigationClient::CommitNavigationCallback commit_callback_;
DISALLOW_COPY_AND_ASSIGN(NavigationState);
};
......
This diff is collapsed.
......@@ -526,13 +526,7 @@ class CONTENT_EXPORT RenderFrameImpl
void AllowBindings(int32_t enabled_bindings_flags) override;
void EnableMojoJsBindings() override;
// mojom::FrameNavigationControl implementation:
void ForwardMessageFromHost(
blink::TransferableMessage message,
const url::Origin& source_origin,
const base::Optional<url::Origin>& target_origin) override;
// mojom::FrameNavigationControl implementation:
// These mirror mojom::NavigationClient, called by NavigationClient.
void CommitNavigation(
mojom::CommonNavigationParamsPtr common_params,
mojom::CommitNavigationParamsPtr commit_params,
......@@ -549,31 +543,7 @@ class CONTENT_EXPORT RenderFrameImpl
mojo::PendingRemote<network::mojom::URLLoaderFactory>
prefetch_loader_factory,
const base::UnguessableToken& devtools_navigation_token,
mojom::FrameNavigationControl::CommitNavigationCallback commit_callback)
override;
// This is the version to be used with PerNavigationMojoInterface enabled.
// It essentially works the same way, except the navigation callback is
// the one from NavigationClient mojo interface.
void CommitPerNavigationMojoInterfaceNavigation(
mojom::CommonNavigationParamsPtr common_params,
mojom::CommitNavigationParamsPtr commit_params,
network::mojom::URLResponseHeadPtr response_head,
mojo::ScopedDataPipeConsumerHandle response_body,
network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
std::unique_ptr<blink::PendingURLLoaderFactoryBundle>
subresource_loader_factories,
base::Optional<std::vector<mojom::TransferrableURLLoaderPtr>>
subresource_overrides,
blink::mojom::ControllerServiceWorkerInfoPtr
controller_service_worker_info,
blink::mojom::ServiceWorkerContainerInfoForClientPtr container_info,
mojo::PendingRemote<network::mojom::URLLoaderFactory>
prefetch_loader_factory,
const base::UnguessableToken& devtools_navigation_token,
mojom::NavigationClient::CommitNavigationCallback
per_navigation_mojo_interface_callback);
mojom::NavigationClient::CommitNavigationCallback commit_callback);
void CommitFailedNavigation(
mojom::CommonNavigationParamsPtr common_params,
mojom::CommitNavigationParamsPtr commit_params,
......@@ -586,6 +556,7 @@ class CONTENT_EXPORT RenderFrameImpl
mojom::NavigationClient::CommitFailedNavigationCallback
per_navigation_mojo_interface_callback);
// mojom::FrameNavigationControl implementation:
void CommitSameDocumentNavigation(
mojom::CommonNavigationParamsPtr common_params,
mojom::CommitNavigationParamsPtr commit_params,
......@@ -598,7 +569,6 @@ class CONTENT_EXPORT RenderFrameImpl
mojo::PendingAssociatedRemote<blink::mojom::DevToolsAgentHost> host,
mojo::PendingAssociatedReceiver<blink::mojom::DevToolsAgent> receiver)
override;
void JavaScriptExecuteRequest(
const base::string16& javascript,
bool wants_result,
......@@ -614,6 +584,10 @@ class CONTENT_EXPORT RenderFrameImpl
bool wants_result,
int32_t world_id,
JavaScriptExecuteRequestInIsolatedWorldCallback callback) override;
void ForwardMessageFromHost(
blink::TransferableMessage message,
const url::Origin& source_origin,
const base::Optional<url::Origin>& target_origin) override;
void OnPortalActivated(
const base::UnguessableToken& portal_token,
mojo::PendingAssociatedRemote<blink::mojom::Portal> portal,
......@@ -1263,35 +1237,11 @@ class CONTENT_EXPORT RenderFrameImpl
blink::WebHistoryItem* item_for_history_navigation,
blink::WebFrameLoadType* load_type);
// These functions avoid duplication between Commit*Navigation and
// Commit*PerNavigationMojoInterfaceNavigation functions.
void CommitNavigationInternal(
mojom::CommonNavigationParamsPtr common_params,
mojom::CommitNavigationParamsPtr commit_params,
network::mojom::URLResponseHeadPtr response_head,
mojo::ScopedDataPipeConsumerHandle response_body,
network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
std::unique_ptr<blink::PendingURLLoaderFactoryBundle>
subresource_loader_factories,
base::Optional<std::vector<mojom::TransferrableURLLoaderPtr>>
subresource_overrides,
blink::mojom::ControllerServiceWorkerInfoPtr
controller_service_worker_info,
blink::mojom::ServiceWorkerContainerInfoForClientPtr container_info,
mojo::PendingRemote<network::mojom::URLLoaderFactory>
prefetch_loader_factory,
const base::UnguessableToken& devtools_navigation_token,
mojom::FrameNavigationControl::CommitNavigationCallback callback,
mojom::NavigationClient::CommitNavigationCallback
per_navigation_mojo_interface_callback);
// Ignores the navigation commit and stop its processing in the RenderFrame.
// This will drop the NavigationRequest in the RenderFrameHost.
// Note: This is only meant to be used before building the DocumentState.
// Commit abort and navigation end is handled by it afterwards.
void AbortCommitNavigation(
mojom::FrameNavigationControl::CommitNavigationCallback callback,
blink::mojom::CommitResult reason);
void AbortCommitNavigation();
// Implements AddMessageToConsole().
void AddMessageToConsoleImpl(blink::mojom::ConsoleMessageLevel level,
......
......@@ -21,13 +21,6 @@ class NavigationRequest;
// before they are processed by the implementation. This enables unit/browser
// tests to scrutinize/alter the parameters, or simulate race conditions by
// triggering other calls just before processing DidCommitProvisionalLoad.
//
// IMPORTANT NOTE: Avoid using this when IsPerNavigationMojoInterfaceEnabled()
// is false as this might not do what you expect it to.
// In real code and without the above flag, DidCommit* calls are sent right
// after OnCrossDocumentCommitProcessed, which is NOT handled by this class.
// You might end up with a deleted NavigationRequest and/or mismatched params
// and NavigationRequest.
class DidCommitNavigationInterceptor : public WebContentsObserver {
public:
// Constructs an instance that will intercept DidCommitProvisionalLoad calls
......
......@@ -279,17 +279,17 @@ void TestRenderFrame::Navigate(network::mojom::URLResponseHeadPtr head,
BindNavigationClient(
mock_navigation_client_
.BindNewEndpointAndPassDedicatedReceiverForTesting());
CommitPerNavigationMojoInterfaceNavigation(
std::move(common_params), std::move(commit_params), std::move(head),
mojo::ScopedDataPipeConsumerHandle(),
network::mojom::URLLoaderClientEndpointsPtr(),
std::make_unique<blink::PendingURLLoaderFactoryBundle>(), base::nullopt,
blink::mojom::ControllerServiceWorkerInfoPtr(),
blink::mojom::ServiceWorkerContainerInfoForClientPtr(),
mojo::NullRemote() /* prefetch_loader_factory */,
base::UnguessableToken::Create(),
base::BindOnce(&MockFrameHost::DidCommitProvisionalLoad,
base::Unretained(mock_frame_host_.get())));
CommitNavigation(std::move(common_params), std::move(commit_params),
std::move(head), mojo::ScopedDataPipeConsumerHandle(),
network::mojom::URLLoaderClientEndpointsPtr(),
std::make_unique<blink::PendingURLLoaderFactoryBundle>(),
base::nullopt,
blink::mojom::ControllerServiceWorkerInfoPtr(),
blink::mojom::ServiceWorkerContainerInfoForClientPtr(),
mojo::NullRemote() /* prefetch_loader_factory */,
base::UnguessableToken::Create(),
base::BindOnce(&MockFrameHost::DidCommitProvisionalLoad,
base::Unretained(mock_frame_host_.get())));
}
void TestRenderFrame::Navigate(mojom::CommonNavigationParamsPtr common_params,
......
......@@ -520,8 +520,6 @@ void TestRenderFrameHost::SendCommitNavigation(
mojo::PendingRemote<network::mojom::URLLoaderFactory>
prefetch_loader_factory,
const base::UnguessableToken& devtools_navigation_token) {
if (!navigation_request)
return;
CHECK(navigation_client);
commit_callback_[navigation_request] =
BuildCommitNavigationCallback(navigation_request);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment