Skip to content

Commit

Permalink
Convert NavigationClient to new Mojo types
Browse files Browse the repository at this point in the history
This CL converts NavigationClientAssociatedPtrInfo,
NavigationClientAssociatedPtr, NavigationClientPtr,
and NavigationClientAssociatedRequest to new
Mojo types.

It also updates BeginNavigation from frame.mojom
and methods and members which implements it with
new Mojo types.

Bug: 955171
Change-Id: I2ed4aa7cdf57361c982e3adc696cc9ead797630b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1797823
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Julie Kim <jkim@igalia.com>
Cr-Commit-Position: refs/heads/master@{#695984}
  • Loading branch information
jkim-julie authored and Commit Bot committed Sep 12, 2019
1 parent 4d0c3bf commit ed2e5ba
Show file tree
Hide file tree
Showing 26 changed files with 139 additions and 109 deletions.
2 changes: 1 addition & 1 deletion content/browser/frame_host/navigation_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2128,7 +2128,7 @@ void NavigationControllerImpl::NotifyUserActivation() {

bool NavigationControllerImpl::StartHistoryNavigationInNewSubframe(
RenderFrameHostImpl* render_frame_host,
mojom::NavigationClientAssociatedPtrInfo* navigation_client) {
mojo::PendingAssociatedRemote<mojom::NavigationClient>* navigation_client) {
NavigationEntryImpl* entry =
GetEntryWithUniqueID(render_frame_host->nav_entry_id());
if (!entry)
Expand Down
4 changes: 3 additions & 1 deletion content/browser/frame_host/navigation_controller_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "content/public/browser/navigation_controller.h"
#include "content/public/browser/navigation_type.h"
#include "content/public/browser/reload_type.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"

struct FrameHostMsg_DidCommitProvisionalLoad_Params;

Expand Down Expand Up @@ -103,7 +104,8 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
// navigation to the default src URL for the frame instead.
bool StartHistoryNavigationInNewSubframe(
RenderFrameHostImpl* render_frame_host,
mojom::NavigationClientAssociatedPtrInfo* navigation_client);
mojo::PendingAssociatedRemote<mojom::NavigationClient>*
navigation_client);

// Navigates to a specified offset from the "current entry". Currently records
// a histogram indicating whether the session history navigation would only
Expand Down
35 changes: 16 additions & 19 deletions content/browser/frame_host/navigation_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -603,8 +603,8 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated(
frame_tree_node, std::move(common_params), std::move(navigation_params),
std::move(commit_params), browser_initiated,
false /* from_begin_navigation */, false /* is_for_commit */, frame_entry,
entry, std::move(navigation_ui_data), nullptr, mojo::NullRemote(),
rfh_restored_from_back_forward_cache));
entry, std::move(navigation_ui_data), mojo::NullAssociatedRemote(),
mojo::NullRemote(), rfh_restored_from_back_forward_cache));

if (frame_entry) {
navigation_request->blob_url_loader_factory_ =
Expand Down Expand Up @@ -637,7 +637,7 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateRendererInitiated(
int current_history_list_length,
bool override_user_agent,
scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory,
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator> navigation_initiator,
scoped_refptr<PrefetchedSignedExchangeCache>
prefetched_signed_exchange_cache) {
Expand Down Expand Up @@ -763,9 +763,8 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateForCommit(
std::move(commit_params), !is_renderer_initiated,
false /* from_begin_navigation */, true /* is_for_commit */,
entry ? entry->GetFrameEntry(frame_tree_node) : nullptr, entry,
nullptr /* navigation_ui_data */,
mojom::NavigationClientAssociatedPtrInfo(), mojo::NullRemote(),
nullptr /* rfh_restored_from_back_forward_cache */));
nullptr /* navigation_ui_data */, mojo::NullAssociatedRemote(),
mojo::NullRemote(), nullptr /* rfh_restored_from_back_forward_cache */));

