From 54dd34e19b56a2b7f0d6cb8d473ce36288dcf8e2 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Tue, 5 Apr 2022 13:22:32 -0400 Subject: [PATCH] chrome: Update expectations with same-site BFCache enabled (fixes issue #3301) With same-site BFCache enabled every navigation can now potentially be served via the BFCache. To support this internally a new top-level RenderFrame object may be created for each new navigation. As a result, OnBrowserCreated may now be called multiple times with the same browser ID in a given renderer process (a behavior previously only seen with cross-site navigations and different renderer processes). BFCache navigations do not trigger the same Chromium notifications as a normal load. To avoid breaking CEF API usage expectations we now synthetically generate the load-related callbacks that would otherwise be missing (OnLoadingStateChange with isLoading=true, OnLoadStart, OnLoadEnd). The |httpStatusCode| argument to OnLoadEnd will be 0 in this case. To test: - Run `FrameHandlerTest.*:MessageRouterTest.*:NavigationTest.*` - Run `NavigationTest.LoadSameOriginLoadURL` for OnBrowserCreated behavior. - Run `NavigationTest.History` for load-related callback behavior. --- libcef/browser/browser_contents_delegate.cc | 4 + libcef/browser/browser_info.cc | 5 +- libcef/renderer/blink_glue.cc | 39 +++ libcef/renderer/blink_glue.h | 27 +++ libcef/renderer/browser_impl.cc | 20 ++ libcef/renderer/browser_impl.h | 4 + libcef/renderer/frame_impl.cc | 18 +- libcef/renderer/frame_impl.h | 16 +- libcef/renderer/render_frame_observer.cc | 41 ++-- tests/ceftests/frame_handler_unittest.cc | 25 +- tests/ceftests/message_router_unittest.cc | 179 +++++++++----- tests/ceftests/navigation_unittest.cc | 256 +++++++++++++------- tests/ceftests/test_util.cc | 22 ++ tests/ceftests/test_util.h | 6 + 14 files changed, 479 insertions(+), 183 deletions(-) diff --git a/libcef/browser/browser_contents_delegate.cc b/libcef/browser/browser_contents_delegate.cc index 47780c093..aa362c672 100644 --- a/libcef/browser/browser_contents_delegate.cc +++ b/libcef/browser/browser_contents_delegate.cc @@ -421,6 +421,10 @@ void CefBrowserContentsDelegate::DidFinishNavigation( // history state). if (!navigation_handle->IsSameDocument()) { OnLoadStart(frame.get(), navigation_handle->GetPageTransition()); + if (navigation_handle->IsServedFromBackForwardCache()) { + // We won't get an OnLoadEnd notification from anywhere else. + OnLoadEnd(frame.get(), navigation_handle->GetURL(), 0); + } } if (is_main_frame) { diff --git a/libcef/browser/browser_info.cc b/libcef/browser/browser_info.cc index e80573afb..b0ba616f1 100644 --- a/libcef/browser/browser_info.cc +++ b/libcef/browser/browser_info.cc @@ -168,13 +168,10 @@ void CefBrowserInfo::FrameHostStateChanged( content::RenderFrameHost::LifecycleState::kInBackForwardCache) && new_state == content::RenderFrameHost::LifecycleState::kActive) { if (auto frame = GetFrameForHost(host)) { - // Should only occur for the main frame. - CHECK(frame->IsMain()); - // Update the associated RFH, which may have changed. frame->MaybeReAttach(this, host); - { + if (frame->IsMain()) { // Update the main frame object. NotificationStateLock lock_scope(this); SetMainFrame(browser_, frame); diff --git a/libcef/renderer/blink_glue.cc b/libcef/renderer/blink_glue.cc index ad57018d2..a0cbcc2bf 100644 --- a/libcef/renderer/blink_glue.cc +++ b/libcef/renderer/blink_glue.cc @@ -22,6 +22,8 @@ #include "third_party/blink/renderer/core/dom/element.h" #include "third_party/blink/renderer/core/dom/node.h" #include "third_party/blink/renderer/core/editing/serializers/serialization.h" +#include "third_party/blink/renderer/core/execution_context/execution_context.h" +#include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_state_observer.h" #include "third_party/blink/renderer/core/exported/web_view_impl.h" #include "third_party/blink/renderer/core/frame/frame_owner.h" #include "third_party/blink/renderer/core/frame/local_dom_window.h" @@ -220,6 +222,43 @@ bool IsScriptForbidden() { return blink::ScriptForbiddenScope::IsScriptForbidden(); } +std::unique_ptr +RegisterExecutionContextLifecycleStateObserver( + v8::Local context, + CefExecutionContextLifecycleStateObserver* observer) { + class Observer : public blink::GarbageCollected, + public blink::ExecutionContextLifecycleStateObserver { + public: + Observer(blink::ExecutionContext* execution_context, + CefExecutionContextLifecycleStateObserver* observer) + : blink::ExecutionContextLifecycleStateObserver(execution_context), + observer_(observer) { + UpdateStateIfNeeded(); + } + + void ContextLifecycleStateChanged( + blink::mojom::blink::FrameLifecycleState state) override { + observer_->ContextLifecycleStateChanged(state); + } + + void ContextDestroyed() override {} + + private: + CefExecutionContextLifecycleStateObserver* observer_; + }; + + class Registration : public CefObserverRegistration { + public: + Registration(blink::Persistent observer) : observer_(observer) {} + + private: + blink::Persistent observer_; + }; + + return std::make_unique(blink::MakeGarbageCollected( + blink::ExecutionContext::From(context), observer)); +} + void RegisterURLSchemeAsSupportingFetchAPI(const blink::WebString& scheme) { blink::SchemeRegistry::RegisterURLSchemeAsSupportingFetchAPI(scheme); } diff --git a/libcef/renderer/blink_glue.h b/libcef/renderer/blink_glue.h index 0d0494610..31716c225 100644 --- a/libcef/renderer/blink_glue.h +++ b/libcef/renderer/blink_glue.h @@ -13,6 +13,7 @@ #include "include/internal/cef_types.h" +#include "third_party/blink/public/mojom/frame/lifecycle.mojom-blink-forward.h" #include "third_party/blink/public/platform/web_common.h" #include "v8/include/v8.h" @@ -72,6 +73,32 @@ BLINK_EXPORT v8::Local ExecuteV8ScriptAndReturnValue( BLINK_EXPORT bool IsScriptForbidden(); +class BLINK_EXPORT CefObserverRegistration { + public: + CefObserverRegistration() = default; + + CefObserverRegistration(const CefObserverRegistration&) = delete; + CefObserverRegistration& operator=(const CefObserverRegistration&) = delete; + + virtual ~CefObserverRegistration() = default; +}; + +class BLINK_EXPORT CefExecutionContextLifecycleStateObserver { + public: + virtual void ContextLifecycleStateChanged( + blink::mojom::blink::FrameLifecycleState state) {} + + protected: + virtual ~CefExecutionContextLifecycleStateObserver() = default; +}; + +// Register an ExecutionContextLifecycleStateObserver. Remains registered until +// the returned object is destroyed. +BLINK_EXPORT std::unique_ptr +RegisterExecutionContextLifecycleStateObserver( + v8::Local context, + CefExecutionContextLifecycleStateObserver* observer); + BLINK_EXPORT void RegisterURLSchemeAsSupportingFetchAPI( const blink::WebString& scheme); diff --git a/libcef/renderer/browser_impl.cc b/libcef/renderer/browser_impl.cc index 75b18a4a5..a1bf27149 100644 --- a/libcef/renderer/browser_impl.cc +++ b/libcef/renderer/browser_impl.cc @@ -393,6 +393,19 @@ void CefBrowserImpl::OnLoadingStateChange(bool isLoading) { return; } + if (was_in_bfcache_) { + // Send the expected callbacks when exiting the BFCache. + DCHECK(!isLoading); + load_handler->OnLoadingStateChange(this, /*isLoading=*/true, + canGoBack, canGoForward); + + auto main_frame = GetMainFrame(); + load_handler->OnLoadStart(this, main_frame, TT_EXPLICIT); + load_handler->OnLoadEnd(this, main_frame, 0); + + was_in_bfcache_ = false; + } + load_handler->OnLoadingStateChange(this, isLoading, canGoBack, canGoForward); last_loading_state_.reset( @@ -401,3 +414,10 @@ void CefBrowserImpl::OnLoadingStateChange(bool isLoading) { } } } + +void CefBrowserImpl::OnEnterBFCache() { + // Reset loading state so that notifications will be resent if/when exiting + // BFCache. + was_in_bfcache_ = true; + last_loading_state_.reset(); +} diff --git a/libcef/renderer/browser_impl.h b/libcef/renderer/browser_impl.h index e8893bb3b..0283f7348 100644 --- a/libcef/renderer/browser_impl.h +++ b/libcef/renderer/browser_impl.h @@ -92,6 +92,7 @@ class CefBrowserImpl : public CefBrowser, public blink::WebViewObserver { void FrameDetached(int64_t frame_id); void OnLoadingStateChange(bool isLoading); + void OnEnterBFCache(); private: // ID of the browser that this RenderView is associated with. During loading @@ -105,6 +106,9 @@ class CefBrowserImpl : public CefBrowser, public blink::WebViewObserver { using FrameMap = std::map>; FrameMap frames_; + // True if the browser was in the BFCache. + bool was_in_bfcache_ = false; + // Map of unique frame ids to CefTrackManager objects that need to be cleaned // up when the frame is deleted. using FrameObjectMap = std::map>; diff --git a/libcef/renderer/frame_impl.cc b/libcef/renderer/frame_impl.cc index 1bc70e69d..af55060ad 100644 --- a/libcef/renderer/frame_impl.cc +++ b/libcef/renderer/frame_impl.cc @@ -35,6 +35,7 @@ #include "content/public/renderer/render_view.h" #include "content/renderer/render_frame_impl.h" #include "third_party/blink/public/mojom/frame/frame.mojom-blink.h" +#include "third_party/blink/public/mojom/frame/lifecycle.mojom-blink.h" #include "third_party/blink/public/platform/web_back_forward_cache_loader_helper.h" #include "third_party/blink/public/platform/web_data.h" #include "third_party/blink/public/platform/web_string.h" @@ -390,7 +391,7 @@ void CefFrameImpl::OnDraggableRegionsChanged() { std::move(regions_arg))); } -void CefFrameImpl::OnContextCreated() { +void CefFrameImpl::OnContextCreated(v8::Local context) { context_created_ = true; CHECK(frame_); @@ -399,6 +400,13 @@ void CefFrameImpl::OnContextCreated() { std::move(action.second).Run(frame_); queued_context_actions_.pop(); } + + execution_context_lifecycle_state_observer_ = + blink_glue::RegisterExecutionContextLifecycleStateObserver(context, this); +} + +void CefFrameImpl::OnContextReleased() { + execution_context_lifecycle_state_observer_.reset(); } void CefFrameImpl::OnDetached() { @@ -684,6 +692,14 @@ void CefFrameImpl::MoveOrResizeStarted() { } } +void CefFrameImpl::ContextLifecycleStateChanged( + blink::mojom::blink::FrameLifecycleState state) { + if (state == blink::mojom::FrameLifecycleState::kFrozen && IsMain() && + blink_glue::IsInBackForwardCache(frame_)) { + browser_->OnEnterBFCache(); + } +} + // Enable deprecation warnings on Windows. See http://crbug.com/585142. #if BUILDFLAG(IS_WIN) #if defined(__clang__) diff --git a/libcef/renderer/frame_impl.h b/libcef/renderer/frame_impl.h index 9b8e42132..01a91f8b4 100644 --- a/libcef/renderer/frame_impl.h +++ b/libcef/renderer/frame_impl.h @@ -11,6 +11,7 @@ #include "include/cef_frame.h" #include "include/cef_v8.h" +#include "libcef/renderer/blink_glue.h" #include "base/memory/weak_ptr.h" #include "base/timer/timer.h" @@ -37,7 +38,10 @@ class CefBrowserImpl; // Implementation of CefFrame. CefFrameImpl objects are owned by the // CefBrowerImpl and will be detached when the browser is notified that the // associated renderer WebFrame will close. -class CefFrameImpl : public CefFrame, public cef::mojom::RenderFrame { +class CefFrameImpl + : public CefFrame, + public cef::mojom::RenderFrame, + public blink_glue::CefExecutionContextLifecycleStateObserver { public: CefFrameImpl(CefBrowserImpl* browser, blink::WebLocalFrame* frame, @@ -90,7 +94,8 @@ class CefFrameImpl : public CefFrame, public cef::mojom::RenderFrame { void OnWasShown(); void OnDidFinishLoad(); void OnDraggableRegionsChanged(); - void OnContextCreated(); + void OnContextCreated(v8::Local context); + void OnContextReleased(); void OnDetached(); blink::WebLocalFrame* web_frame() const { return frame_; } @@ -139,6 +144,10 @@ class CefFrameImpl : public CefFrame, public cef::mojom::RenderFrame { void DidStopLoading() override; void MoveOrResizeStarted() override; + // blink_glue::CefExecutionContextLifecycleStateObserver methods: + void ContextLifecycleStateChanged( + blink::mojom::blink::FrameLifecycleState state) override; + CefBrowserImpl* browser_; blink::WebLocalFrame* frame_; const int64 frame_id_; @@ -168,6 +177,9 @@ class CefFrameImpl : public CefFrame, public cef::mojom::RenderFrame { mojo::Remote browser_frame_; + std::unique_ptr + execution_context_lifecycle_state_observer_; + base::WeakPtrFactory weak_ptr_factory_{this}; IMPLEMENT_REFCOUNTING(CefFrameImpl); diff --git a/libcef/renderer/render_frame_observer.cc b/libcef/renderer/render_frame_observer.cc index bd172c8e9..d8a670ab9 100644 --- a/libcef/renderer/render_frame_observer.cc +++ b/libcef/renderer/render_frame_observer.cc @@ -148,7 +148,7 @@ void CefRenderFrameObserver::DidCreateScriptContext( } // Do this last, in case the client callback modified the window object. - framePtr->OnContextCreated(); + framePtr->OnContextCreated(context); } void CefRenderFrameObserver::WillReleaseScriptContext( @@ -157,31 +157,32 @@ void CefRenderFrameObserver::WillReleaseScriptContext( blink::WebLocalFrame* frame = render_frame()->GetWebFrame(); CefRefPtr browserPtr = CefBrowserImpl::GetBrowserForMainFrame(frame->Top()); - if (browserPtr) { - CefRefPtr application = CefAppManager::Get()->GetApplication(); - if (application) { - CefRefPtr handler = - application->GetRenderProcessHandler(); - if (handler) { - CefRefPtr framePtr = browserPtr->GetWebFrameImpl(frame); + if (!browserPtr) + return; - v8::Isolate* isolate = blink::MainThreadIsolate(); - v8::HandleScope handle_scope(isolate); + CefRefPtr handler; + CefRefPtr application = CefAppManager::Get()->GetApplication(); + if (application) + handler = application->GetRenderProcessHandler(); - // The released context should not be used for script execution. - // Depending on how the context is released this may or may not already - // be set. - blink_glue::CefScriptForbiddenScope forbidScript; + CefRefPtr framePtr = browserPtr->GetWebFrameImpl(frame); - CefRefPtr contextPtr( - new CefV8ContextImpl(isolate, context)); + if (handler) { + v8::Isolate* isolate = blink::MainThreadIsolate(); + v8::HandleScope handle_scope(isolate); - handler->OnContextReleased(browserPtr.get(), framePtr.get(), - contextPtr); - } - } + // The released context should not be used for script execution. + // Depending on how the context is released this may or may not already + // be set. + blink_glue::CefScriptForbiddenScope forbidScript; + + CefRefPtr contextPtr(new CefV8ContextImpl(isolate, context)); + + handler->OnContextReleased(browserPtr.get(), framePtr.get(), contextPtr); } + framePtr->OnContextReleased(); + CefV8ReleaseContext(context); } diff --git a/tests/ceftests/frame_handler_unittest.cc b/tests/ceftests/frame_handler_unittest.cc index 6b7518fac..45abe1d78 100644 --- a/tests/ceftests/frame_handler_unittest.cc +++ b/tests/ceftests/frame_handler_unittest.cc @@ -13,6 +13,7 @@ #include "include/wrapper/cef_closure_task.h" #include "tests/ceftests/routing_test_handler.h" #include "tests/ceftests/test_handler.h" +#include "tests/ceftests/test_util.h" #include "tests/gtest/include/gtest/gtest.h" // Set to 1 to enable verbose debugging info logging. @@ -580,7 +581,7 @@ class OrderMainTestHandler : public RoutingTestHandler, public CefFrameHandler { // Messages for the old and new frames are interleaved during cross-origin // navigation. if (pending_main_frame_) { - EXPECT_TRUE(IsCrossOrigin()); + EXPECT_TRUE(IsCrossOriginOrSameSiteBFCacheEnabled()); pending_main_frame_->OnQuery(browser, frame, request); } else { EXPECT_TRUE(current_main_frame_); @@ -602,7 +603,8 @@ class OrderMainTestHandler : public RoutingTestHandler, public CefFrameHandler { pending_main_frame_ = new FrameStatus(frame); pending_main_frame_->SetAdditionalDebugInfo(GetAdditionalDebugInfo()); pending_main_frame_->SetIsFirstMain(!got_after_created_); - pending_main_frame_->SetIsLastMain(!IsCrossOrigin() || IsLastNavigation()); + pending_main_frame_->SetIsLastMain( + !IsCrossOriginOrSameSiteBFCacheEnabled() || IsLastNavigation()); pending_main_frame_->OnFrameCreated(browser, frame); } @@ -614,7 +616,7 @@ class OrderMainTestHandler : public RoutingTestHandler, public CefFrameHandler { // May arrive before or after OnMainFrameChanged switches the frame (after // on initial browser creation, before on cross-origin navigation). if (pending_main_frame_) { - EXPECT_TRUE(IsCrossOrigin()); + EXPECT_TRUE(IsCrossOriginOrSameSiteBFCacheEnabled()); pending_main_frame_->OnFrameAttached(browser, frame); } else { EXPECT_TRUE(current_main_frame_); @@ -665,7 +667,7 @@ class OrderMainTestHandler : public RoutingTestHandler, public CefFrameHandler { if (old_frame && new_frame) { // Main frame changed due to cross-origin navigation. - EXPECT_TRUE(IsCrossOrigin()); + EXPECT_TRUE(IsCrossOriginOrSameSiteBFCacheEnabled()); main_frame_changed_ct_++; } @@ -687,6 +689,10 @@ class OrderMainTestHandler : public RoutingTestHandler, public CefFrameHandler { virtual bool IsLastNavigation() const { return true; } virtual bool IsCrossOrigin() const { return false; } + bool IsCrossOriginOrSameSiteBFCacheEnabled() const { + return IsCrossOrigin() || IsSameSiteBFCacheEnabled(); + } + virtual std::string GetAdditionalDebugInfo() const { return std::string(); } virtual bool AllQueriesDelivered(std::string* msg = nullptr) const { @@ -720,7 +726,7 @@ class OrderMainTestHandler : public RoutingTestHandler, public CefFrameHandler { #endif const std::string& next_url = GetNextMainURL(); if (!next_url.empty()) { - if (!IsCrossOrigin()) { + if (!IsCrossOriginOrSameSiteBFCacheEnabled()) { // Reusing the same main frame for same origin nav. current_main_frame_->ResetMainLoadStatus(); } @@ -790,12 +796,13 @@ const char kOrderMainUrlPrefix[] = "http://tests-frame-handler"; class NavigateOrderMainTestHandler : public OrderMainTestHandler { public: NavigateOrderMainTestHandler(bool cross_origin, int additional_nav_ct = 2) - : cross_origin_(cross_origin), additional_nav_ct_(additional_nav_ct) { - // Once for each cross-origin LoadURL call. - expected_main_frame_changed_ct_ = cross_origin ? additional_nav_ct_ : 0; - } + : cross_origin_(cross_origin), additional_nav_ct_(additional_nav_ct) {} void RunTest() override { + // Once for each cross-origin LoadURL call. + expected_main_frame_changed_ct_ = + IsCrossOriginOrSameSiteBFCacheEnabled() ? additional_nav_ct_ : 0; + // Resources for the 2nd+ navigation. for (int i = 1; i <= additional_nav_ct_; i++) { AddResource(GetURLForNav(i), GetMainHtmlForNav(i), "text/html"); diff --git a/tests/ceftests/message_router_unittest.cc b/tests/ceftests/message_router_unittest.cc index d939a060c..dac2c7310 100644 --- a/tests/ceftests/message_router_unittest.cc +++ b/tests/ceftests/message_router_unittest.cc @@ -12,11 +12,16 @@ #include "include/cef_v8.h" #include "include/wrapper/cef_closure_task.h" #include "tests/ceftests/routing_test_handler.h" +#include "tests/ceftests/test_util.h" #include "tests/gtest/include/gtest/gtest.h" #include "tests/shared/renderer/client_app_renderer.h" using client::ClientAppRenderer; +#define S1(N) #N +#define S2(N) S1(N) +#define LINESTR S2(__LINE__) + namespace { const char kTestDomainRoot[] = "http://tests-mr"; @@ -65,10 +70,12 @@ class MRRenderDelegate : public ClientAppRenderer::Delegate { frame->SendProcessMessage(PID_BROWSER, message); return true; } else { - EXPECT_EQ(1U, arguments.size()); + EXPECT_EQ(2U, arguments.size()); EXPECT_TRUE(arguments[0]->IsInt()); + EXPECT_TRUE(arguments[1]->IsInt()); - const int expected_count = arguments[0]->GetIntValue(); + const int line_no = arguments[0]->GetIntValue(); + const int expected_count = arguments[1]->GetIntValue(); int actual_count = -1; CefRefPtr context = CefV8Context::GetCurrentContext(); @@ -87,8 +94,8 @@ class MRRenderDelegate : public ClientAppRenderer::Delegate { if (expected_count != actual_count) { std::stringstream ss; - ss << message_name << " failed; expected " << expected_count - << ", got " << actual_count; + ss << message_name << " failed (line " << line_no << "); expected " + << expected_count << ", got " << actual_count; exception = ss.str(); } } @@ -329,9 +336,12 @@ class HarnessTestHandler : public SingleLoadTestHandler { html = ""; } else { @@ -339,9 +349,12 @@ class HarnessTestHandler : public SingleLoadTestHandler { html = ""; } @@ -414,9 +427,12 @@ class SingleQueryTestHandler : public SingleLoadTestHandler { html = ""; return html; @@ -874,9 +915,12 @@ class SingleUnhandledQueryTestHandler : public SingleLoadTestHandler { html = ""; return html; @@ -1088,10 +1137,10 @@ class MultiQueryManager : public CefMessageRouterBrowserSide::Handler { // No requests should exist. if (assert_total) - html += "window.mrtAssertTotalCount(0);\n"; + html += "window.mrtAssertTotalCount(" LINESTR ",0);\n"; if (assert_browser) - html += "window.mrtAssertBrowserCount(0);\n"; - html += "window.mrtAssertContextCount(0);\n"; + html += "window.mrtAssertBrowserCount(" LINESTR ",0);\n"; + html += "window.mrtAssertContextCount(" LINESTR ",0);\n"; if (synchronous_) { // Run all of the queries synchronously. None will complete before the @@ -1106,10 +1155,11 @@ class MultiQueryManager : public CefMessageRouterBrowserSide::Handler { // Pending requests should match the total created. const std::string& total_val = GetIntString(total_ct); if (assert_total) - html += "window.mrtAssertTotalCount(" + total_val + ");\n"; + html += "window.mrtAssertTotalCount(" LINESTR "," + total_val + ");\n"; if (assert_browser) - html += "window.mrtAssertBrowserCount(" + total_val + ");\n"; - html += "window.mrtAssertContextCount(" + total_val + ");\n"; + html += + "window.mrtAssertBrowserCount(" LINESTR "," + total_val + ");\n"; + html += "window.mrtAssertContextCount(" LINESTR "," + total_val + ");\n"; int cancel_ct = 0; @@ -1126,10 +1176,13 @@ class MultiQueryManager : public CefMessageRouterBrowserSide::Handler { // Pending requests should match the total not canceled. const std::string& cancel_val = GetIntString(total_ct - cancel_ct); if (assert_total) - html += "window.mrtAssertTotalCount(" + cancel_val + ");\n"; + html += + "window.mrtAssertTotalCount(" LINESTR "," + cancel_val + ");\n"; if (assert_browser) - html += "window.mrtAssertBrowserCount(" + cancel_val + ");\n"; - html += "window.mrtAssertContextCount(" + cancel_val + ");\n"; + html += + "window.mrtAssertBrowserCount(" LINESTR "," + cancel_val + ");\n"; + html += + "window.mrtAssertContextCount(" LINESTR "," + cancel_val + ");\n"; } } else { // Run all of the queries asynchronously. Some may complete before @@ -2876,9 +2929,13 @@ class MultiQueryMultiNavigateTestHandler url3_ = std::string(same_origin_ ? kTestDomain1 : kTestDomain3) + "browser3.html"; - AddManagedResource(url1_, true, true); - AddManagedResource(url2_, true, true); - AddManagedResource(url3_, true, true); + // With same-site BFCache enabled a new browser will be created for each + // same-site navigation in the renderer process, resulting in "total count" + // values that potentially span multiple navigations. + const bool should_assert = !(same_origin_ && IsSameSiteBFCacheEnabled()); + AddManagedResource(url1_, should_assert, should_assert); + AddManagedResource(url2_, should_assert, should_assert); + AddManagedResource(url3_, should_assert, should_assert); Finalize(); // 1. Load the 1st url. diff --git a/tests/ceftests/navigation_unittest.cc b/tests/ceftests/navigation_unittest.cc index f6f4ee3fb..b9ef1acb3 100644 --- a/tests/ceftests/navigation_unittest.cc +++ b/tests/ceftests/navigation_unittest.cc @@ -15,6 +15,9 @@ #include "tests/shared/browser/client_app_browser.h" #include "tests/shared/renderer/client_app_renderer.h" +// Set to 1 to enable verbose debugging info logging. +#define VERBOSE_DEBUGGING 0 + using client::ClientAppBrowser; using client::ClientAppRenderer; @@ -34,7 +37,28 @@ const cef_transition_type_t kTransitionExplicitForwardBack = static_cast(kTransitionExplicitLoad | TT_FORWARD_BACK_FLAG); -enum NavAction { NA_LOAD = 1, NA_BACK, NA_FORWARD, NA_CLEAR }; +enum NavAction { NA_LOAD = 1, NA_BACK, NA_FORWARD }; + +#if VERBOSE_DEBUGGING +const char* NavActionString(NavAction action) { + switch (action) { + case NA_LOAD: + return "LOAD"; + case NA_BACK: + return "BACK"; + case NA_FORWARD: + return "FORWARD"; + } + return "INVALID"; +} +#endif // VERBOSE_DEBUGGING + +bool ExpectResourceLoadEvents(NavAction action) { + if (IsSameSiteBFCacheEnabled()) { + return action == NA_LOAD; + } + return true; +} typedef struct { NavAction action; // What to do @@ -52,8 +76,6 @@ static NavListItem kHNavList[] = { {NA_FORWARD, kHNav2, true, false}, // . X {NA_LOAD, kHNav3, true, false}, // . . X {NA_BACK, kHNav2, true, true}, // . X . - // TODO(cef): Enable once ClearHistory is implemented - // {NA_CLEAR, kHNav2, false, false}, // X }; #define NAV_LIST_SIZE() (sizeof(kHNavList) / sizeof(NavListItem)) @@ -83,6 +105,10 @@ class HistoryNavRendererTest : public ClientAppRenderer::Delegate, bool canGoBack, bool canGoForward) override { const NavListItem& item = kHNavList[nav_]; +#if VERBOSE_DEBUGGING + LOG(INFO) << "render nav=" << nav_ << " " << NavActionString(item.action) + << " OnLoadingStateChange isLoading=" << isLoading; +#endif const std::string& url = browser->GetMainFrame()->GetURL(); EXPECT_STREQ(item.target, url.c_str()); @@ -97,8 +123,10 @@ class HistoryNavRendererTest : public ClientAppRenderer::Delegate, << "nav: " << nav_ << " isLoading: " << isLoading; if (isLoading) { + EXPECT_FALSE(got_loading_state_start_); got_loading_state_start_.yes(); } else { + EXPECT_FALSE(got_loading_state_end_); got_loading_state_end_.yes(); SendTestResultsIfDone(browser, browser->GetMainFrame()); } @@ -108,7 +136,12 @@ class HistoryNavRendererTest : public ClientAppRenderer::Delegate, CefRefPtr frame, TransitionType transition_type) override { const NavListItem& item = kHNavList[nav_]; +#if VERBOSE_DEBUGGING + LOG(INFO) << "render nav=" << nav_ << " " << NavActionString(item.action) + << " OnLoadStart"; +#endif + EXPECT_FALSE(got_load_start_); got_load_start_.yes(); const std::string& url = frame->GetURL(); @@ -124,7 +157,12 @@ class HistoryNavRendererTest : public ClientAppRenderer::Delegate, CefRefPtr frame, int httpStatusCode) override { const NavListItem& item = kHNavList[nav_]; +#if VERBOSE_DEBUGGING + LOG(INFO) << "render nav=" << nav_ << " " << NavActionString(item.action) + << " OnLoadEnd"; +#endif + EXPECT_FALSE(got_load_end_); got_load_end_.yes(); const std::string& url = frame->GetURL(); @@ -146,6 +184,12 @@ class HistoryNavRendererTest : public ClientAppRenderer::Delegate, // Send the test results. void SendTestResults(CefRefPtr browser, CefRefPtr frame) { +#if VERBOSE_DEBUGGING + const NavListItem& item = kHNavList[nav_]; + LOG(INFO) << "render nav=" << nav_ << " " << NavActionString(item.action) + << " SendTestResults"; +#endif + EXPECT_TRUE(got_loading_state_start_); EXPECT_TRUE(got_loading_state_end_); EXPECT_TRUE(got_load_start_); @@ -273,11 +317,7 @@ class NavigationEntryVisitor : public CefNavigationEntryVisitor { // Browser side. class HistoryNavTestHandler : public TestHandler { public: - HistoryNavTestHandler() - : nav_(0), - load_end_confirmation_(false), - load_state_change_loaded_confirmation_(false), - renderer_confirmation_(false) {} + HistoryNavTestHandler() = default; void RunTest() override { // Add the resources that we will navigate to/from. @@ -322,19 +362,20 @@ class HistoryNavTestHandler : public TestHandler { case NA_FORWARD: browser->GoForward(); break; - case NA_CLEAR: - // TODO(cef): Enable once ClearHistory is implemented - // browser->GetHost()->ClearHistory(); - // Not really a navigation action so go to the next one. - nav_++; - RunNav(browser); - break; default: break; } } void RunNextNavIfReady(CefRefPtr browser) { +#if VERBOSE_DEBUGGING + LOG(INFO) << "browser nav=" << nav_ + << " load_end_confirmation_=" << load_end_confirmation_ + << " load_state_change_loaded_confirmation_=" + << load_state_change_loaded_confirmation_ + << " renderer_confirmation_=" << renderer_confirmation_; +#endif + if (load_end_confirmation_ && load_state_change_loaded_confirmation_ && renderer_confirmation_) { load_end_confirmation_ = false; @@ -358,6 +399,12 @@ class HistoryNavTestHandler : public TestHandler { bool is_redirect) override { const NavListItem& item = kHNavList[nav_]; +#if VERBOSE_DEBUGGING + LOG(INFO) << "browser nav=" << nav_ << " " << NavActionString(item.action) + << " OnBeforeBrowse"; +#endif + + EXPECT_FALSE(got_before_browse_[nav_]); got_before_browse_[nav_].yes(); std::string url = request->GetURL(); @@ -409,6 +456,7 @@ class HistoryNavTestHandler : public TestHandler { << "nav=" << nav_ << " url=" << url; } + EXPECT_FALSE(got_before_resource_load_[nav_]); got_before_resource_load_[nav_].yes(); if (url == item.target) @@ -421,12 +469,21 @@ class HistoryNavTestHandler : public TestHandler { bool isLoading, bool canGoBack, bool canGoForward) override { - if (isLoading) - return; - const NavListItem& item = kHNavList[nav_]; - got_loading_state_change_[nav_].yes(); +#if VERBOSE_DEBUGGING + LOG(INFO) << "browser nav=" << nav_ << " " << NavActionString(item.action) + << " OnLoadingStateChange isLoading=" << isLoading; +#endif + + if (isLoading) { + EXPECT_FALSE(got_loading_state_change_loading_[nav_]); + got_loading_state_change_loading_[nav_].yes(); + return; + } + + EXPECT_FALSE(got_loading_state_change_loaded_[nav_]); + got_loading_state_change_loaded_[nav_].yes(); if (item.can_go_back == canGoBack) got_correct_can_go_back_[nav_].yes(); @@ -445,6 +502,12 @@ class HistoryNavTestHandler : public TestHandler { const NavListItem& item = kHNavList[nav_]; +#if VERBOSE_DEBUGGING + LOG(INFO) << "browser nav=" << nav_ << " " << NavActionString(item.action) + << " OnLoadStart"; +#endif + + EXPECT_FALSE(got_load_start_[nav_]); got_load_start_[nav_].yes(); if (item.action == NA_LOAD) { @@ -467,6 +530,12 @@ class HistoryNavTestHandler : public TestHandler { const NavListItem& item = kHNavList[nav_]; +#if VERBOSE_DEBUGGING + LOG(INFO) << "browser nav=" << nav_ << " " << NavActionString(item.action) + << " OnLoadEnd"; +#endif + + EXPECT_FALSE(got_load_end_[nav_]); got_load_end_[nav_].yes(); // Test that navigation entries are correct. @@ -489,6 +558,7 @@ class HistoryNavTestHandler : public TestHandler { CefProcessId source_process, CefRefPtr message) override { if (message->GetName().ToString() == kHistoryNavMsg) { + EXPECT_FALSE(got_before_navigation_[nav_]); got_before_navigation_[nav_].yes(); // Test that the renderer side succeeded. @@ -506,16 +576,17 @@ class HistoryNavTestHandler : public TestHandler { return false; } - int nav_; - bool load_end_confirmation_; - bool load_state_change_loaded_confirmation_; - bool renderer_confirmation_; + int nav_ = 0; + bool load_end_confirmation_ = false; + bool load_state_change_loaded_confirmation_ = false; + bool renderer_confirmation_ = false; TrackCallback got_before_browse_[NAV_LIST_SIZE()]; TrackCallback got_before_navigation_[NAV_LIST_SIZE()]; TrackCallback got_before_resource_load_[NAV_LIST_SIZE()]; TrackCallback got_correct_target_[NAV_LIST_SIZE()]; - TrackCallback got_loading_state_change_[NAV_LIST_SIZE()]; + TrackCallback got_loading_state_change_loading_[NAV_LIST_SIZE()]; + TrackCallback got_loading_state_change_loaded_[NAV_LIST_SIZE()]; TrackCallback got_correct_can_go_back_[NAV_LIST_SIZE()]; TrackCallback got_correct_can_go_forward_[NAV_LIST_SIZE()]; TrackCallback got_load_start_[NAV_LIST_SIZE()]; @@ -535,24 +606,27 @@ TEST(NavigationTest, History) { handler->ExecuteTest(); for (size_t i = 0; i < NAV_LIST_SIZE(); ++i) { - if (kHNavList[i].action != NA_CLEAR) { - ASSERT_TRUE(handler->got_before_browse_[i]) << "i = " << i; - ASSERT_TRUE(handler->got_before_navigation_[i]) << "i = " << i; - ASSERT_TRUE(handler->got_before_resource_load_[i]) << "i = " << i; - ASSERT_TRUE(handler->got_correct_target_[i]) << "i = " << i; - ASSERT_TRUE(handler->got_load_start_[i]) << "i = " << i; - ASSERT_TRUE(handler->got_correct_load_start_url_[i]) << "i = " << i; + if (ExpectResourceLoadEvents(kHNavList[i].action)) { + EXPECT_TRUE(handler->got_before_browse_[i]) << "i = " << i; + EXPECT_TRUE(handler->got_before_resource_load_[i]) << "i = " << i; + EXPECT_TRUE(handler->got_correct_target_[i]) << "i = " << i; + } else { + EXPECT_FALSE(handler->got_before_browse_[i]) << "i = " << i; + EXPECT_FALSE(handler->got_before_resource_load_[i]) << "i = " << i; + EXPECT_FALSE(handler->got_correct_target_[i]) << "i = " << i; } - ASSERT_TRUE(handler->got_loading_state_change_[i]) << "i = " << i; - ASSERT_TRUE(handler->got_correct_can_go_back_[i]) << "i = " << i; - ASSERT_TRUE(handler->got_correct_can_go_forward_[i]) << "i = " << i; + EXPECT_TRUE(handler->got_before_navigation_[i]) << "i = " << i; + EXPECT_TRUE(handler->got_load_start_[i]) << "i = " << i; + EXPECT_TRUE(handler->got_correct_load_start_url_[i]) << "i = " << i; + EXPECT_TRUE(handler->got_load_end_[i]) << "i = " << i; + EXPECT_TRUE(handler->got_correct_load_end_url_[i]) << "i = " << i; + EXPECT_TRUE(handler->got_correct_history_[i]) << "i = " << i; - if (kHNavList[i].action != NA_CLEAR) { - ASSERT_TRUE(handler->got_load_end_[i]) << "i = " << i; - ASSERT_TRUE(handler->got_correct_history_[i]) << "i = " << i; - ASSERT_TRUE(handler->got_correct_load_end_url_[i]) << "i = " << i; - } + EXPECT_TRUE(handler->got_loading_state_change_loading_[i]) << "i = " << i; + EXPECT_TRUE(handler->got_loading_state_change_loaded_[i]) << "i = " << i; + EXPECT_TRUE(handler->got_correct_can_go_back_[i]) << "i = " << i; + EXPECT_TRUE(handler->got_correct_can_go_forward_[i]) << "i = " << i; } ReleaseAndWaitForDestructor(handler); @@ -1004,22 +1078,22 @@ TEST(NavigationTest, Redirect) { CefClearSchemeHandlerFactories(); WaitForIOThread(); - ASSERT_TRUE(handler->got_nav1_before_resource_load_); - ASSERT_TRUE(handler->got_nav3_before_resource_load_); - ASSERT_TRUE(handler->got_nav4_before_resource_load_); - ASSERT_FALSE(handler->got_invalid_before_resource_load_); - ASSERT_TRUE(handler->got_nav4_load_start_); - ASSERT_FALSE(handler->got_invalid_load_start_); - ASSERT_TRUE(handler->got_nav4_load_end_); - ASSERT_FALSE(handler->got_invalid_load_end_); - ASSERT_TRUE(handler->got_nav1_redirect_); - ASSERT_FALSE(handler->got_nav2_redirect_); - ASSERT_TRUE(handler->got_nav3_redirect_); - ASSERT_FALSE(handler->got_invalid_redirect_); - ASSERT_TRUE(g_got_nav1_request); - ASSERT_TRUE(g_got_nav3_request); - ASSERT_TRUE(g_got_nav4_request); - ASSERT_FALSE(g_got_invalid_request); + EXPECT_TRUE(handler->got_nav1_before_resource_load_); + EXPECT_TRUE(handler->got_nav3_before_resource_load_); + EXPECT_TRUE(handler->got_nav4_before_resource_load_); + EXPECT_FALSE(handler->got_invalid_before_resource_load_); + EXPECT_TRUE(handler->got_nav4_load_start_); + EXPECT_FALSE(handler->got_invalid_load_start_); + EXPECT_TRUE(handler->got_nav4_load_end_); + EXPECT_FALSE(handler->got_invalid_load_end_); + EXPECT_TRUE(handler->got_nav1_redirect_); + EXPECT_FALSE(handler->got_nav2_redirect_); + EXPECT_TRUE(handler->got_nav3_redirect_); + EXPECT_FALSE(handler->got_invalid_redirect_); + EXPECT_TRUE(g_got_nav1_request); + EXPECT_TRUE(g_got_nav3_request); + EXPECT_TRUE(g_got_nav4_request); + EXPECT_FALSE(g_got_invalid_request); ReleaseAndWaitForDestructor(handler); } @@ -1038,11 +1112,11 @@ TEST(NavigationTest, RedirectDestroy) { CefClearSchemeHandlerFactories(); WaitForIOThread(); - ASSERT_TRUE(handler->got_nav1_redirect_); - ASSERT_TRUE(g_got_nav1_request); - ASSERT_FALSE(g_got_nav3_request); - ASSERT_FALSE(g_got_nav4_request); - ASSERT_FALSE(g_got_invalid_request); + EXPECT_TRUE(handler->got_nav1_redirect_); + EXPECT_TRUE(g_got_nav1_request); + EXPECT_FALSE(g_got_nav3_request); + EXPECT_FALSE(g_got_nav4_request); + EXPECT_FALSE(g_got_invalid_request); ReleaseAndWaitForDestructor(handler); } @@ -1613,8 +1687,7 @@ const char kLoadNavTestCmdKey[] = "nav-load-test"; class LoadNavRendererTest : public ClientAppRenderer::Delegate, public CefLoadHandler { public: - LoadNavRendererTest() : run_test_(false), browser_id_(0), load_ct_(0) {} - ~LoadNavRendererTest() override { EXPECT_EQ(0, browser_id_); } + LoadNavRendererTest() = default; void OnBrowserCreated(CefRefPtr app, CefRefPtr browser, @@ -1623,10 +1696,16 @@ class LoadNavRendererTest : public ClientAppRenderer::Delegate, if (!run_test_) return; - EXPECT_EQ(0, browser_id_); - browser_id_ = browser->GetIdentifier(); - EXPECT_GT(browser_id_, 0); - got_browser_created_.yes(); + // We'll get multiple calls to OnBrowserCreated for same-site navigations + // with same-site BFCache enabled. + const int new_browser_id = browser->GetIdentifier(); + if (browser_id_ != 0) { + EXPECT_EQ(browser_id_, new_browser_id); + } else { + browser_id_ = new_browser_id; + EXPECT_GT(browser_id_, 0); + } + browser_created_ct_++; } void OnBrowserDestroyed(CefRefPtr app, @@ -1634,11 +1713,9 @@ class LoadNavRendererTest : public ClientAppRenderer::Delegate, if (!run_test_) return; - EXPECT_TRUE(got_browser_created_); - EXPECT_TRUE(got_loading_state_end_); - + EXPECT_GT(load_ct_, 0); + EXPECT_GT(browser_created_ct_, 0); EXPECT_EQ(browser_id_, browser->GetIdentifier()); - browser_id_ = 0; } CefRefPtr GetLoadHandler( @@ -1654,10 +1731,7 @@ class LoadNavRendererTest : public ClientAppRenderer::Delegate, bool canGoBack, bool canGoForward) override { if (!isLoading) { - EXPECT_TRUE(got_browser_created_); - - got_loading_state_end_.yes(); - + EXPECT_GT(browser_created_ct_, 0); EXPECT_EQ(browser_id_, browser->GetIdentifier()); load_ct_++; @@ -1679,16 +1753,16 @@ class LoadNavRendererTest : public ClientAppRenderer::Delegate, EXPECT_TRUE(args.get()); EXPECT_TRUE(args->SetBool(0, result)); EXPECT_TRUE(args->SetInt(1, browser->GetIdentifier())); - EXPECT_TRUE(args->SetInt(2, load_ct_)); + EXPECT_TRUE(args->SetInt(2, browser_created_ct_)); + EXPECT_TRUE(args->SetInt(3, load_ct_)); frame->SendProcessMessage(PID_BROWSER, return_msg); } - bool run_test_; + bool run_test_ = false; - int browser_id_; - int load_ct_; - TrackCallback got_browser_created_; - TrackCallback got_loading_state_end_; + int browser_id_ = 0; + int load_ct_ = 0; + int browser_created_ct_ = 0; IMPLEMENT_REFCOUNTING(LoadNavRendererTest); }; @@ -1708,9 +1782,7 @@ class LoadNavTestHandler : public TestHandler { bool cancel_in_open_url = false) : mode_(mode), same_origin_(same_origin), - cancel_in_open_url_(cancel_in_open_url), - browser_id_current_(0), - renderer_load_ct_(0) {} + cancel_in_open_url_(cancel_in_open_url) {} std::string GetURL2() const { return same_origin_ ? kLoadNavSameOrigin2 : kLoadNavCrossOrigin2; @@ -1961,7 +2033,9 @@ class LoadNavTestHandler : public TestHandler { EXPECT_EQ(browser_id_current_, args->GetInt(1)); - renderer_load_ct_ = args->GetInt(2); + renderer_browser_created_ct_ = args->GetInt(2); + EXPECT_GE(renderer_browser_created_ct_, 1); + renderer_load_ct_ = args->GetInt(3); EXPECT_GE(renderer_load_ct_, 1); // Continue with the test. @@ -1985,6 +2059,7 @@ class LoadNavTestHandler : public TestHandler { // We should only navigate a single time if the 2nd load is canceled. EXPECT_EQ(1, renderer_load_ct_); + EXPECT_EQ(1, renderer_browser_created_ct_); } else { EXPECT_TRUE(got_before_browse_); EXPECT_TRUE(got_before_resource_load_); @@ -1995,9 +2070,17 @@ class LoadNavTestHandler : public TestHandler { if (same_origin_) { // The renderer process should always be reused. EXPECT_EQ(2, renderer_load_ct_); + if (IsSameSiteBFCacheEnabled()) { + // We expect multiple calls to OnBrowserCreated for same-site + // navigations with same-site BFCache enabled. + EXPECT_EQ(2, renderer_browser_created_ct_); + } else { + EXPECT_EQ(1, renderer_browser_created_ct_); + } } else { // Each renderer process is only used for a single navigation. EXPECT_EQ(1, renderer_load_ct_); + EXPECT_EQ(1, renderer_browser_created_ct_); } } @@ -2014,8 +2097,9 @@ class LoadNavTestHandler : public TestHandler { const bool same_origin_; const bool cancel_in_open_url_; - int browser_id_current_; - int renderer_load_ct_; + int browser_id_current_ = 0; + int renderer_browser_created_ct_ = 0; + int renderer_load_ct_ = 0; TrackCallback got_before_browse_; TrackCallback got_open_url_from_tab_; diff --git a/tests/ceftests/test_util.cc b/tests/ceftests/test_util.cc index f8ea0995e..723c3129e 100644 --- a/tests/ceftests/test_util.cc +++ b/tests/ceftests/test_util.cc @@ -294,6 +294,28 @@ bool IsChromeRuntimeEnabled() { return state ? true : false; } +bool IsBFCacheEnabled() { + // Supported by the Chrome runtime only, see issue #3237. + if (!IsChromeRuntimeEnabled()) + return false; + + // Enabled by default starting in M96. + static int state = -1; + if (state == -1) { + CefRefPtr command_line = + CefCommandLine::GetGlobalCommandLine(); + const std::string& value = command_line->GetSwitchValue("disable-features"); + state = value.find("BackForwardCache") == std::string::npos ? 1 : 0; + } + return state ? true : false; +} + +bool IsSameSiteBFCacheEnabled() { + // Same-site BFCache is enabled by default starting in M101 and does not have + // a separate configuration flag. + return IsBFCacheEnabled(); +} + bool IgnoreURL(const std::string& url) { return IsChromeRuntimeEnabled() && url.find("/favicon.ico") != std::string::npos; diff --git a/tests/ceftests/test_util.h b/tests/ceftests/test_util.h index b95a8b38f..14906b734 100644 --- a/tests/ceftests/test_util.h +++ b/tests/ceftests/test_util.h @@ -84,6 +84,12 @@ bool TestOldResourceAPI(); // Returns true if the Chrome runtime is enabled. bool IsChromeRuntimeEnabled(); +// Returns true if BFCache is enabled. +bool IsBFCacheEnabled(); + +// Returns true if same-site BFCache is enabled. +bool IsSameSiteBFCacheEnabled(); + // Returns true if requests for |url| should be ignored by tests. bool IgnoreURL(const std::string& url);