From 7a56371b84fb67f3ac0dad1a0dadc604fe584e54 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Wed, 15 Sep 2021 14:40:08 +0300 Subject: [PATCH] Fix draggable region update with BackForwardCache enabled (see issue #2421) When BackForwardCache is enabled and the user navigates the main frame back/forward a new RFH may be created for an existing main frame GlobalId value and CefFrameHostImpl (e.g. an object that was previously Detach()ed after main frame navigation called SetMainFrame, but for which RenderFrameDeleted was not subsequently called due to insertion in the BackForwardCache). In this case we can re-attach the new RFH to the existing main frame CefFrameHostImpl in RenderFrameHostStateChanged and resume processing of messages. Swapping back/forward to an existing (already loaded) renderer does not trigger new notifications for draggable regions (e.g. RenderFrameObserver:: DraggableRegionsChanged is not called by default). We therefore explicitly request an update of draggable regions by sending the DidStopLoading message to the renderer. A new |reattached| parameter is added to CefFrameHandler::OnFrameAttached to support identification of BackForwardCache usage by the client. To test with unit tests: Run `ceftests --gtest_filter=DraggableRegionsTest.DraggableRegionsCrossOrigin --enable-features=BackForwardCache` To test manually: 1. Run `cefclient --enable-features=BackForwardCache --use-views --url=http://tests/draggable`, note that draggable regions work. 2. Load https://www.google.com via the address bar, note that draggable regions are removed. 3. Go back to http://tests/draggable, note that draggable regions work. 4. Go forward to https://www.google.com, note that draggable regions are removed. --- include/capi/cef_frame_handler_capi.h | 9 +- include/cef_api_hash.h | 8 +- include/cef_frame_handler.h | 7 +- libcef/browser/browser_info.cc | 47 +++++- libcef/browser/browser_info.h | 8 + libcef/browser/frame_host_impl.cc | 52 +++++-- libcef/browser/frame_host_impl.h | 8 + libcef/renderer/frame_impl.cc | 11 ++ libcef_dll/cpptoc/frame_handler_cpptoc.cc | 8 +- libcef_dll/ctocpp/frame_handler_ctocpp.cc | 7 +- libcef_dll/ctocpp/frame_handler_ctocpp.h | 5 +- tests/ceftests/draggable_regions_unittest.cc | 154 ++++++++++++++----- tests/ceftests/frame_handler_unittest.cc | 13 +- 13 files changed, 261 insertions(+), 76 deletions(-) diff --git a/include/capi/cef_frame_handler_capi.h b/include/capi/cef_frame_handler_capi.h index 99e81876d..330e16a26 100644 --- a/include/capi/cef_frame_handler_capi.h +++ b/include/capi/cef_frame_handler_capi.h @@ -33,7 +33,7 @@ // by hand. See the translator.README.txt file in the tools directory for // more information. // -// $hash=503984bf98aa52ff67ce52f26a560bbb1d4439bc$ +// $hash=f6be5f7509ee3ccfe16f226470897223cc131014$ // #ifndef CEF_INCLUDE_CAPI_CEF_FRAME_HANDLER_CAPI_H_ @@ -147,11 +147,14 @@ typedef struct _cef_frame_handler_t { /// // Called when a frame can begin routing commands to/from the associated - // renderer process. Any commands that were queued have now been dispatched. + // renderer process. |reattached| will be true (1) if the frame was re- + // attached after exiting the BackForwardCache. Any commands that were queued + // have now been dispatched. /// void(CEF_CALLBACK* on_frame_attached)(struct _cef_frame_handler_t* self, struct _cef_browser_t* browser, - struct _cef_frame_t* frame); + struct _cef_frame_t* frame, + int reattached); /// // Called when a frame loses its connection to the renderer process and will diff --git a/include/cef_api_hash.h b/include/cef_api_hash.h index 46175fad6..64d851d07 100644 --- a/include/cef_api_hash.h +++ b/include/cef_api_hash.h @@ -42,13 +42,13 @@ // way that may cause binary incompatibility with other builds. The universal // hash value will change if any platform is affected whereas the platform hash // values will change only if that particular platform is affected. -#define CEF_API_HASH_UNIVERSAL "e32fc367fb311d2d68f097d42a75357a3e339fdd" +#define CEF_API_HASH_UNIVERSAL "c464806198318de6438ad40a51ad3337d148de5f" #if defined(OS_WIN) -#define CEF_API_HASH_PLATFORM "e40f20dc24610956c9e2768f4c9535502d68f70e" +#define CEF_API_HASH_PLATFORM "65859434903e434f3c1680624ececd22e963b19a" #elif defined(OS_MAC) -#define CEF_API_HASH_PLATFORM "cb578fa9c253d93944f63d9bdc7a8f9e0a417bae" +#define CEF_API_HASH_PLATFORM "237e6ca8d5187f1bf69520f2eb7b3d3fa7bc27b1" #elif defined(OS_LINUX) -#define CEF_API_HASH_PLATFORM "10b47123ea31edcf4a978a36e4ee2aa5bc755b2d" +#define CEF_API_HASH_PLATFORM "95f07bfbfdc0cde077bd76ffbe875c1cdcfa4974" #endif #ifdef __cplusplus diff --git a/include/cef_frame_handler.h b/include/cef_frame_handler.h index 5d53c0ae4..540aa951a 100644 --- a/include/cef_frame_handler.h +++ b/include/cef_frame_handler.h @@ -132,11 +132,14 @@ class CefFrameHandler : public virtual CefBaseRefCounted { /// // Called when a frame can begin routing commands to/from the associated - // renderer process. Any commands that were queued have now been dispatched. + // renderer process. |reattached| will be true if the frame was re-attached + // after exiting the BackForwardCache. Any commands that were queued have now + // been dispatched. /// /*--cef()--*/ virtual void OnFrameAttached(CefRefPtr browser, - CefRefPtr frame) {} + CefRefPtr frame, + bool reattached) {} /// // Called when a frame loses its connection to the renderer process and will diff --git a/libcef/browser/browser_info.cc b/libcef/browser/browser_info.cc index ada824d5b..1142db954 100644 --- a/libcef/browser/browser_info.cc +++ b/libcef/browser/browser_info.cc @@ -155,7 +155,26 @@ void CefBrowserInfo::FrameHostStateChanged( content::RenderFrameHost::LifecycleState new_state) { CEF_REQUIRE_UIT(); - // We currently only care about BackForwardCache state changes. + if ((old_state == content::RenderFrameHost::LifecycleState::kPrerendering || + old_state == + content::RenderFrameHost::LifecycleState::kInBackForwardCache) && + new_state == content::RenderFrameHost::LifecycleState::kActive) { + if (auto frame = GetFrameForHost(host)) { + // 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); + } + + // Update draggable regions. + frame->MaybeSendDidStopLoading(); + } + } + + // Update BackForwardCache state. bool added_to_bfcache = new_state == content::RenderFrameHost::LifecycleState::kInBackForwardCache; @@ -345,6 +364,26 @@ void CefBrowserInfo::MaybeExecuteFrameNotification( std::move(pending_action).Run(frame_handler); } +void CefBrowserInfo::MaybeNotifyDraggableRegionsChanged( + CefRefPtr browser, + CefRefPtr frame, + std::vector draggable_regions) { + CEF_REQUIRE_UIT(); + DCHECK(frame->IsMain()); + + if (draggable_regions == draggable_regions_) + return; + + draggable_regions_ = std::move(draggable_regions); + + if (auto client = browser->GetClient()) { + if (auto handler = client->GetDragHandler()) { + handler->OnDraggableRegionsChanged(browser.get(), frame, + draggable_regions_); + } + } +} + // Passing in |browser| here because |browser_| may already be cleared. void CefBrowserInfo::SetMainFrame(CefRefPtr browser, CefRefPtr frame) { @@ -352,6 +391,12 @@ void CefBrowserInfo::SetMainFrame(CefRefPtr browser, DCHECK(browser); DCHECK(!frame || frame->IsMain()); + if (frame && main_frame_ && + frame->GetIdentifier() == main_frame_->GetIdentifier()) { + // Nothing to do. + return; + } + CefRefPtr old_frame; if (main_frame_) { old_frame = main_frame_; diff --git a/libcef/browser/browser_info.h b/libcef/browser/browser_info.h index 0a2126075..18863f089 100644 --- a/libcef/browser/browser_info.h +++ b/libcef/browser/browser_info.h @@ -147,6 +147,11 @@ class CefBrowserInfo : public base::RefCountedThreadSafe { // executed immediately. void MaybeExecuteFrameNotification(FrameNotifyOnceAction pending_action); + void MaybeNotifyDraggableRegionsChanged( + CefRefPtr browser, + CefRefPtr frame, + std::vector draggable_regions); + private: friend class base::RefCountedThreadSafe; @@ -234,6 +239,9 @@ class CefBrowserInfo : public base::RefCountedThreadSafe { // The current main frame. CefRefPtr main_frame_; + // Only accessed on the UI thread. + std::vector draggable_regions_; + DISALLOW_COPY_AND_ASSIGN(CefBrowserInfo); }; diff --git a/libcef/browser/frame_host_impl.cc b/libcef/browser/frame_host_impl.cc index c38680fbe..63dd40702 100644 --- a/libcef/browser/frame_host_impl.cc +++ b/libcef/browser/frame_host_impl.cc @@ -460,6 +460,36 @@ bool CefFrameHostImpl::Detach() { return first_detach; } +void CefFrameHostImpl::MaybeReAttach( + scoped_refptr browser_info, + content::RenderFrameHost* render_frame_host) { + CEF_REQUIRE_UIT(); + if (is_attached_ && render_frame_host_ == render_frame_host) { + // Nothing to do here. + return; + } + + // We expect that Detach() was called previously. + CHECK(!is_temporary()); + CHECK(!is_attached_); + CHECK(!render_frame_host_); + + // The RFH may change but the GlobalId should remain the same. + CHECK_EQ(frame_id_, + frame_util::MakeFrameId(render_frame_host->GetGlobalId())); + + { + base::AutoLock lock_scope(state_lock_); + browser_info_ = browser_info; + } + + render_frame_host_ = render_frame_host; + RefreshAttributes(); + + // Restore the RenderFrame connection. + FrameAttachedInternal(/*reattached=*/true); +} + // kMainFrameId must be -1 to align with renderer expectations. const int64_t CefFrameHostImpl::kMainFrameId = -1; const int64_t CefFrameHostImpl::kFocusedFrameId = -2; @@ -552,6 +582,10 @@ void CefFrameHostImpl::SendMessage(const std::string& name, } void CefFrameHostImpl::FrameAttached() { + FrameAttachedInternal(/*reattached=*/false); +} + +void CefFrameHostImpl::FrameAttachedInternal(bool reattached) { CEF_REQUIRE_UIT(); auto browser_info = GetBrowserInfo(); @@ -573,13 +607,13 @@ void CefFrameHostImpl::FrameAttached() { } browser_info->MaybeExecuteFrameNotification(base::BindOnce( - [](CefRefPtr self, + [](CefRefPtr self, bool reattached, CefRefPtr handler) { if (auto browser = self->GetBrowserHostBase()) { - handler->OnFrameAttached(browser, self); + handler->OnFrameAttached(browser, self, reattached); } }, - CefRefPtr(this))); + CefRefPtr(this), reattached)); } } @@ -596,13 +630,6 @@ void CefFrameHostImpl::UpdateDraggableRegions( if (!browser) return; - CefRefPtr handler; - auto client = browser->GetClient(); - if (client) - handler = client->GetDragHandler(); - if (!handler) - return; - std::vector draggable_regions; if (regions) { draggable_regions.reserve(regions->size()); @@ -615,7 +642,10 @@ void CefFrameHostImpl::UpdateDraggableRegions( } } - handler->OnDraggableRegionsChanged(browser.get(), this, draggable_regions); + // Delegate to BrowserInfo so that current state is maintained with + // cross-origin navigation. + browser_info_->MaybeNotifyDraggableRegionsChanged( + browser, this, std::move(draggable_regions)); } void CefExecuteJavaScriptWithUserGestureForTests(CefRefPtr frame, diff --git a/libcef/browser/frame_host_impl.h b/libcef/browser/frame_host_impl.h index 42e35031b..e5ca3ee8c 100644 --- a/libcef/browser/frame_host_impl.h +++ b/libcef/browser/frame_host_impl.h @@ -119,6 +119,12 @@ class CefFrameHostImpl : public CefFrame, public cef::mojom::BrowserFrame { // if this was the first call to Detach() for the frame. bool Detach(); + // A frame has swapped to active status from prerendering or the back-forward + // cache. We may need to re-attach if the RFH has changed. See + // https://crbug.com/1179502#c8 for additional background. + void MaybeReAttach(scoped_refptr browser_info, + content::RenderFrameHost* render_frame_host); + // cef::mojom::BrowserFrame methods forwarded from CefBrowserFrame. void SendMessage(const std::string& name, base::Value arguments) override; void FrameAttached() override; @@ -153,6 +159,8 @@ class CefFrameHostImpl : public CefFrame, public cef::mojom::BrowserFrame { void SendToRenderFrame(const std::string& function_name, RenderFrameAction action); + void FrameAttachedInternal(bool reattached); + const bool is_main_frame_; // The following members may be read/modified from any thread. All access must diff --git a/libcef/renderer/frame_impl.cc b/libcef/renderer/frame_impl.cc index 576937107..1039cea98 100644 --- a/libcef/renderer/frame_impl.cc +++ b/libcef/renderer/frame_impl.cc @@ -335,6 +335,12 @@ void CefFrameImpl::OnDidFinishLoad() { } void CefFrameImpl::OnDraggableRegionsChanged() { + // Match the behavior in ChromeRenderFrameObserver::DraggableRegionsChanged. + // Only the main frame is allowed to control draggable regions, to avoid other + // frames manipulate the regions in the browser process. + if (frame_->Parent() != nullptr) + return; + blink::WebVector webregions = frame_->GetDocument().DraggableRegions(); std::vector regions; @@ -508,6 +514,11 @@ void CefFrameImpl::DidStopLoading() { // the same browser then the other occurrences will be discarded in // OnLoadingStateChange. browser_->OnLoadingStateChange(false); + + // Refresh draggable regions. Otherwise, we may not receive updated regions + // after navigation because LocalFrameView::UpdateDocumentAnnotatedRegion + // lacks sufficient context. + OnDraggableRegionsChanged(); } void CefFrameImpl::MoveOrResizeStarted() { diff --git a/libcef_dll/cpptoc/frame_handler_cpptoc.cc b/libcef_dll/cpptoc/frame_handler_cpptoc.cc index 322873c9f..cfe431373 100644 --- a/libcef_dll/cpptoc/frame_handler_cpptoc.cc +++ b/libcef_dll/cpptoc/frame_handler_cpptoc.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=4d483792f68dc51dbc52722820a15795c4a1baad$ +// $hash=55d17a525f5a5a35a3b75f3636ddecb10bf37b99$ // #include "libcef_dll/cpptoc/frame_handler_cpptoc.h" @@ -49,7 +49,8 @@ frame_handler_on_frame_created(struct _cef_frame_handler_t* self, void CEF_CALLBACK frame_handler_on_frame_attached(struct _cef_frame_handler_t* self, cef_browser_t* browser, - cef_frame_t* frame) { + cef_frame_t* frame, + int reattached) { shutdown_checker::AssertNotShutdown(); // AUTO-GENERATED CONTENT - DELETE THIS COMMENT BEFORE MODIFYING @@ -68,7 +69,8 @@ frame_handler_on_frame_attached(struct _cef_frame_handler_t* self, // Execute CefFrameHandlerCppToC::Get(self)->OnFrameAttached( - CefBrowserCToCpp::Wrap(browser), CefFrameCToCpp::Wrap(frame)); + CefBrowserCToCpp::Wrap(browser), CefFrameCToCpp::Wrap(frame), + reattached ? true : false); } void CEF_CALLBACK diff --git a/libcef_dll/ctocpp/frame_handler_ctocpp.cc b/libcef_dll/ctocpp/frame_handler_ctocpp.cc index 01cd16ab8..496a44d6e 100644 --- a/libcef_dll/ctocpp/frame_handler_ctocpp.cc +++ b/libcef_dll/ctocpp/frame_handler_ctocpp.cc @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=1b752686dc1743bed6957503b1cd6999f9a2a4f1$ +// $hash=08e97b352e24a9d677d4f7f6e8c71d681bc41f76$ // #include "libcef_dll/ctocpp/frame_handler_ctocpp.h" @@ -46,7 +46,8 @@ void CefFrameHandlerCToCpp::OnFrameCreated(CefRefPtr browser, NO_SANITIZE("cfi-icall") void CefFrameHandlerCToCpp::OnFrameAttached(CefRefPtr browser, - CefRefPtr frame) { + CefRefPtr frame, + bool reattached) { shutdown_checker::AssertNotShutdown(); cef_frame_handler_t* _struct = GetStruct(); @@ -66,7 +67,7 @@ void CefFrameHandlerCToCpp::OnFrameAttached(CefRefPtr browser, // Execute _struct->on_frame_attached(_struct, CefBrowserCppToC::Wrap(browser), - CefFrameCppToC::Wrap(frame)); + CefFrameCppToC::Wrap(frame), reattached); } NO_SANITIZE("cfi-icall") diff --git a/libcef_dll/ctocpp/frame_handler_ctocpp.h b/libcef_dll/ctocpp/frame_handler_ctocpp.h index 6c78d4eaf..b02e28323 100644 --- a/libcef_dll/ctocpp/frame_handler_ctocpp.h +++ b/libcef_dll/ctocpp/frame_handler_ctocpp.h @@ -9,7 +9,7 @@ // implementations. See the translator.README.txt file in the tools directory // for more information. // -// $hash=d254c51cf1312313c462b034a42ff5ea95c41119$ +// $hash=e20bf114ab84481c3c1c1978b06c32b88f2aa136$ // #ifndef CEF_LIBCEF_DLL_CTOCPP_FRAME_HANDLER_CTOCPP_H_ @@ -37,7 +37,8 @@ class CefFrameHandlerCToCpp : public CefCToCppRefCounted browser, CefRefPtr frame) override; void OnFrameAttached(CefRefPtr browser, - CefRefPtr frame) override; + CefRefPtr frame, + bool reattached) override; void OnFrameDetached(CefRefPtr browser, CefRefPtr frame) override; void OnMainFrameChanged(CefRefPtr browser, diff --git a/tests/ceftests/draggable_regions_unittest.cc b/tests/ceftests/draggable_regions_unittest.cc index 806b1c410..5feff9fde 100644 --- a/tests/ceftests/draggable_regions_unittest.cc +++ b/tests/ceftests/draggable_regions_unittest.cc @@ -9,7 +9,6 @@ namespace { -const char kTestURLWithRegions[] = "http://test.com/regions"; const char kTestHTMLWithRegions[] = "" " " @@ -23,10 +22,8 @@ const char kTestHTMLWithRegions[] = " " ""; -const char kTestURLWithoutRegions[] = "http://test.com/no-regions"; const char kTestHTMLWithoutRegions[] = "Hello World!"; -const char kTestURLWithChangingRegions[] = "http://test.com/changing-regions"; const char kTestHTMLWithChangingRegions[] = "" " " @@ -48,39 +45,64 @@ const char kTestHTMLWithChangingRegions[] = " " ""; -class DraggableRegionsTestHandler : public TestHandler, public CefDragHandler { +class DraggableRegionsTestHandler : public TestHandler, + public CefDragHandler, + public CefFrameHandler { public: - DraggableRegionsTestHandler() : step_(kStepWithRegions) {} + // Test steps executed in order. + enum Step { + // Nav 1: Two regions (get notification). + kStepWithRegions = 1, + // Nav 2: Starts with the same region as Nav 1 (no notification), + // then a changed region (get notification). + kStepWithChangingRegions, + // Nav 3: No regions (get notification). + kStepWithoutRegions, + // GoBack: Two regions (get notification), then a changed region (get + // notification). Note the first notification is not sent if + // BackForwardCache is enabled. + kStepWithChangingRegions2, + kStepWithChangingRegions3, + // GoForward: No regions (get notification). + kStepWithoutRegions2, + + kStepMax = kStepWithoutRegions2, + }; + + explicit DraggableRegionsTestHandler(bool same_origin) + : same_origin_(same_origin) {} void RunTest() override { // Add HTML documents with and without draggable regions. - AddResource(kTestURLWithRegions, kTestHTMLWithRegions, "text/html"); - AddResource(kTestURLWithoutRegions, kTestHTMLWithoutRegions, "text/html"); - AddResource(kTestURLWithChangingRegions, kTestHTMLWithChangingRegions, + AddResource(GetURL(kStepWithRegions), kTestHTMLWithRegions, "text/html"); + AddResource(GetURL(kStepWithChangingRegions), kTestHTMLWithChangingRegions, + "text/html"); + AddResource(GetURL(kStepWithoutRegions), kTestHTMLWithoutRegions, "text/html"); // Create the browser - CreateBrowser(kTestURLWithRegions); + CreateBrowser(GetURL(kStepWithRegions)); // Time out the test after a reasonable period of time. SetTestTimeout(); } CefRefPtr GetDragHandler() override { return this; } + CefRefPtr GetFrameHandler() override { return this; } void OnDraggableRegionsChanged( CefRefPtr browser, CefRefPtr frame, const std::vector& regions) override { - EXPECT_TRUE(CefCurrentlyOn(TID_UI)); + EXPECT_UI_THREAD(); EXPECT_TRUE(browser->IsSame(GetBrowser())); EXPECT_TRUE(frame->IsMain()); - did_call_on_draggable_regions_changed_.yes(); + draggable_regions_changed_ct_++; switch (step_) { case kStepWithRegions: - case kStepWithChangingRegions1: + case kStepWithChangingRegions2: EXPECT_EQ(2U, regions.size()); EXPECT_NEAR(50, regions[0].bounds.x, 1); EXPECT_NEAR(50, regions[0].bounds.y, 1); @@ -93,7 +115,8 @@ class DraggableRegionsTestHandler : public TestHandler, public CefDragHandler { EXPECT_NEAR(50, regions[1].bounds.height, 1); EXPECT_EQ(0, regions[1].draggable); break; - case kStepWithChangingRegions2: + case kStepWithChangingRegions: + case kStepWithChangingRegions3: EXPECT_EQ(2U, regions.size()); EXPECT_EQ(0, regions[0].bounds.x); EXPECT_EQ(0, regions[0].bounds.y); @@ -107,16 +130,35 @@ class DraggableRegionsTestHandler : public TestHandler, public CefDragHandler { EXPECT_EQ(0, regions[1].draggable); break; case kStepWithoutRegions: - // Should not be reached. - EXPECT_TRUE(false); + case kStepWithoutRegions2: + EXPECT_TRUE(regions.empty()); break; } NextTest(browser); } + void OnFrameAttached(CefRefPtr browser, + CefRefPtr frame, + bool reattached) override { + EXPECT_UI_THREAD(); + EXPECT_TRUE(browser->IsSame(GetBrowser())); + EXPECT_TRUE(frame->IsMain()); + + if (reattached) { + // When BackForwardCache is enabled and we go back to + // kTestHTMLWithChangingRegions, draggable regions will already be in the + // final position because the page content is not reloaded. + if (step_ == kStepWithChangingRegions2) { + step_ = kStepWithChangingRegions3; + expected_draggable_regions_changed_ct_--; + } + } + } + void DestroyTest() override { - EXPECT_FALSE(did_call_on_draggable_regions_changed_); + EXPECT_EQ(expected_draggable_regions_changed_ct_, + draggable_regions_changed_ct_); TestHandler::DestroyTest(); } @@ -125,51 +167,79 @@ class DraggableRegionsTestHandler : public TestHandler, public CefDragHandler { void NextTest(CefRefPtr browser) { CefRefPtr frame(browser->GetMainFrame()); - did_call_on_draggable_regions_changed_.reset(); - switch (step_) { case kStepWithRegions: - step_ = kStepWithChangingRegions1; - frame->LoadURL(kTestURLWithChangingRegions); + step_ = kStepWithChangingRegions; + frame->LoadURL(GetURL(kStepWithChangingRegions)); break; - case kStepWithChangingRegions1: - step_ = kStepWithChangingRegions2; - break; - case kStepWithChangingRegions2: + case kStepWithChangingRegions: step_ = kStepWithoutRegions; - frame->LoadURL(kTestURLWithoutRegions); - // Needed because this test doesn't call OnDraggableRegionsChanged. - CefPostDelayedTask( - TID_UI, - base::BindOnce(&DraggableRegionsTestHandler::DestroyTest, this), - 500); + frame->LoadURL(GetURL(kStepWithoutRegions)); break; case kStepWithoutRegions: { - // Should not be reached. - EXPECT_TRUE(false); + step_ = kStepWithChangingRegions2; + browser->GoBack(); + break; + } + case kStepWithChangingRegions2: { + step_ = kStepWithChangingRegions3; + break; + } + case kStepWithChangingRegions3: { + step_ = kStepWithoutRegions2; + browser->GoForward(); + break; + } + case kStepWithoutRegions2: { + DestroyTest(); break; } } } - enum Step { - kStepWithRegions, - kStepWithChangingRegions1, - kStepWithChangingRegions2, - kStepWithoutRegions, - } step_; + std::string GetURL(Step step) const { + // When |same_origin_| is true every other URL gets a different origin. + switch (step) { + case kStepWithRegions: + return same_origin_ ? "http://test.com/regions" + : "http://test2.com/regions"; + case kStepWithChangingRegions: + case kStepWithChangingRegions2: + case kStepWithChangingRegions3: + return "http://test.com/changing-regions"; + case kStepWithoutRegions: + case kStepWithoutRegions2: + return same_origin_ ? "http://test.com/no-regions" + : "http://test2.com/no-regions"; + } - TrackCallback did_call_on_draggable_regions_changed_; + NOTREACHED(); + return ""; + } + + const bool same_origin_; + + Step step_ = kStepWithRegions; + int draggable_regions_changed_ct_ = 0; + int expected_draggable_regions_changed_ct_ = kStepMax; IMPLEMENT_REFCOUNTING(DraggableRegionsTestHandler); }; } // namespace -// Verify that draggable regions work. -TEST(DraggableRegionsTest, DraggableRegions) { +// Verify that draggable regions work in the same origin. +TEST(DraggableRegionsTest, DraggableRegionsSameOrigin) { CefRefPtr handler = - new DraggableRegionsTestHandler(); + new DraggableRegionsTestHandler(/*same_origin=*/true); + handler->ExecuteTest(); + ReleaseAndWaitForDestructor(handler); +} + +// Verify that draggable regions work cross-origin. +TEST(DraggableRegionsTest, DraggableRegionsCrossOrigin) { + CefRefPtr handler = + new DraggableRegionsTestHandler(/*same_origin=*/false); handler->ExecuteTest(); ReleaseAndWaitForDestructor(handler); } diff --git a/tests/ceftests/frame_handler_unittest.cc b/tests/ceftests/frame_handler_unittest.cc index 8cfa40b90..6b7518fac 100644 --- a/tests/ceftests/frame_handler_unittest.cc +++ b/tests/ceftests/frame_handler_unittest.cc @@ -607,7 +607,8 @@ class OrderMainTestHandler : public RoutingTestHandler, public CefFrameHandler { } void OnFrameAttached(CefRefPtr browser, - CefRefPtr frame) override { + CefRefPtr frame, + bool reattached) override { EXPECT_UI_THREAD(); // May arrive before or after OnMainFrameChanged switches the frame (after @@ -1080,7 +1081,8 @@ class OrderSubTestHandler : public NavigateOrderMainTestHandler { } void OnFrameAttached(CefRefPtr browser, - CefRefPtr frame) override { + CefRefPtr frame, + bool reattached) override { if (!frame->IsMain()) { auto map = GetFrameMap(frame); auto status = map->GetFrameStatus(frame); @@ -1088,7 +1090,7 @@ class OrderSubTestHandler : public NavigateOrderMainTestHandler { return; } - NavigateOrderMainTestHandler::OnFrameAttached(browser, frame); + NavigateOrderMainTestHandler::OnFrameAttached(browser, frame, reattached); } void OnFrameDetached(CefRefPtr browser, @@ -1507,14 +1509,15 @@ class PopupOrderMainTestHandler : public OrderMainTestHandler { } void OnFrameAttached(CefRefPtr browser, - CefRefPtr frame) override { + CefRefPtr frame, + bool reattached) override { if (temp_main_frame_ && temp_main_frame_->IsSame(frame)) { EXPECT_TRUE(cross_origin_); temp_main_frame_->OnFrameAttached(browser, frame); return; } - OrderMainTestHandler::OnFrameAttached(browser, frame); + OrderMainTestHandler::OnFrameAttached(browser, frame, reattached); } void OnMainFrameChanged(CefRefPtr browser,