// Update the state of the NavigationRequest to match the fact that the
// navigation just committed.
Expand All @@ -787,7 +786,7 @@ NavigationRequest::NavigationRequest(
const FrameNavigationEntry* frame_entry,
NavigationEntryImpl* entry,
std::unique_ptr<NavigationUIData> navigation_ui_data,
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator> navigation_initiator,
RenderFrameHostImpl* rfh_restored_from_back_forward_cache)
: frame_tree_node_(frame_tree_node),
Expand All @@ -807,8 +806,8 @@ NavigationRequest::NavigationRequest(
net_error_(net::OK),
expected_render_process_host_id_(ChildProcessHost::kInvalidUniqueID),
devtools_navigation_token_(base::UnguessableToken::Create()),
request_navigation_client_(nullptr),
commit_navigation_client_(nullptr),
request_navigation_client_(mojo::NullAssociatedRemote()),
commit_navigation_client_(mojo::NullAssociatedRemote()),
rfh_restored_from_back_forward_cache_(
rfh_restored_from_back_forward_cache) {
DCHECK(browser_initiated || common_params_->initiator_origin.has_value());
Expand Down Expand Up @@ -2199,7 +2198,7 @@ void NavigationRequest::CommitErrorPage(
// consistent with the URL being requested.
commit_params_->origin_to_commit =
url::Origin::Create(common_params_->url).DeriveNewOpaqueOrigin();
if (IsPerNavigationMojoInterfaceEnabled() && request_navigation_client_ &&
if (IsPerNavigationMojoInterfaceEnabled() &&
request_navigation_client_.is_bound()) {
if (associated_site_instance_id_ ==
render_frame_host_->GetSiteInstance()->GetId()) {
Expand Down Expand Up @@ -2267,7 +2266,7 @@ void NavigationRequest::CommitNavigation() {

frame_tree_node_->TransferNavigationRequestOwnership(render_frame_host_);

if (IsPerNavigationMojoInterfaceEnabled() && request_navigation_client_ &&
if (IsPerNavigationMojoInterfaceEnabled() &&
request_navigation_client_.is_bound()) {
if (associated_site_instance_id_ ==
render_frame_host_->GetSiteInstance()->GetId()) {
Expand Down Expand Up @@ -2622,19 +2621,17 @@ void NavigationRequest::OnRendererAbortedNavigation() {
}

void NavigationRequest::HandleInterfaceDisconnection(
mojom::NavigationClientAssociatedPtr* navigation_client,
mojo::AssociatedRemote<mojom::NavigationClient>* navigation_client,
base::OnceClosure error_handler) {
navigation_client->set_connection_error_handler(std::move(error_handler));
navigation_client->set_disconnect_handler(std::move(error_handler));
}

void NavigationRequest::IgnoreInterfaceDisconnection() {
return request_navigation_client_.set_connection_error_handler(
base::DoNothing());
return request_navigation_client_.set_disconnect_handler(base::DoNothing());
}

void NavigationRequest::IgnoreCommitInterfaceDisconnection() {
return commit_navigation_client_.set_connection_error_handler(
base::DoNothing());
return commit_navigation_client_.set_disconnect_handler(base::DoNothing());
}

bool NavigationRequest::IsSameDocument() const {
Expand Down Expand Up @@ -3082,15 +3079,15 @@ void NavigationRequest::UpdateStateFollowingRedirect(
}

void NavigationRequest::SetNavigationClient(
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
int32_t associated_site_instance_id) {
DCHECK(from_begin_navigation_ ||
common_params_->is_history_navigation_in_new_child_frame);
DCHECK(!request_navigation_client_);
if (!navigation_client.is_valid())
return;

request_navigation_client_ = mojom::NavigationClientAssociatedPtr();
request_navigation_client_.reset();
request_navigation_client_.Bind(std::move(navigation_client));

// Binds the OnAbort callback
Expand Down
44 changes: 24 additions & 20 deletions content/browser/frame_host/navigation_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
#include "content/public/browser/navigation_type.h"
#include "content/public/browser/render_process_host_observer.h"
#include "content/public/common/previews_state.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/system/data_pipe.h"
#include "net/base/proxy_server.h"
Expand Down Expand Up @@ -156,7 +158,7 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate,
int current_history_list_length,
bool override_user_agent,
scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory,
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator>
navigation_initiator,
scoped_refptr<PrefetchedSignedExchangeCache>
Expand Down Expand Up @@ -449,7 +451,7 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate,
// navigation in a subframe. This allows a browser-initiated NavigationRequest
// to be canceled by the renderer.
void SetNavigationClient(
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
int32_t associated_site_instance_id);

// Whether the new document created by this navigation will be loaded from a
Expand Down Expand Up @@ -538,20 +540,21 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate,
private:
friend class NavigationRequestTest;

NavigationRequest(FrameTreeNode* frame_tree_node,
mojom::CommonNavigationParamsPtr common_params,
mojom::BeginNavigationParamsPtr begin_params,
mojom::CommitNavigationParamsPtr commit_params,
bool browser_initiated,
bool from_begin_navigation,
bool is_for_commit,
const FrameNavigationEntry* frame_navigation_entry,
NavigationEntryImpl* navitation_entry,
std::unique_ptr<NavigationUIData> navigation_ui_data,
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator>
navigation_initiator,
RenderFrameHostImpl* rfh_restored_from_back_forward_cache);
NavigationRequest(
FrameTreeNode* frame_tree_node,
mojom::CommonNavigationParamsPtr common_params,
mojom::BeginNavigationParamsPtr begin_params,
mojom::CommitNavigationParamsPtr commit_params,
bool browser_initiated,
bool from_begin_navigation,
bool is_for_commit,
const FrameNavigationEntry* frame_navigation_entry,
NavigationEntryImpl* navitation_entry,
std::unique_ptr<NavigationUIData> navigation_ui_data,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator>
navigation_initiator,
RenderFrameHostImpl* rfh_restored_from_back_forward_cache);

// NavigationURLLoaderDelegate implementation.
void OnRequestRedirected(
Expand Down Expand Up @@ -682,8 +685,9 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate,
// Binds the given error_handler to be called when an interface disconnection
// happens on the renderer side.
// Only used with PerNavigationMojoInterface enabled.
void HandleInterfaceDisconnection(mojom::NavigationClientAssociatedPtr*,
base::OnceClosure error_handler);
void HandleInterfaceDisconnection(
mojo::AssociatedRemote<mojom::NavigationClient>*,
base::OnceClosure error_handler);

// When called, this NavigationRequest will no longer interpret the interface
// disconnection on the renderer side as an AbortNavigation.
Expand Down Expand Up @@ -962,14 +966,14 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate,
// case of a renderer initiated navigation. It is expected to be bound until
// this navigation commits or is canceled.
// Only valid when PerNavigationMojoInterface is enabled.
mojom::NavigationClientAssociatedPtr request_navigation_client_;
mojo::AssociatedRemote<mojom::NavigationClient> request_navigation_client_;
base::Optional<int32_t> associated_site_instance_id_;

// The NavigationClient interface used to commit the navigation. For now, this
// is only used for same-site renderer-initiated navigation.
// TODO(clamy, ahemery): Extend to all types of navigation.
// Only valid when PerNavigationMojoInterface is enabled.
mojom::NavigationClientAssociatedPtr commit_navigation_client_;
mojo::AssociatedRemote<mojom::NavigationClient> commit_navigation_client_;

// If set, any redirects to HTTP for this navigation will be upgraded to
// HTTPS. This is used only on subframe navigations, when
Expand Down
4 changes: 2 additions & 2 deletions content/browser/frame_host/navigator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ NavigationController* Navigator::GetController() {

bool Navigator::StartHistoryNavigationInNewSubframe(
RenderFrameHostImpl* render_frame_host,
mojom::NavigationClientAssociatedPtrInfo* navigation_client) {
mojo::PendingAssociatedRemote<mojom::NavigationClient>* navigation_client) {
return false;
}

Expand All @@ -32,7 +32,7 @@ void Navigator::OnBeginNavigation(
mojom::CommonNavigationParamsPtr common_params,
mojom::BeginNavigationParamsPtr begin_params,
scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory,
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator> navigation_initiator,
scoped_refptr<PrefetchedSignedExchangeCache>
prefetched_signed_exchange_cache) {}
Expand Down
6 changes: 4 additions & 2 deletions content/browser/frame_host/navigator.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "content/common/content_export.h"
#include "content/common/navigation_params.mojom.h"
#include "content/public/browser/navigation_controller.h"
#include "mojo/public/cpp/bindings/pending_associated_remote.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "third_party/blink/public/web/web_triggering_event_info.h"
#include "ui/base/window_open_disposition.h"
Expand Down Expand Up @@ -86,7 +87,8 @@ class CONTENT_EXPORT Navigator : public base::RefCounted<Navigator> {
// FrameNavigationEntry can't be found or the navigation fails.
virtual bool StartHistoryNavigationInNewSubframe(
RenderFrameHostImpl* render_frame_host,
mojom::NavigationClientAssociatedPtrInfo* navigation_client);
mojo::PendingAssociatedRemote<mojom::NavigationClient>*
navigation_client);

// Navigation requests -------------------------------------------------------

Expand Down Expand Up @@ -150,7 +152,7 @@ class CONTENT_EXPORT Navigator : public base::RefCounted<Navigator> {
mojom::CommonNavigationParamsPtr common_params,
mojom::BeginNavigationParamsPtr begin_params,
scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory,
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator>
navigation_initiator,
scoped_refptr<PrefetchedSignedExchangeCache>
Expand Down
4 changes: 2 additions & 2 deletions content/browser/frame_host/navigator_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ void NavigatorImpl::DidFailLoadWithError(

bool NavigatorImpl::StartHistoryNavigationInNewSubframe(
RenderFrameHostImpl* render_frame_host,
mojom::NavigationClientAssociatedPtrInfo* navigation_client) {
mojo::PendingAssociatedRemote<mojom::NavigationClient>* navigation_client) {
return controller_->StartHistoryNavigationInNewSubframe(render_frame_host,
navigation_client);
}
Expand Down Expand Up @@ -618,7 +618,7 @@ void NavigatorImpl::OnBeginNavigation(
mojom::CommonNavigationParamsPtr common_params,
mojom::BeginNavigationParamsPtr begin_params,
scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory,
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator> navigation_initiator,
scoped_refptr<PrefetchedSignedExchangeCache>
prefetched_signed_exchange_cache) {
Expand Down
5 changes: 3 additions & 2 deletions content/browser/frame_host/navigator_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ class CONTENT_EXPORT NavigatorImpl : public Navigator {
bool was_within_same_document) override;
bool StartHistoryNavigationInNewSubframe(
RenderFrameHostImpl* render_frame_host,
mojom::NavigationClientAssociatedPtrInfo* navigation_client) override;
mojo::PendingAssociatedRemote<mojom::NavigationClient>* navigation_client)
override;
void Navigate(std::unique_ptr<NavigationRequest> request,
ReloadType reload_type,
RestoreType restore_type) override;
Expand Down Expand Up @@ -97,7 +98,7 @@ class CONTENT_EXPORT NavigatorImpl : public Navigator {
mojom::CommonNavigationParamsPtr common_params,
mojom::BeginNavigationParamsPtr begin_params,
scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory,
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator>
navigation_initiator,
scoped_refptr<PrefetchedSignedExchangeCache>
Expand Down
16 changes: 8 additions & 8 deletions content/browser/frame_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -542,14 +542,14 @@ struct PendingNavigation {
mojom::CommonNavigationParamsPtr common_params;
mojom::BeginNavigationParamsPtr begin_navigation_params;
scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory;
mojom::NavigationClientAssociatedPtrInfo navigation_client;
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client;
mojo::PendingRemote<blink::mojom::NavigationInitiator> navigation_initiator;

PendingNavigation(
mojom::CommonNavigationParamsPtr common_params,
mojom::BeginNavigationParamsPtr begin_navigation_params,
scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory,
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator>
navigation_initiator);
};
Expand All @@ -558,7 +558,7 @@ PendingNavigation::PendingNavigation(
mojom::CommonNavigationParamsPtr common_params,
mojom::BeginNavigationParamsPtr begin_navigation_params,
scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory,
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator> navigation_initiator)
: common_params(std::move(common_params)),
begin_navigation_params(std::move(begin_navigation_params)),
Expand Down Expand Up @@ -4180,7 +4180,7 @@ void RenderFrameHostImpl::BeginNavigation(
mojom::CommonNavigationParamsPtr common_params,
mojom::BeginNavigationParamsPtr begin_params,
mojo::PendingRemote<blink::mojom::BlobURLToken> blob_url_token,
mojom::NavigationClientAssociatedPtrInfo navigation_client,
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client,
mojo::PendingRemote<blink::mojom::NavigationInitiator>
navigation_initiator) {
if (frame_tree_node_->render_manager()->is_attaching_inner_delegate()) {
Expand Down Expand Up @@ -5943,11 +5943,11 @@ std::set<int> RenderFrameHostImpl::GetNavigationEntryIdsPendingCommit() {
return result;
}

mojom::NavigationClientAssociatedPtr
mojo::AssociatedRemote<mojom::NavigationClient>
RenderFrameHostImpl::GetNavigationClientFromInterfaceProvider() {
mojom::NavigationClientAssociatedPtr navigation_client_ptr;
GetRemoteAssociatedInterfaces()->GetInterface(&navigation_client_ptr);
return navigation_client_ptr;
mojo::AssociatedRemote<mojom::NavigationClient> navigation_client_remote;
GetRemoteAssociatedInterfaces()->GetInterface(&navigation_client_remote);
return navigation_client_remote;
}

void RenderFrameHostImpl::NavigationRequestCancelled(
Expand Down
Loading

0 comments on commit ed2e5ba

Please sign in to comment.