diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index b849e5ce9877cf..b321a295fe3839 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -3254,71 +3254,84 @@ void RenderFrameImpl::CommitNavigation( &item_for_history_navigation, &load_type); } - base::OnceClosure continue_navigation; - if (commit_status == blink::mojom::CommitResult::Ok) { - base::WeakPtr weak_this = weak_factory_.GetWeakPtr(); - // Check if the navigation being committed originated as a client redirect. - bool is_client_redirect = - !!(common_params.transition & ui::PAGE_TRANSITION_CLIENT_REDIRECT); - - // Perform a navigation for loadDataWithBaseURL if needed (for main frames). - // Note: the base URL might be invalid, so also check the data URL string. - bool should_load_data_url = !common_params.base_url_for_data_url.is_empty(); -#if defined(OS_ANDROID) - should_load_data_url |= !request_params.data_url_as_string.empty(); -#endif - if (is_main_frame_ && should_load_data_url) { - LoadDataURL(common_params, request_params, load_type, - item_for_history_navigation, is_client_redirect, - std::move(document_state)); - } else { - WebURLRequest request = CreateURLRequestForCommit( - common_params, request_params, std::move(url_loader_client_endpoints), - head); - // Note: we cannot use base::AutoReset here, since |this| can be deleted - // in the next call and auto reset will introduce use-after-free bug. - committing_main_request_ = true; - frame_->CommitNavigation( - request, load_type, item_for_history_navigation, is_client_redirect, - devtools_navigation_token, - BuildNavigationParams( - common_params, request_params, - BuildServiceWorkerNetworkProviderForNavigation( - &request_params, std::move(controller_service_worker_info))), - std::move(document_state)); - // The commit can result in this frame being removed. Use a - // WeakPtr as an easy way to detect whether this has occured. If so, this - // method should return immediately and not touch any part of the object, - // otherwise it will result in a use-after-free bug. - if (!weak_this) - return; - committing_main_request_ = false; - RequestExtraData* extra_data = - static_cast(request.GetExtraData()); - continue_navigation = - extra_data->TakeContinueNavigationFunctionOwnerShip(); - } - } else { + if (commit_status != blink::mojom::CommitResult::Ok) { // The browser expects the frame to be loading this navigation. Inform it // that the load stopped if needed. if (frame_ && !frame_->IsLoading()) Send(new FrameHostMsg_DidStopLoading(routing_id_)); + return; } - // Reset the source location now that the commit checks have been processed. - frame_->GetDocumentLoader()->ResetSourceLocation(); - if (frame_->GetProvisionalDocumentLoader()) - frame_->GetProvisionalDocumentLoader()->ResetSourceLocation(); + base::WeakPtr weak_this = weak_factory_.GetWeakPtr(); + // Check if the navigation being committed originated as a client redirect. + bool is_client_redirect = + !!(common_params.transition & ui::PAGE_TRANSITION_CLIENT_REDIRECT); + + std::unique_ptr navigation_params = + BuildNavigationParams( + common_params, request_params, + BuildServiceWorkerNetworkProviderForNavigation( + &request_params, std::move(controller_service_worker_info))); - // Continue the navigation. - // TODO(arthursonzogni): Pass the data needed to continue the navigation to - // this function instead of storing it in the - // NavigationResponseOverrideParameters. The architecture of committing the - // navigation in the renderer process should be simplified and avoid going - // through the ResourceFetcher for the main resource. - if (continue_navigation) { - base::AutoReset replaying(&replaying_main_response_, true); - std::move(continue_navigation).Run(); + // Perform a navigation to a data url if needed (for main frames). + // Note: the base URL might be invalid, so also check the data URL string. + bool should_load_data_url = !common_params.base_url_for_data_url.is_empty(); +#if defined(OS_ANDROID) + should_load_data_url |= !request_params.data_url_as_string.empty(); +#endif + if (is_main_frame_ && should_load_data_url) { + std::string mime_type; + std::string charset; + std::string data; + GURL base_url; + DecodeDataURL(common_params, request_params, &mime_type, &charset, &data, + &base_url); + frame_->CommitDataNavigation( + WebURLRequest(base_url), WebData(data.c_str(), data.length()), + WebString::FromUTF8(mime_type), WebString::FromUTF8(charset), + // Needed so that history-url-only changes don't become reloads. + common_params.history_url_for_data_url, load_type, + item_for_history_navigation, is_client_redirect, + std::move(navigation_params), std::move(document_state)); + // The commit can result in this frame being removed. Use a + // WeakPtr as an easy way to detect whether this has occured. If so, this + // method should return immediately and not touch any part of the object, + // otherwise it will result in a use-after-free bug. + if (!weak_this) + return; + } else { + WebURLRequest request = + CreateURLRequestForCommit(common_params, request_params, + std::move(url_loader_client_endpoints), head); + // Note: we cannot use base::AutoReset here, since |this| can be deleted + // in the next call and auto reset will introduce use-after-free bug. + committing_main_request_ = true; + frame_->CommitNavigation(request, load_type, item_for_history_navigation, + is_client_redirect, devtools_navigation_token, + std::move(navigation_params), + std::move(document_state)); + // The commit can result in this frame being removed. Use a + // WeakPtr as an easy way to detect whether this has occured. If so, this + // method should return immediately and not touch any part of the object, + // otherwise it will result in a use-after-free bug. + if (!weak_this) + return; + committing_main_request_ = false; + + // Continue the navigation. + // TODO(arthursonzogni): Pass the data needed to continue the navigation to + // this function instead of storing it in the + // NavigationResponseOverrideParameters. The architecture of committing the + // navigation in the renderer process should be simplified and avoid going + // through the ResourceFetcher for the main resource. + RequestExtraData* extra_data = + static_cast(request.GetExtraData()); + base::OnceClosure continue_navigation = + extra_data->TakeContinueNavigationFunctionOwnerShip(); + if (continue_navigation) { + base::AutoReset replaying(&replaying_main_response_, true); + std::move(continue_navigation).Run(); + } } } @@ -6835,22 +6848,23 @@ void RenderFrameImpl::BeginNavigationInternal( } } -void RenderFrameImpl::LoadDataURL( +void RenderFrameImpl::DecodeDataURL( const CommonNavigationParams& common_params, const RequestNavigationParams& request_params, - blink::WebFrameLoadType load_type, - blink::WebHistoryItem item_for_history_navigation, - bool is_client_redirect, - std::unique_ptr navigation_data) { + std::string* mime_type, + std::string* charset, + std::string* data, + GURL* base_url) { // A loadData request with a specified base URL. GURL data_url = common_params.url; #if defined(OS_ANDROID) if (!request_params.data_url_as_string.empty()) { #if DCHECK_IS_ON() { - std::string mime_type, charset, data; - DCHECK(net::DataURL::Parse(data_url, &mime_type, &charset, &data)); - DCHECK(data.empty()); + std::string mime_type_tmp, charset_tmp, data_tmp; + DCHECK(net::DataURL::Parse(data_url, &mime_type_tmp, &charset_tmp, + &data_tmp)); + DCHECK(data_tmp.empty()); } #endif data_url = GURL(request_params.data_url_as_string); @@ -6859,23 +6873,10 @@ void RenderFrameImpl::LoadDataURL( } } #endif - std::string mime_type, charset, data; - if (net::DataURL::Parse(data_url, &mime_type, &charset, &data)) { - const GURL base_url = common_params.base_url_for_data_url.is_empty() - ? common_params.url - : common_params.base_url_for_data_url; - - frame_->CommitDataNavigation( - WebURLRequest(base_url), WebData(data.c_str(), data.length()), - WebString::FromUTF8(mime_type), WebString::FromUTF8(charset), - // Needed so that history-url-only changes don't become reloads. - common_params.history_url_for_data_url, load_type, - item_for_history_navigation, is_client_redirect, - BuildNavigationParams( - common_params, request_params, - BuildServiceWorkerNetworkProviderForNavigation( - &request_params, nullptr /* controller_service_worker_info */)), - std::move(navigation_data)); + if (net::DataURL::Parse(data_url, mime_type, charset, data)) { + *base_url = common_params.base_url_for_data_url.is_empty() + ? common_params.url + : common_params.base_url_for_data_url; } else { CHECK(false) << "Invalid URL passed: " << common_params.url.possibly_invalid_spec(); diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h index dac0a331197352..c39fe81d772839 100644 --- a/content/renderer/render_frame_impl.h +++ b/content/renderer/render_frame_impl.h @@ -1151,14 +1151,13 @@ class CONTENT_EXPORT RenderFrameImpl // Sends a FrameHostMsg_BeginNavigation to the browser void BeginNavigationInternal(std::unique_ptr info); - // Loads a data url. - void LoadDataURL( - const CommonNavigationParams& common_params, - const RequestNavigationParams& request_params, - blink::WebFrameLoadType load_type, - blink::WebHistoryItem item_for_history_navigation, - bool is_client_redirect, - std::unique_ptr navigation_data); + // Decodes a data url for navigation commit. + void DecodeDataURL(const CommonNavigationParams& common_params, + const RequestNavigationParams& request_params, + std::string* mime_type, + std::string* charset, + std::string* data, + GURL* base_url); // Sends a proper FrameHostMsg_DidFailProvisionalLoadWithError_Params IPC for // the failed request |request|. diff --git a/third_party/blink/public/web/web_document_loader.h b/third_party/blink/public/web/web_document_loader.h index b11eb4d45b2b1f..b45799fc11c77b 100644 --- a/third_party/blink/public/web/web_document_loader.h +++ b/third_party/blink/public/web/web_document_loader.h @@ -119,8 +119,6 @@ class BLINK_EXPORT WebDocumentLoader { virtual WebServiceWorkerNetworkProvider* GetServiceWorkerNetworkProvider() = 0; - virtual void ResetSourceLocation() = 0; - // Can be used to temporarily suspend feeding the parser with new data. The // parser will be allowed to read new data when ResumeParser() is called the // same number of time than BlockParser(). diff --git a/third_party/blink/renderer/core/exported/web_document_loader_impl.cc b/third_party/blink/renderer/core/exported/web_document_loader_impl.cc index f7dc115595f390..c2a427ae071098 100644 --- a/third_party/blink/renderer/core/exported/web_document_loader_impl.cc +++ b/third_party/blink/renderer/core/exported/web_document_loader_impl.cc @@ -144,10 +144,6 @@ WebDocumentLoaderImpl::GetServiceWorkerNetworkProvider() { return DocumentLoader::GetServiceWorkerNetworkProvider(); } -void WebDocumentLoaderImpl::ResetSourceLocation() { - DocumentLoader::ResetSourceLocation(); -} - void WebDocumentLoaderImpl::BlockParser() { DocumentLoader::BlockParser(); } diff --git a/third_party/blink/renderer/core/exported/web_document_loader_impl.h b/third_party/blink/renderer/core/exported/web_document_loader_impl.h index d83f96afc82c3c..b4b27c73fa86fd 100644 --- a/third_party/blink/renderer/core/exported/web_document_loader_impl.h +++ b/third_party/blink/renderer/core/exported/web_document_loader_impl.h @@ -79,7 +79,6 @@ class CORE_EXPORT WebDocumentLoaderImpl final : public DocumentLoader, void SetServiceWorkerNetworkProvider( std::unique_ptr) override; WebServiceWorkerNetworkProvider* GetServiceWorkerNetworkProvider() override; - void ResetSourceLocation() override; void BlockParser() override; void ResumeParser() override; bool IsArchive() const override; diff --git a/third_party/blink/renderer/core/loader/document_loader.cc b/third_party/blink/renderer/core/loader/document_loader.cc index 8be6c78f48a545..2856aa9d655e69 100644 --- a/third_party/blink/renderer/core/loader/document_loader.cc +++ b/third_party/blink/renderer/core/loader/document_loader.cc @@ -300,10 +300,6 @@ void DocumentLoader::SetServiceWorkerNetworkProvider( service_worker_network_provider_ = std::move(provider); } -void DocumentLoader::ResetSourceLocation() { - source_location_ = nullptr; -} - std::unique_ptr DocumentLoader::CopySourceLocation() const { return source_location_ ? source_location_->Clone() : nullptr; } @@ -966,8 +962,10 @@ void DocumentLoader::StartLoading() { DCHECK_EQ(state_, kNotStarted); state_ = kProvisional; - if (MaybeLoadEmpty()) + if (MaybeLoadEmpty()) { + source_location_ = nullptr; return; + } DCHECK(!GetTiming().NavigationStart().is_null()); // The fetch has already started in the browser, @@ -984,6 +982,7 @@ void DocumentLoader::StartLoading() { // make some modification to the request, e.g. adding the referer header. request_ = GetResource()->IsLoading() ? GetResource()->GetResourceRequest() : fetch_params.GetResourceRequest(); + source_location_ = nullptr; } void DocumentLoader::DidInstallNewDocument(Document* document) { diff --git a/third_party/blink/renderer/core/loader/document_loader.h b/third_party/blink/renderer/core/loader/document_loader.h index 5f96048de77f4b..3d096766b0e036 100644 --- a/third_party/blink/renderer/core/loader/document_loader.h +++ b/third_party/blink/renderer/core/loader/document_loader.h @@ -224,8 +224,7 @@ class CORE_EXPORT DocumentLoader return service_worker_network_provider_.get(); } - // Allows to specify the SourceLocation that triggered the navigation. - void ResetSourceLocation(); + // Returns a SourceLocation that triggered the navigation if any. std::unique_ptr CopySourceLocation() const; void LoadFailed(const ResourceError&);