From 69178d519e730f9023fa9af6979f8a78922854ba Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Tue, 28 Nov 2017 18:00:50 -0500 Subject: [PATCH] Fix bugs and test failures with browser-side navigation (issue #2290) --- BUILD.gn | 2 + libcef/browser/browser_host_impl.cc | 228 +++++++++++++----- libcef/browser/browser_host_impl.h | 42 +++- libcef/browser/content_browser_client.cc | 27 +-- libcef/browser/extensions/extension_system.cc | 2 +- libcef/browser/frame_host_impl.cc | 4 +- libcef/browser/frame_host_impl.h | 3 +- libcef/browser/net/net_util.cc | 22 ++ libcef/browser/net/net_util.h | 21 ++ libcef/browser/net/network_delegate.cc | 32 ++- libcef/browser/net/network_delegate.h | 4 - libcef/browser/net/url_request_interceptor.cc | 24 +- .../browser/plugins/plugin_service_filter.cc | 4 + libcef_dll/wrapper_types.h | 2 +- tests/ceftests/frame_unittest.cc | 113 ++++++--- tests/ceftests/navigation_unittest.cc | 87 ++++++- tests/ceftests/plugin_unittest.cc | 1 - tests/ceftests/request_context_unittest.cc | 8 +- tests/ceftests/request_unittest.cc | 10 +- tests/ceftests/scheme_handler_unittest.cc | 8 +- tests/ceftests/test_handler.cc | 16 ++ tests/ceftests/test_handler.h | 3 + 22 files changed, 481 insertions(+), 182 deletions(-) create mode 100644 libcef/browser/net/net_util.cc create mode 100644 libcef/browser/net/net_util.h diff --git a/BUILD.gn b/BUILD.gn index 945cc81a8..52e1e9616 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -450,6 +450,8 @@ static_library("libcef_static") { "libcef/browser/net/devtools_scheme_handler.h", "libcef/browser/net/internal_scheme_handler.cc", "libcef/browser/net/internal_scheme_handler.h", + "libcef/browser/net/net_util.cc", + "libcef/browser/net/net_util.h", "libcef/browser/net/network_delegate.cc", "libcef/browser/net/network_delegate.h", "libcef/browser/net/resource_request_job.cc", diff --git a/libcef/browser/browser_host_impl.cc b/libcef/browser/browser_host_impl.cc index 4e7aed9cb..20de140a3 100644 --- a/libcef/browser/browser_host_impl.cc +++ b/libcef/browser/browser_host_impl.cc @@ -82,6 +82,10 @@ using content::KeyboardEventProcessingResult; namespace { +const int kUnspecifiedFrameTreeNodeId = -3; +const int kMainFrameTreeNodeId = -2; +const int kUnusedFrameTreeNodeId = -1; + // Associates a CefBrowserHostImpl instance with a WebContents. This object will // be deleted automatically when the WebContents is destroyed. class WebContentsUserDataAdapter : public base::SupportsUserData::Data { @@ -1390,7 +1394,9 @@ CefRefPtr CefBrowserHostImpl::GetFrame(int64 identifier) { if (main_frame_id_ == CefFrameHostImpl::kInvalidFrameId) { // A main frame does not exist yet. Return the placeholder frame that // provides limited functionality. - return placeholder_frame_.get(); + return GetOrCreatePendingFrame(kMainFrameTreeNodeId, + CefFrameHostImpl::kInvalidFrameId, nullptr) + .get(); } if (identifier == CefFrameHostImpl::kMainFrameId) { @@ -1406,7 +1412,7 @@ CefRefPtr CefBrowserHostImpl::GetFrame(int64 identifier) { if (identifier == CefFrameHostImpl::kInvalidFrameId) return nullptr; - FrameMap::const_iterator it = frames_.find(identifier); + FrameIdMap::const_iterator it = frames_.find(identifier); if (it != frames_.end()) return it->second.get(); @@ -1416,7 +1422,7 @@ CefRefPtr CefBrowserHostImpl::GetFrame(int64 identifier) { CefRefPtr CefBrowserHostImpl::GetFrame(const CefString& name) { base::AutoLock lock_scope(state_lock_); - FrameMap::const_iterator it = frames_.begin(); + FrameIdMap::const_iterator it = frames_.begin(); for (; it != frames_.end(); ++it) { if (it->second->GetName() == name) return it->second.get(); @@ -1436,7 +1442,7 @@ void CefBrowserHostImpl::GetFrameIdentifiers(std::vector& identifiers) { if (identifiers.size() > 0) identifiers.clear(); - FrameMap::const_iterator it = frames_.begin(); + FrameIdMap::const_iterator it = frames_.begin(); for (; it != frames_.end(); ++it) identifiers.push_back(it->first); } @@ -1447,7 +1453,7 @@ void CefBrowserHostImpl::GetFrameNames(std::vector& names) { if (names.size() > 0) names.clear(); - FrameMap::const_iterator it = frames_.begin(); + FrameIdMap::const_iterator it = frames_.begin(); for (; it != frames_.end(); ++it) names.push_back(it->second->GetName()); } @@ -1578,7 +1584,7 @@ CefRefPtr CefBrowserHostImpl::GetFrameForRequest( content::ResourceRequestInfo::ForRequest(request); if (!info) return nullptr; - return GetOrCreateFrame(info->GetRenderFrameID(), + return GetOrCreateFrame(info->GetRenderFrameID(), info->GetFrameTreeNodeId(), CefFrameHostImpl::kUnspecifiedFrameId, info->IsMainFrame(), base::string16(), GURL()); } @@ -2198,10 +2204,10 @@ void CefBrowserHostImpl::AddNewContents(content::WebContents* source, void CefBrowserHostImpl::LoadingStateChanged(content::WebContents* source, bool to_different_document) { const int current_index = - web_contents_->GetController().GetLastCommittedEntryIndex(); - const int max_index = web_contents_->GetController().GetEntryCount() - 1; + source->GetController().GetLastCommittedEntryIndex(); + const int max_index = source->GetController().GetEntryCount() - 1; - const bool is_loading = web_contents_->IsLoading(); + const bool is_loading = source->IsLoading(); const bool can_go_back = (current_index > 0); const bool can_go_forward = (current_index < max_index); @@ -2631,17 +2637,26 @@ void CefBrowserHostImpl::FrameDeleted( base::AutoLock lock_scope(state_lock_); - const int64 frame_id = render_frame_host->GetRoutingID(); - FrameMap::iterator it = frames_.find(frame_id); - if (it != frames_.end()) { - it->second->Detach(); - frames_.erase(it); + if (render_routing_id >= 0) { + FrameIdMap::iterator it = frames_.find(render_routing_id); + if (it != frames_.end()) { + it->second->Detach(); + frames_.erase(it); + } + + if (main_frame_id_ == render_routing_id) + main_frame_id_ = CefFrameHostImpl::kInvalidFrameId; + if (focused_frame_id_ == render_routing_id) + focused_frame_id_ = CefFrameHostImpl::kInvalidFrameId; } - if (main_frame_id_ == frame_id) - main_frame_id_ = CefFrameHostImpl::kInvalidFrameId; - if (focused_frame_id_ == frame_id) - focused_frame_id_ = CefFrameHostImpl::kInvalidFrameId; + if (frame_tree_node_id >= 0) { + FrameTreeNodeIdMap::iterator it = pending_frames_.find(frame_tree_node_id); + if (it != pending_frames_.end()) { + it->second->Detach(); + pending_frames_.erase(it); + } + } } void CefBrowserHostImpl::RenderViewCreated( @@ -2654,6 +2669,9 @@ void CefBrowserHostImpl::RenderViewCreated( content::Source(render_view_host)); } + // RenderFrameCreated is otherwise not called for new popup browsers. + RenderFrameCreated(render_view_host->GetMainFrame()); + platform_delegate_->RenderViewCreated(render_view_host); } @@ -2706,25 +2724,28 @@ void CefBrowserHostImpl::RenderProcessGone(base::TerminationStatus status) { void CefBrowserHostImpl::DidFinishNavigation( content::NavigationHandle* navigation_handle) { - // This method may be called with a nullptr RenderFrameHost (RFH) when a - // provisional load is started. It should be called again with a non-nullptr - // RFH once the provisional load is committed or if the provisional load - // fails. - if (!navigation_handle->GetRenderFrameHost()) - return; - const net::Error error_code = navigation_handle->GetNetErrorCode(); + + // With PlzNavigate the RenderFrameHost will only be nullptr if the + // provisional load fails, in which case |error_code| will be ERR_ABORTED. + DCHECK(navigation_handle->GetRenderFrameHost() || + error_code == net::ERR_ABORTED); + + const int64 frame_id = + navigation_handle->GetRenderFrameHost() + ? navigation_handle->GetRenderFrameHost()->GetRoutingID() + : CefFrameHostImpl::kUnspecifiedFrameId; + const bool is_main_frame = navigation_handle->IsInMainFrame(); + const GURL& url = + (error_code == net::OK ? navigation_handle->GetURL() : GURL()); + + CefRefPtr frame = + GetOrCreateFrame(frame_id, navigation_handle->GetFrameTreeNodeId(), + CefFrameHostImpl::kUnspecifiedFrameId, is_main_frame, + base::string16(), url); + if (error_code == net::OK) { // The navigation has been committed. - const bool is_main_frame = navigation_handle->IsInMainFrame(); - const GURL& url = navigation_handle->GetURL(); - - // This also updates the URL associated with the frame. - CefRefPtr frame = GetOrCreateFrame( - navigation_handle->GetRenderFrameHost()->GetRoutingID(), - CefFrameHostImpl::kUnspecifiedFrameId, is_main_frame, base::string16(), - url); - // Don't call OnLoadStart for same page navigations (fragments, // history state). if (!navigation_handle->IsSameDocument()) @@ -2735,11 +2756,6 @@ void CefBrowserHostImpl::DidFinishNavigation( } else { // The navigation failed before commit. Originates from // RenderFrameHostImpl::OnDidFailProvisionalLoadWithError. - CefRefPtr frame = GetOrCreateFrame( - navigation_handle->GetRenderFrameHost()->GetRoutingID(), - CefFrameHostImpl::kUnspecifiedFrameId, - navigation_handle->IsInMainFrame(), base::string16(), GURL()); - // OnLoadStart/OnLoadEnd will not be called. OnLoadError(frame, navigation_handle->GetURL(), error_code); } @@ -2768,9 +2784,11 @@ void CefBrowserHostImpl::DidFailLoad( // The navigation failed after commit. OnLoadStart was called so we also call // OnLoadEnd. const bool is_main_frame = !render_frame_host->GetParent(); - CefRefPtr frame = GetOrCreateFrame( - render_frame_host->GetRoutingID(), CefFrameHostImpl::kUnspecifiedFrameId, - is_main_frame, base::string16(), validated_url); + CefRefPtr frame = + GetOrCreateFrame(render_frame_host->GetRoutingID(), + render_frame_host->GetFrameTreeNodeId(), + CefFrameHostImpl::kUnspecifiedFrameId, is_main_frame, + base::string16(), validated_url); OnLoadError(frame, validated_url, error_code); OnLoadEnd(frame, validated_url, error_code); } @@ -2922,7 +2940,8 @@ void CefBrowserHostImpl::OnFrameIdentified(int64 frame_id, int64 parent_frame_id, base::string16 name) { bool is_main_frame = (parent_frame_id == CefFrameHostImpl::kMainFrameId); - GetOrCreateFrame(frame_id, parent_frame_id, is_main_frame, name, GURL()); + GetOrCreateFrame(frame_id, kUnspecifiedFrameTreeNodeId, parent_frame_id, + is_main_frame, name, GURL()); } void CefBrowserHostImpl::OnFrameFocused( @@ -2937,13 +2956,13 @@ void CefBrowserHostImpl::OnFrameFocused( if (focused_frame_id_ != CefFrameHostImpl::kInvalidFrameId) { // Unfocus the previously focused frame. - FrameMap::const_iterator it = frames_.find(frame_id); + FrameIdMap::const_iterator it = frames_.find(frame_id); if (it != frames_.end()) unfocused_frame = it->second; } // Focus the newly focused frame. - FrameMap::iterator it = frames_.find(frame_id); + FrameIdMap::iterator it = frames_.find(frame_id); if (it != frames_.end()) focused_frame = it->second; @@ -2962,8 +2981,9 @@ void CefBrowserHostImpl::OnDidFinishLoad(int64 frame_id, bool is_main_frame, int http_status_code) { CefRefPtr frame = - GetOrCreateFrame(frame_id, CefFrameHostImpl::kUnspecifiedFrameId, - is_main_frame, base::string16(), validated_url); + GetOrCreateFrame(frame_id, kUnspecifiedFrameTreeNodeId, + CefFrameHostImpl::kUnspecifiedFrameId, is_main_frame, + base::string16(), validated_url); // Give internal scheme handlers an opportunity to update content. scheme::DidFinishLoad(frame, validated_url); @@ -3117,10 +3137,6 @@ CefBrowserHostImpl::CefBrowserHostImpl( response_manager_.reset(new CefResponseManager); - placeholder_frame_ = new CefFrameHostImpl( - this, CefFrameHostImpl::kInvalidFrameId, true, CefString(), CefString(), - CefFrameHostImpl::kInvalidFrameId); - PrefsTabHelper::CreateForWebContents(web_contents_.get()); printing::CefPrintViewManager::CreateForWebContents(web_contents_.get()); @@ -3209,13 +3225,13 @@ void CefBrowserHostImpl::OnExtensionHostDeleted() { CefRefPtr CefBrowserHostImpl::GetOrCreateFrame( int64 frame_id, + int frame_tree_node_id, int64 parent_frame_id, bool is_main_frame, base::string16 frame_name, const GURL& frame_url) { - DCHECK(frame_id > CefFrameHostImpl::kInvalidFrameId); - if (frame_id <= CefFrameHostImpl::kInvalidFrameId) - return nullptr; + // We need either a valid |frame_id| or a valid |frame_tree_node_id|. + DCHECK(frame_id >= 0 || frame_tree_node_id >= kUnusedFrameTreeNodeId); CefString url; if (frame_url.is_valid()) @@ -3228,13 +3244,50 @@ CefRefPtr CefBrowserHostImpl::GetOrCreateFrame( CefRefPtr frame; bool frame_created = false; - { - base::AutoLock lock_scope(state_lock_); + base::AutoLock lock_scope(state_lock_); + if (frame_id < 0) { + // With PlzNavigate the renderer process representation might not exist yet. + if (is_main_frame && main_frame_id_ != CefFrameHostImpl::kInvalidFrameId) { + // Operating in the main frame. Continue using the existing main frame + // object until the new renderer process representation is created. + frame_id = main_frame_id_; + } else { + if (is_main_frame) { + // Always use the same pending object for the main frame. + frame_tree_node_id = kMainFrameTreeNodeId; + } + + // Operating in a sub-frame, or the main frame hasn't yet navigated for + // the first time. Use a pending object keyed on |frame_tree_node_id|. + frame = GetOrCreatePendingFrame(frame_tree_node_id, parent_frame_id, + &frame_created); + } + } + + if (!frame) { + // Delete the pending object, if any. + { + FrameTreeNodeIdMap::iterator it = + pending_frames_.find(frame_tree_node_id); + if (it != pending_frames_.end()) { + DCHECK_EQ(is_main_frame, it->second->IsMain()); + + // Persist URL and name to the new frame. + if (url.empty()) + url = it->second->GetURL(); + if (name.empty()) + name = it->second->GetName(); + + pending_frames_.erase(it); + } + } + + // Update the main frame object if the ID has changed. if (is_main_frame && main_frame_id_ != frame_id) { if (main_frame_id_ != CefFrameHostImpl::kInvalidFrameId) { // Remove the old main frame object before adding the new one. - FrameMap::iterator it = frames_.find(main_frame_id_); + FrameIdMap::iterator it = frames_.find(main_frame_id_); if (it != frames_.end()) { // Persist URL and name to the new main frame. if (url.empty()) @@ -3249,15 +3302,18 @@ CefRefPtr CefBrowserHostImpl::GetOrCreateFrame( if (focused_frame_id_ == main_frame_id_) focused_frame_id_ = frame_id; } + main_frame_id_ = frame_id; } - // Check if a frame object already exists. - FrameMap::const_iterator it = frames_.find(frame_id); - if (it != frames_.end()) - frame = it->second.get(); + // Check if a frame object already exists for the ID. If so, re-use it. + { + FrameIdMap::const_iterator it = frames_.find(frame_id); + if (it != frames_.end()) + frame = it->second; + } - if (!frame.get()) { + if (!frame) { frame = new CefFrameHostImpl(this, frame_id, is_main_frame, url, name, parent_frame_id); frame_created = true; @@ -3266,13 +3322,40 @@ CefRefPtr CefBrowserHostImpl::GetOrCreateFrame( } if (!frame_created) - frame->SetAttributes(url, name, parent_frame_id); + frame->SetAttributes(is_main_frame, url, name, parent_frame_id); return frame.get(); } +CefRefPtr CefBrowserHostImpl::GetOrCreatePendingFrame( + int frame_tree_node_id, + int64 parent_frame_id, + bool* created) { + const bool is_main_frame = (frame_tree_node_id == kMainFrameTreeNodeId); + DCHECK(is_main_frame || frame_tree_node_id >= 0); + + state_lock_.AssertAcquired(); + + FrameTreeNodeIdMap::const_iterator it = + pending_frames_.find(frame_tree_node_id); + if (it != pending_frames_.end()) { + DCHECK_EQ(is_main_frame, it->second->IsMain()); + return it->second; + } + + CefRefPtr frame = new CefFrameHostImpl( + this, CefFrameHostImpl::kInvalidFrameId, is_main_frame, CefString(), + CefString(), parent_frame_id); + pending_frames_.insert(std::make_pair(frame_tree_node_id, frame)); + if (created) + *created = true; + + return frame; +} + void CefBrowserHostImpl::DetachAllFrames() { - FrameMap frames; + FrameIdMap frames; + FrameTreeNodeIdMap pending_frames; { base::AutoLock lock_scope(state_lock_); @@ -3280,15 +3363,26 @@ void CefBrowserHostImpl::DetachAllFrames() { frames = frames_; frames_.clear(); + pending_frames = pending_frames_; + pending_frames_.clear(); + if (main_frame_id_ != CefFrameHostImpl::kInvalidFrameId) main_frame_id_ = CefFrameHostImpl::kInvalidFrameId; if (focused_frame_id_ != CefFrameHostImpl::kInvalidFrameId) focused_frame_id_ = CefFrameHostImpl::kInvalidFrameId; } - FrameMap::const_iterator it = frames.begin(); - for (; it != frames.end(); ++it) - it->second->Detach(); + { + FrameIdMap::const_iterator it = frames.begin(); + for (; it != frames.end(); ++it) + it->second->Detach(); + } + + { + FrameTreeNodeIdMap::const_iterator it = pending_frames.begin(); + for (; it != pending_frames.end(); ++it) + it->second->Detach(); + } } gfx::Point CefBrowserHostImpl::GetScreenPoint(const gfx::Point& view) const { diff --git a/libcef/browser/browser_host_impl.h b/libcef/browser/browser_host_impl.h index b65c97c5a..12bd2a9d3 100644 --- a/libcef/browser/browser_host_impl.h +++ b/libcef/browser/browser_host_impl.h @@ -586,13 +586,36 @@ class CefBrowserHostImpl : public CefBrowserHost, void DestroyExtensionHost(); void OnExtensionHostDeleted(); - // Updates and returns an existing frame or creates a new frame. Pass - // CefFrameHostImpl::kUnspecifiedFrameId for |parent_frame_id| if unknown. + // Update or create a frame object. |frame_id| (renderer routing id) will be + // >= 0 if the frame currently exists in the renderer process. |frame_id| will + // be < 0 for the main frame if it has not yet navigated for the first time, + // or for sub-frames if PlzNavigate is enabled and the sub-frame does not yet + // have a renderer process representation. |frame_tree_node_id| will be + // kUnspecifiedFrameTreeNodeId for calls that originate from the renderer + // process (meaning that |frame_id| should be >= 0); kUnusedFrameTreeNodeId + // if PlzNavigate is disabled; or >= 0 otherwise. |parent_frame_id| will be + // CefFrameHostImpl::kUnspecifiedFrameId if unknown. In cases where |frame_id| + // is < 0 either the existing main frame object or a pending object will be + // returned depending on current state. CefRefPtr GetOrCreateFrame(int64 frame_id, + int frame_tree_node_id, int64 parent_frame_id, bool is_main_frame, base::string16 frame_name, const GURL& frame_url); + + // Returns a pending frame object. If the main frame has not yet navigated for + // the first time then |frame_tree_node_id| will be kMainFrameTreeNodeId and a + // single pending object will be returned. Otherwise, this method will be + // called with a |frame_tree_node_id| value >= 0 when PlzNavigate is enabled + // and there will then be one pending object for each frame that does not yet + // have a renderer process representation. |parent_frame_id| will be + // CefFrameHostImpl::kUnspecifiedFrameId if unknown. |created| will be set to + // true if |created| is non-nullptr and the frame object was created. + CefRefPtr GetOrCreatePendingFrame(int frame_tree_node_id, + int64 parent_frame_id, + bool* created); + // Remove the references to all frames and mark them as detached. void DetachAllFrames(); @@ -636,15 +659,20 @@ class CefBrowserHostImpl : public CefBrowserHost, std::queue queued_messages_; bool queue_messages_; - // Map of unique frame ids to CefFrameHostImpl references. - typedef std::map> FrameMap; - FrameMap frames_; + // Map of frame tree node id to CefFrameHostImpl. These are frames that do not + // yet have a renderer process representation. + typedef std::map> FrameTreeNodeIdMap; + FrameTreeNodeIdMap pending_frames_; + + // Map of unique frame id (renderer routing id) to CefFrameHostImpl. These are + // frames that do have a renderer process representation. + typedef std::map> FrameIdMap; + FrameIdMap frames_; + // The unique frame id currently identified as the main frame. int64 main_frame_id_; // The unique frame id currently identified as the focused frame. int64 focused_frame_id_; - // Used when no other frame exists. Provides limited functionality. - CefRefPtr placeholder_frame_; // Represents the current browser destruction state. Only accessed on the UI // thread. diff --git a/libcef/browser/content_browser_client.cc b/libcef/browser/content_browser_client.cc index 0537caa16..de5f4dccd 100644 --- a/libcef/browser/content_browser_client.cc +++ b/libcef/browser/content_browser_client.cc @@ -433,16 +433,6 @@ bool NavigationOnUIThread( return ignore_navigation; } -void FindFrameHostForNavigationHandle( - content::NavigationHandle* navigation_handle, - content::RenderFrameHost** matching_frame_host, - content::RenderFrameHost* current_frame_host) { - content::RenderFrameHostImpl* current_impl = - static_cast(current_frame_host); - if (current_impl->navigation_handle() == navigation_handle) - *matching_frame_host = current_frame_host; -} - } // namespace CefContentBrowserClient::CefContentBrowserClient() : browser_main_parts_(NULL) { @@ -919,19 +909,12 @@ CefContentBrowserClient::CreateThrottlesForNavigation( int64 parent_frame_id = CefFrameHostImpl::kUnspecifiedFrameId; if (!is_main_frame) { - // Identify the RenderFrameHostImpl that originated the navigation. - // TODO(cef): It would be better if NavigationHandle could directly report - // the owner RenderFrameHostImpl. - // There is additional complexity here if PlzNavigate is enabled. See - // comments in content/browser/frame_host/navigation_handle_impl.h. - content::WebContents* web_contents = navigation_handle->GetWebContents(); - content::RenderFrameHost* parent_frame_host = NULL; - web_contents->ForEachFrame(base::Bind(FindFrameHostForNavigationHandle, - navigation_handle, - &parent_frame_host)); + // Identify the RenderFrameHost that originated the navigation. + content::RenderFrameHost* parent_frame_host = + navigation_handle->GetParentFrame(); DCHECK(parent_frame_host); - - parent_frame_id = parent_frame_host->GetRoutingID(); + if (parent_frame_host) + parent_frame_id = parent_frame_host->GetRoutingID(); if (parent_frame_id < 0) parent_frame_id = CefFrameHostImpl::kUnspecifiedFrameId; } diff --git a/libcef/browser/extensions/extension_system.cc b/libcef/browser/extensions/extension_system.cc index f3325965f..28b36816d 100644 --- a/libcef/browser/extensions/extension_system.cc +++ b/libcef/browser/extensions/extension_system.cc @@ -225,7 +225,7 @@ void CefExtensionSystem::Init() { // CefExtensionWebContentsObserver::RenderViewCreated in the browser // process. if (PdfExtensionEnabled()) { - LoadExtension(pdf_extension_util::GetManifest(), + LoadExtension(ParseManifest(pdf_extension_util::GetManifest()), base::FilePath(FILE_PATH_LITERAL("pdf")), true /* internal */, nullptr, nullptr); } diff --git a/libcef/browser/frame_host_impl.cc b/libcef/browser/frame_host_impl.cc index 8786be8c8..00bf7af2c 100644 --- a/libcef/browser/frame_host_impl.cc +++ b/libcef/browser/frame_host_impl.cc @@ -212,10 +212,12 @@ void CefFrameHostImpl::SetFocused(bool focused) { is_focused_ = focused; } -void CefFrameHostImpl::SetAttributes(const CefString& url, +void CefFrameHostImpl::SetAttributes(bool is_main_frame, + const CefString& url, const CefString& name, int64 parent_frame_id) { base::AutoLock lock_scope(state_lock_); + is_main_frame_ = is_main_frame; if (!url.empty() && url != url_) url_ = url; if (!name.empty() && name != name_) diff --git a/libcef/browser/frame_host_impl.h b/libcef/browser/frame_host_impl.h index f0cb69b0f..9c188837d 100644 --- a/libcef/browser/frame_host_impl.h +++ b/libcef/browser/frame_host_impl.h @@ -57,7 +57,8 @@ class CefFrameHostImpl : public CefFrame { void VisitDOM(CefRefPtr visitor) override; void SetFocused(bool focused); - void SetAttributes(const CefString& url, + void SetAttributes(bool is_main_frame, + const CefString& url, const CefString& name, int64 parent_frame_id); diff --git a/libcef/browser/net/net_util.cc b/libcef/browser/net/net_util.cc new file mode 100644 index 000000000..ea503f0dd --- /dev/null +++ b/libcef/browser/net/net_util.cc @@ -0,0 +1,22 @@ +// Copyright (c) 2017 The Chromium Embedded Framework Authors. All rights +// reserved. Use of this source code is governed by a BSD-style license that +// can be found in the LICENSE file. + +#include "libcef/browser/net/net_util.h" + +#include "net/url_request/url_request.h" +#include "url/url_constants.h" + +namespace net_util { + +bool IsInternalRequest(net::URLRequest* request) { + // With PlzNavigate we now receive blob URLs. Ignore these URLs. + // See https://crbug.com/776884 for details. + if (request->url().SchemeIs(url::kBlobScheme)) { + return true; + } + + return false; +} + +}; // namespace net_util diff --git a/libcef/browser/net/net_util.h b/libcef/browser/net/net_util.h new file mode 100644 index 000000000..de9d7122e --- /dev/null +++ b/libcef/browser/net/net_util.h @@ -0,0 +1,21 @@ +// Copyright (c) 2017 The Chromium Embedded Framework Authors. All rights +// reserved. Use of this source code is governed by a BSD-style license that +// can be found in the LICENSE file. + +#ifndef CEF_LIBCEF_BROWSER_NET_NET_UTIL_H_ +#define CEF_LIBCEF_BROWSER_NET_NET_UTIL_H_ +#pragma once + +namespace net { +class URLRequest; +} + +namespace net_util { + +// Returns true if |request| is handled internally and should not be exposed via +// the CEF API. +bool IsInternalRequest(net::URLRequest* request); + +}; // namespace net_util + +#endif // CEF_LIBCEF_BROWSER_NET_NET_UTIL_H_ diff --git a/libcef/browser/net/network_delegate.cc b/libcef/browser/net/network_delegate.cc index 516fefeb6..68e8af58f 100644 --- a/libcef/browser/net/network_delegate.cc +++ b/libcef/browser/net/network_delegate.cc @@ -9,6 +9,7 @@ #include "include/cef_urlrequest.h" #include "libcef/browser/browser_host_impl.h" +#include "libcef/browser/net/net_util.h" #include "libcef/browser/net/source_stream.h" #include "libcef/browser/net/url_request_user_data.h" #include "libcef/browser/thread_util.h" @@ -219,14 +220,9 @@ class CefAuthCallbackImpl : public CefAuthCallback { IMPLEMENT_REFCOUNTING(CefAuthCallbackImpl); }; -} // namespace - -CefNetworkDelegate::CefNetworkDelegate() : force_google_safesearch_(nullptr) {} - -CefNetworkDelegate::~CefNetworkDelegate() {} - -// static -bool CefNetworkDelegate::AreExperimentalCookieFeaturesEnabled() { +// Match the logic from ChromeNetworkDelegate and +// RenderFrameMessageFilter::OnSetCookie. +bool AreExperimentalCookieFeaturesEnabled() { static bool initialized = false; static bool enabled = false; if (!initialized) { @@ -237,9 +233,18 @@ bool CefNetworkDelegate::AreExperimentalCookieFeaturesEnabled() { return enabled; } +} // namespace + +CefNetworkDelegate::CefNetworkDelegate() : force_google_safesearch_(nullptr) {} + +CefNetworkDelegate::~CefNetworkDelegate() {} + std::unique_ptr CefNetworkDelegate::CreateSourceStream( net::URLRequest* request, std::unique_ptr upstream) { + if (net_util::IsInternalRequest(request)) + return upstream; + CefRefPtr cef_filter; CefRefPtr browser = @@ -275,6 +280,9 @@ int CefNetworkDelegate::OnBeforeURLRequest( net::URLRequest* request, const net::CompletionCallback& callback, GURL* new_url) { + if (net_util::IsInternalRequest(request)) + return net::OK; + const bool force_google_safesearch = (force_google_safesearch_ && force_google_safesearch_->GetValue()); @@ -330,6 +338,9 @@ int CefNetworkDelegate::OnBeforeURLRequest( } void CefNetworkDelegate::OnCompleted(net::URLRequest* request, bool started) { + if (net_util::IsInternalRequest(request)) + return; + if (!started) return; @@ -381,6 +392,9 @@ net::NetworkDelegate::AuthRequiredResponse CefNetworkDelegate::OnAuthRequired( const net::AuthChallengeInfo& auth_info, const AuthCallback& callback, net::AuthCredentials* credentials) { + if (net_util::IsInternalRequest(request)) + return AUTH_REQUIRED_RESPONSE_NO_ACTION; + CefRefPtr browser = CefBrowserHostImpl::GetBrowserForRequest(request); if (browser.get()) { @@ -434,5 +448,5 @@ bool CefNetworkDelegate::OnCanAccessFile( } bool CefNetworkDelegate::OnAreExperimentalCookieFeaturesEnabled() const { - return AreExperimentalCookieFeaturesEnabled(); + return ::AreExperimentalCookieFeaturesEnabled(); } diff --git a/libcef/browser/net/network_delegate.h b/libcef/browser/net/network_delegate.h index 346b51ee7..5f5a86904 100644 --- a/libcef/browser/net/network_delegate.h +++ b/libcef/browser/net/network_delegate.h @@ -20,10 +20,6 @@ class CefNetworkDelegate : public net::NetworkDelegateImpl { CefNetworkDelegate(); ~CefNetworkDelegate() override; - // Match the logic from ChromeNetworkDelegate and - // RenderFrameMessageFilter::OnSetCookie. - static bool AreExperimentalCookieFeaturesEnabled(); - void set_force_google_safesearch(BooleanPrefMember* force_google_safesearch) { force_google_safesearch_ = force_google_safesearch; } diff --git a/libcef/browser/net/url_request_interceptor.cc b/libcef/browser/net/url_request_interceptor.cc index 034e33314..867ec7c25 100644 --- a/libcef/browser/net/url_request_interceptor.cc +++ b/libcef/browser/net/url_request_interceptor.cc @@ -7,6 +7,7 @@ #include #include "libcef/browser/browser_host_impl.h" +#include "libcef/browser/net/net_util.h" #include "libcef/browser/net/resource_request_job.h" #include "libcef/browser/thread_util.h" #include "libcef/common/net/http_header_utils.h" @@ -29,11 +30,8 @@ CefRequestInterceptor::~CefRequestInterceptor() { net::URLRequestJob* CefRequestInterceptor::MaybeInterceptRequest( net::URLRequest* request, net::NetworkDelegate* network_delegate) const { - // With PlzNavigate we now receive blob URLs here. - // Ignore these URLs. See https://crbug.com/776884 for details. - if (request->url().SchemeIs(url::kBlobScheme)) { + if (net_util::IsInternalRequest(request)) return nullptr; - } CefRefPtr browser = CefBrowserHostImpl::GetBrowserForRequest(request); @@ -59,13 +57,16 @@ net::URLRequestJob* CefRequestInterceptor::MaybeInterceptRequest( } } - return NULL; + return nullptr; } net::URLRequestJob* CefRequestInterceptor::MaybeInterceptRedirect( net::URLRequest* request, net::NetworkDelegate* network_delegate, const GURL& location) const { + if (net_util::IsInternalRequest(request)) + return nullptr; + CefRefPtr browser = CefBrowserHostImpl::GetBrowserForRequest(request); if (browser.get()) { @@ -100,24 +101,27 @@ net::URLRequestJob* CefRequestInterceptor::MaybeInterceptRedirect( } } - return NULL; + return nullptr; } net::URLRequestJob* CefRequestInterceptor::MaybeInterceptResponse( net::URLRequest* request, net::NetworkDelegate* network_delegate) const { + if (net_util::IsInternalRequest(request)) + return nullptr; + CefRefPtr browser = CefBrowserHostImpl::GetBrowserForRequest(request); if (!browser.get()) - return NULL; + return nullptr; CefRefPtr client = browser->GetClient(); if (!client.get()) - return NULL; + return nullptr; CefRefPtr handler = client->GetRequestHandler(); if (!handler.get()) - return NULL; + return nullptr; CefRefPtr frame = browser->GetFrameForRequest(request); @@ -132,7 +136,7 @@ net::URLRequestJob* CefRequestInterceptor::MaybeInterceptResponse( // Give the client an opportunity to retry or redirect the request. if (!handler->OnResourceResponse(browser.get(), frame, cefRequest.get(), cefResponse.get())) { - return NULL; + return nullptr; } // This flag will be reset by URLRequest::RestartWithJob() calling diff --git a/libcef/browser/plugins/plugin_service_filter.cc b/libcef/browser/plugins/plugin_service_filter.cc index 4aa182387..13cbe4aa6 100644 --- a/libcef/browser/plugins/plugin_service_filter.cc +++ b/libcef/browser/plugins/plugin_service_filter.cc @@ -24,6 +24,10 @@ bool CefPluginServiceFilter::IsPluginAvailable( bool is_main_frame, const url::Origin& main_frame_origin, content::WebPluginInfo* plugin) { + // With PlzNavigate this can be called before the renderer process exists. + if (render_process_id < 0) + return true; + CefResourceContext* resource_context = const_cast( reinterpret_cast(context)); CefViewHostMsg_GetPluginInfo_Status status = diff --git a/libcef_dll/wrapper_types.h b/libcef_dll/wrapper_types.h index 2b7daf50a..62f345851 100644 --- a/libcef_dll/wrapper_types.h +++ b/libcef_dll/wrapper_types.h @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=9a3b6de92214d6b132bd66d84724272c886a3758$ +// $hash=778fe7c495b5646ffbed281f033ef1bd51f82a4f$ // #ifndef CEF_LIBCEF_DLL_WRAPPER_TYPES_H_ diff --git a/tests/ceftests/frame_unittest.cc b/tests/ceftests/frame_unittest.cc index 13ed8bb10..f21d063f0 100644 --- a/tests/ceftests/frame_unittest.cc +++ b/tests/ceftests/frame_unittest.cc @@ -736,9 +736,14 @@ class FrameNavExpectationsRendererSingleNav V_EXPECT_TRUE(got_load_end_); V_EXPECT_TRUE(got_loading_state_change_start_); V_EXPECT_TRUE(got_loading_state_change_end_); - V_EXPECT_TRUE(got_before_navigation_); V_EXPECT_FALSE(got_finalize_); + if (IsBrowserSideNavigationEnabled()) { + V_EXPECT_FALSE(got_before_navigation_); + } else { + V_EXPECT_TRUE(got_before_navigation_); + } + got_finalize_.yes(); V_RETURN(); @@ -1018,8 +1023,10 @@ class FrameNavExpectationsBrowserTestSingleNav CefRefPtr frame, const std::string& url) override { V_DECLARE(); - V_EXPECT_TRUE( - VerifySingleBrowserFrames(browser, frame, true, std::string())); + // When browser-side navigation is enabled this method will be called + // before the frame is created. + V_EXPECT_TRUE(VerifySingleBrowserFrames( + browser, frame, !IsBrowserSideNavigationEnabled(), std::string())); V_EXPECT_TRUE(parent::OnBeforeBrowse(browser, frame, url)); V_RETURN(); } @@ -1027,8 +1034,10 @@ class FrameNavExpectationsBrowserTestSingleNav bool GetResourceHandler(CefRefPtr browser, CefRefPtr frame) override { V_DECLARE(); - V_EXPECT_TRUE( - VerifySingleBrowserFrames(browser, frame, true, std::string())); + // When browser-side navigation is enabled this method will be called + // before the frame is created. + V_EXPECT_TRUE(VerifySingleBrowserFrames( + browser, frame, !IsBrowserSideNavigationEnabled(), std::string())); V_EXPECT_TRUE(parent::GetResourceHandler(browser, frame)); V_RETURN(); } @@ -1527,8 +1536,11 @@ class FrameNavExpectationsBrowserTestMultiNav std::string expected_url; if (nav() > 0) expected_url = GetPreviousMainURL(); - V_EXPECT_TRUE( - VerifySingleBrowserFrames(browser, frame, true, expected_url)); + // When browser-side navigation is enabled this method will be called + // before the frame is created for the first navigation. + V_EXPECT_TRUE(VerifySingleBrowserFrames( + browser, frame, nav() == 0 ? !IsBrowserSideNavigationEnabled() : true, + expected_url)); V_EXPECT_TRUE(parent::OnBeforeBrowse(browser, frame, url)); V_RETURN(); } @@ -1539,8 +1551,11 @@ class FrameNavExpectationsBrowserTestMultiNav std::string expected_url; if (nav() > 0) expected_url = GetPreviousMainURL(); - V_EXPECT_TRUE( - VerifySingleBrowserFrames(browser, frame, true, expected_url)); + // When browser-side navigation is enabled this method will be called + // before the frame is created for the first navigation. + V_EXPECT_TRUE(VerifySingleBrowserFrames( + browser, frame, nav() == 0 ? !IsBrowserSideNavigationEnabled() : true, + expected_url)); V_EXPECT_TRUE(parent::GetResourceHandler(browser, frame)); V_RETURN(); } @@ -1723,33 +1738,48 @@ bool VerifyBrowserIframe(CefRefPtr browser, V_EXPECT_TRUE(frame2.get()); // Verify that the name matches. - V_EXPECT_TRUE(frame0->GetName().ToString() == kFrame0Name); - V_EXPECT_TRUE(frame1->GetName().ToString() == kFrame1Name); - V_EXPECT_TRUE(frame2->GetName().ToString() == kFrame2Name); + V_EXPECT_TRUE(frame0->GetName().ToString() == kFrame0Name) + << "expected: " << kFrame0Name + << " actual: " << frame0->GetName().ToString(); + V_EXPECT_TRUE(frame1->GetName().ToString() == kFrame1Name) + << "expected: " << kFrame1Name + << " actual: " << frame1->GetName().ToString(); + V_EXPECT_TRUE(frame2->GetName().ToString() == kFrame2Name) + << "expected: " << kFrame2Name + << " actual: " << frame2->GetName().ToString(); // Verify that the URL matches. frame0url = GetMultiNavURL(origin, 0); - V_EXPECT_TRUE(frame0->GetURL() == frame0url); + V_EXPECT_TRUE(frame0->GetURL() == frame0url) + << "expected: " << frame0url + << " actual: " << frame0->GetURL().ToString(); frame1url = GetMultiNavURL(origin, 1); - V_EXPECT_TRUE(frame1->GetURL() == frame1url); + V_EXPECT_TRUE(frame1->GetURL() == frame1url) + << "expected: " << frame1url + << " actual: " << frame1->GetURL().ToString(); frame2url = GetMultiNavURL(origin, 2); - V_EXPECT_TRUE(frame2->GetURL() == frame2url); + V_EXPECT_TRUE(frame2->GetURL() == frame2url) + << "expected: " << frame2url + << " actual: " << frame2->GetURL().ToString(); // Verify that the frame id is valid. frame0id = frame0->GetIdentifier(); - V_EXPECT_TRUE(frame0id > 0); + V_EXPECT_TRUE(frame0id > 0) << "actual: " << frame0id; frame1id = frame1->GetIdentifier(); - V_EXPECT_TRUE(frame1id > 0); + V_EXPECT_TRUE(frame1id > 0) << "actual: " << frame1id; frame2id = frame2->GetIdentifier(); - V_EXPECT_TRUE(frame2id > 0); + V_EXPECT_TRUE(frame2id > 0) << "actual: " << frame2id; // Verify that the current frame has the correct id. if (frame_number == 0) { - V_EXPECT_TRUE(frame->GetIdentifier() == frame0id); + V_EXPECT_TRUE(frame->GetIdentifier() == frame0id) + << "expected: " << frame0id << " actual: " << frame->GetIdentifier(); } else if (frame_number == 1) { - V_EXPECT_TRUE(frame->GetIdentifier() == frame1id); + V_EXPECT_TRUE(frame->GetIdentifier() == frame1id) + << "expected: " << frame1id << " actual: " << frame->GetIdentifier(); } else if (frame_number == 2) { - V_EXPECT_TRUE(frame->GetIdentifier() == frame2id); + V_EXPECT_TRUE(frame->GetIdentifier() == frame2id) + << "expected: " << frame2id << " actual: " << frame->GetIdentifier(); } // Find frames by id. @@ -1761,33 +1791,46 @@ bool VerifyBrowserIframe(CefRefPtr browser, V_EXPECT_TRUE(frame2b.get()); // Verify that the id matches. - V_EXPECT_TRUE(frame0b->GetIdentifier() == frame0id); - V_EXPECT_TRUE(frame1b->GetIdentifier() == frame1id); - V_EXPECT_TRUE(frame2b->GetIdentifier() == frame2id); + V_EXPECT_TRUE(frame0b->GetIdentifier() == frame0id) + << "expected: " << frame0id << " actual: " << frame0b->GetIdentifier(); + V_EXPECT_TRUE(frame1b->GetIdentifier() == frame1id) + << "expected: " << frame1id << " actual: " << frame1b->GetIdentifier(); + V_EXPECT_TRUE(frame2b->GetIdentifier() == frame2id) + << "expected: " << frame2id << " actual: " << frame2b->GetIdentifier(); size_t frame_count = browser->GetFrameCount(); - V_EXPECT_TRUE(frame_count == 3U) << "actual " << frame_count; + V_EXPECT_TRUE(frame_count == 3U) << "actual: " << frame_count; // Verify the GetFrameNames result. std::vector names; browser->GetFrameNames(names); - V_EXPECT_TRUE(names.size() == 3U); - V_EXPECT_TRUE(names[0].ToString() == kFrame0Name); - V_EXPECT_TRUE(names[1].ToString() == kFrame1Name); - V_EXPECT_TRUE(names[2].ToString() == kFrame2Name); + V_EXPECT_TRUE(names.size() == 3U) << "actual: " << names.size(); + V_EXPECT_TRUE(names[0].ToString() == kFrame0Name) + << "expected: " << kFrame0Name << " actual: " << names[0].ToString(); + V_EXPECT_TRUE(names[1].ToString() == kFrame1Name) + << "expected: " << kFrame1Name << " actual: " << names[1].ToString(); + V_EXPECT_TRUE(names[2].ToString() == kFrame2Name) + << "expected: " << kFrame2Name << " actual: " << names[2].ToString(); // Verify the GetFrameIdentifiers result. std::vector idents; browser->GetFrameIdentifiers(idents); - V_EXPECT_TRUE(idents.size() == 3U); - V_EXPECT_TRUE(idents[0] == frame0->GetIdentifier()); - V_EXPECT_TRUE(idents[1] == frame1->GetIdentifier()); - V_EXPECT_TRUE(idents[2] == frame2->GetIdentifier()); + V_EXPECT_TRUE(idents.size() == 3U) << "actual: " << idents.size(); + V_EXPECT_TRUE(idents[0] == frame0id) + << "expected: " << frame0id << " actual: " << idents[0]; + V_EXPECT_TRUE(idents[1] == frame1id) + << "expected: " << frame1id << " actual: " << idents[1]; + V_EXPECT_TRUE(idents[2] == frame2id) + << "expected: " << frame2id << " actual: " << idents[2]; // Verify parent hierarchy. V_EXPECT_FALSE(frame0->GetParent().get()); - V_EXPECT_TRUE(frame1->GetParent()->GetIdentifier() == frame0id); - V_EXPECT_TRUE(frame2->GetParent()->GetIdentifier() == frame1id); + V_EXPECT_TRUE(frame1->GetParent()->GetIdentifier() == frame0id) + << "expected: " << frame0id + << " actual: " << frame1->GetParent()->GetIdentifier(); + V_EXPECT_TRUE(frame2->GetParent()->GetIdentifier() == frame1id) + << "expected: " << frame1id + << " actual: " << frame2->GetParent()->GetIdentifier(); V_RETURN(); } diff --git a/tests/ceftests/navigation_unittest.cc b/tests/ceftests/navigation_unittest.cc index 74189a5e8..399d67451 100644 --- a/tests/ceftests/navigation_unittest.cc +++ b/tests/ceftests/navigation_unittest.cc @@ -940,8 +940,7 @@ class RedirectTestHandler : public TestHandler { got_nav3_redirect_.yes(); EXPECT_EQ(303, response->GetStatus()); - EXPECT_STREQ("See Other", - response->GetStatusText().ToString().c_str()); + EXPECT_STREQ("See Other", response->GetStatusText().ToString().c_str()); EXPECT_STREQ("text/html", response->GetMimeType().ToString().c_str()); } else { got_invalid_redirect_.yes(); @@ -1169,21 +1168,20 @@ class OrderNavLoadState { got_load_end_.yes(); } - bool IsStarted() { + bool IsStarted() const { return got_loading_state_start_ || got_loading_state_end_ || got_load_start_ || got_load_end_; } - bool IsDone() { + bool IsDone() const { return got_loading_state_start_ && got_loading_state_end_ && got_load_start_ && got_load_end_; } - private: bool Verify(bool got_loading_state_start, bool got_loading_state_end, bool got_load_start, - bool got_load_end) { + bool got_load_end) const { EXPECT_EQ(got_loading_state_start, got_loading_state_start_) << "Popup: " << is_popup_ << "; Browser Side: " << browser_side_; EXPECT_EQ(got_loading_state_end, got_loading_state_end_) @@ -1198,6 +1196,7 @@ class OrderNavLoadState { got_load_start == got_load_start_ && got_load_end == got_load_end_; } + private: bool is_popup_; bool browser_side_; @@ -1384,6 +1383,15 @@ class OrderNavRendererTest : public ClientAppRenderer::Delegate, SendTestResultsIfDone(browser); } + void OnLoadError(CefRefPtr browser, + CefRefPtr frame, + ErrorCode errorCode, + const CefString& errorText, + const CefString& failedUrl) override { + ADD_FAILURE() << "renderer OnLoadError url: " << failedUrl.ToString() + << " error: " << errorCode; + } + protected: void SendTestResultsIfDone(CefRefPtr browser) { bool done = false; @@ -1580,6 +1588,15 @@ class OrderNavTestHandler : public TestHandler { ContinueIfReady(browser); } + void OnLoadError(CefRefPtr browser, + CefRefPtr frame, + ErrorCode errorCode, + const CefString& errorText, + const CefString& failedUrl) override { + ADD_FAILURE() << "browser OnLoadError url: " << failedUrl.ToString() + << " error: " << errorCode; + } + bool OnProcessMessageReceived(CefRefPtr browser, CefProcessId source_process, CefRefPtr message) override { @@ -1627,6 +1644,9 @@ class OrderNavTestHandler : public TestHandler { EXPECT_TRUE(got_before_browse_main_); EXPECT_TRUE(got_before_browse_popup_); + EXPECT_TRUE(state_main_.Verify(true, true, true, true)); + EXPECT_TRUE(state_popup_.Verify(true, true, true, true)); + TestHandler::DestroyTest(); } @@ -2405,8 +2425,15 @@ class PopupJSWindowOpenTestHandler : public TestHandler { void OnAfterCreated(CefRefPtr browser) override { TestHandler::OnAfterCreated(browser); - if (browser->IsPopup()) + if (browser->IsPopup()) { after_created_ct_++; + if (!popup1_) + popup1_ = browser; + else if (!popup2_) + popup2_ = browser; + else + ADD_FAILURE(); + } } void OnLoadingStateChange(CefRefPtr browser, @@ -2418,10 +2445,23 @@ class PopupJSWindowOpenTestHandler : public TestHandler { if (browser->IsPopup()) { const std::string& url = browser->GetMainFrame()->GetURL(); - if (load_end_ct_ == 0) + if (url == kPopupJSOpenPopupUrl) { + EXPECT_TRUE(browser->IsSame(popup2_)); + popup2_ = nullptr; + + if (IsBrowserSideNavigationEnabled()) { + // OnLoadingStateChange is not currently called for browser-side + // navigations of empty popups. See https://crbug.com/789252. + // Explicitly close the empty popup here as a workaround. + CloseBrowser(popup1_, true); + popup1_ = nullptr; + } + } else { + // Empty popup. EXPECT_TRUE(url.empty()); - else - EXPECT_STREQ(kPopupJSOpenPopupUrl, url.c_str()); + EXPECT_TRUE(browser->IsSame(popup1_)); + popup1_ = nullptr; + } load_end_ct_++; CloseBrowser(browser, true); @@ -2436,6 +2476,15 @@ class PopupJSWindowOpenTestHandler : public TestHandler { } } + void OnLoadError(CefRefPtr browser, + CefRefPtr frame, + ErrorCode errorCode, + const CefString& errorText, + const CefString& failedUrl) override { + ADD_FAILURE() << "OnLoadError url: " << failedUrl.ToString() + << " error: " << errorCode; + } + void OnBeforeClose(CefRefPtr browser) override { TestHandler::OnBeforeClose(browser); @@ -2448,12 +2497,22 @@ class PopupJSWindowOpenTestHandler : public TestHandler { void DestroyTest() override { EXPECT_EQ(2U, before_popup_ct_); EXPECT_EQ(2U, after_created_ct_); - EXPECT_EQ(2U, load_end_ct_); EXPECT_EQ(2U, before_close_ct_); + if (IsBrowserSideNavigationEnabled()) { + // OnLoadingStateChange is not currently called for browser-side + // navigations of empty popups. See https://crbug.com/789252. + EXPECT_EQ(1U, load_end_ct_); + } else { + EXPECT_EQ(2U, load_end_ct_); + } + TestHandler::DestroyTest(); } + CefRefPtr popup1_; + CefRefPtr popup2_; + size_t before_popup_ct_; size_t after_created_ct_; size_t load_end_ct_; @@ -3131,8 +3190,10 @@ class CancelAfterNavTestHandler : public TestHandler { got_get_resource_handler_.yes(); - CefPostDelayedTask( - TID_UI, base::Bind(&CancelAfterNavTestHandler::CancelLoad, this), 100); + // The required delay is longer when browser-side navigation is enabled. + CefPostDelayedTask(TID_UI, + base::Bind(&CancelAfterNavTestHandler::CancelLoad, this), + IsBrowserSideNavigationEnabled() ? 500 : 100); return new StalledSchemeHandler(); } diff --git a/tests/ceftests/plugin_unittest.cc b/tests/ceftests/plugin_unittest.cc index 432f2e6cc..130e3387c 100644 --- a/tests/ceftests/plugin_unittest.cc +++ b/tests/ceftests/plugin_unittest.cc @@ -361,7 +361,6 @@ class PluginTestHandler : public RoutingTestHandler, return new CefStreamResourceHandler("application/pdf", stream); } - NOTREACHED(); return NULL; } diff --git a/tests/ceftests/request_context_unittest.cc b/tests/ceftests/request_context_unittest.cc index b07712dac..9b177fa72 100644 --- a/tests/ceftests/request_context_unittest.cc +++ b/tests/ceftests/request_context_unittest.cc @@ -855,7 +855,13 @@ class PopupNavTestHandler : public TestHandler { EXPECT_FALSE(got_popup_load_end2_); } else if (mode_ == NAVIGATE_AFTER_CREATION) { EXPECT_FALSE(got_popup_load_start_); - EXPECT_TRUE(got_popup_load_error_); + if (IsBrowserSideNavigationEnabled()) { + // With browser-side navigation we will never actually begin the + // navigation to the 1st popup URL, so there will be no load error. + EXPECT_FALSE(got_popup_load_error_); + } else { + EXPECT_TRUE(got_popup_load_error_); + } EXPECT_FALSE(got_popup_load_end_); EXPECT_TRUE(got_popup_load_start2_); EXPECT_FALSE(got_popup_load_error2_); diff --git a/tests/ceftests/request_unittest.cc b/tests/ceftests/request_unittest.cc index 716b6f85b..1f1cd6afe 100644 --- a/tests/ceftests/request_unittest.cc +++ b/tests/ceftests/request_unittest.cc @@ -591,8 +591,10 @@ class TypeTestHandler : public TestHandler { if (destroyed_) return; - if (completed_browser_side_ && completed_render_side_) + if (completed_browser_side_ && + (completed_render_side_ || IsBrowserSideNavigationEnabled())) { DestroyTest(); + } } void DestroyTest() override { @@ -602,7 +604,11 @@ class TypeTestHandler : public TestHandler { // Verify test expectations. EXPECT_TRUE(completed_browser_side_); - EXPECT_TRUE(completed_render_side_); + if (IsBrowserSideNavigationEnabled()) { + EXPECT_FALSE(completed_render_side_); + } else { + EXPECT_TRUE(completed_render_side_); + } EXPECT_TRUE(browse_expectations_.IsDone(true)); EXPECT_TRUE(load_expectations_.IsDone(true)); EXPECT_TRUE(get_expectations_.IsDone(true)); diff --git a/tests/ceftests/scheme_handler_unittest.cc b/tests/ceftests/scheme_handler_unittest.cc index 49841f10c..37f70bc6a 100644 --- a/tests/ceftests/scheme_handler_unittest.cc +++ b/tests/ceftests/scheme_handler_unittest.cc @@ -158,17 +158,11 @@ class TestSchemeHandler : public TestHandler { const CefString& errorText, const CefString& failedUrl) override { test_results_->got_error.yes(); -#if defined(OS_LINUX) - // CustomStandardXHR* tests are flaky on Linux, sometimes returning - // ERR_ABORTED. Make the tests less flaky by also accepting that value. + // Tests sometimes also fail with ERR_ABORTED. if (!(test_results_->expected_error_code == 0 && errorCode == ERR_ABORTED)) { EXPECT_EQ(test_results_->expected_error_code, errorCode); } -#else - // Check that the error code matches the expectation. - EXPECT_EQ(test_results_->expected_error_code, errorCode); -#endif DestroyTest(); } diff --git a/tests/ceftests/test_handler.cc b/tests/ceftests/test_handler.cc index c6603f952..55ce6f178 100644 --- a/tests/ceftests/test_handler.cc +++ b/tests/ceftests/test_handler.cc @@ -490,3 +490,19 @@ bool TestFailed() { return ::testing::UnitTest::GetInstance()->Failed(); } } + +bool IsBrowserSideNavigationEnabled() { + static bool initialized = false; + static bool value = false; // Default value. + if (!initialized) { + CefRefPtr command_line = + CefCommandLine::GetGlobalCommandLine(); + if (command_line->HasSwitch("enable-browser-side-navigation")) { + value = true; + } else if (command_line->HasSwitch("disable-browser-side-navigation")) { + value = false; + } + initialized = true; + } + return value; +} diff --git a/tests/ceftests/test_handler.h b/tests/ceftests/test_handler.h index 6aeaf11ad..9cb21e518 100644 --- a/tests/ceftests/test_handler.h +++ b/tests/ceftests/test_handler.h @@ -327,6 +327,9 @@ void ReleaseAndWaitForDestructor(CefRefPtr& handler, int delay_ms = 2000) { // Returns true if the currently running test has failed. bool TestFailed(); +// Returns true if browser-side navigation is enabled. +bool IsBrowserSideNavigationEnabled(); + // Helper macros for executing checks in a method with a boolean return value. // For example: //