From 199390b7e36a173fccbf48eeefa670b2b7083b16 Mon Sep 17 00:00:00 2001 From: Nate Chapin Date: Mon, 2 May 2022 20:19:07 +0000 Subject: [PATCH] Make NavigationApi resilient to memory purging destroying its ExecutionContext NavigationApi can be invoked and try to get a ScriptState or create ExecutionContextClients with a LocalDOMWindow that has been purged via LocalFrame::ForciblyPurgeV8Memory(). Ensure that NavigationApi is disabled after purge. (cherry picked from commit e52842ea74ca234ad76b4f7b606ede494a8785d5) Bug: 1316437, 1319341 Change-Id: Id621452da6fe1bb58f965c92c6afb5dafe5833ce Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3615532 Reviewed-by: Domenic Denicola Commit-Queue: Nate Chapin Cr-Original-Commit-Position: refs/heads/main@{#997490} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3621615 Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/branch-heads/5005@{#358} Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738} --- .../core/navigation_api/navigation_api.cc | 16 +++--- .../navigation_api/navigation_api_test.cc | 55 +++++++++++++++++++ 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/third_party/blink/renderer/core/navigation_api/navigation_api.cc b/third_party/blink/renderer/core/navigation_api/navigation_api.cc index c9d3276c172da0..154adc6742ca59 100644 --- a/third_party/blink/renderer/core/navigation_api/navigation_api.cc +++ b/third_party/blink/renderer/core/navigation_api/navigation_api.cc @@ -292,7 +292,7 @@ void NavigationApi::UpdateForNavigation(HistoryItem& item, // A same-document navigation (e.g., a document.open()) in a // |HasEntriesAndEventsDisabled()| situation will try to operate on an empty // |entries_|. The navigation API considers this a no-op. - if (entries_.IsEmpty()) + if (HasEntriesAndEventsDisabled()) return; NavigationHistoryEntry* old_current = currentEntry(); @@ -716,11 +716,11 @@ void NavigationApi::PromoteUpcomingNavigationToOngoing(const String& key) { bool NavigationApi::HasEntriesAndEventsDisabled() const { auto* frame = GetSupplementable()->GetFrame(); - return !frame || - !GetSupplementable() - ->GetFrame() - ->Loader() - .HasLoadedNonInitialEmptyDocument() || + // Disable for initial empty documents, opaque origins, or in detached + // windows. Also, in destroyed-but-not-detached windows due to memory purging + // (see https://crbug.com/1319341). + return !frame || GetSupplementable()->IsContextDestroyed() || + !frame->Loader().HasLoadedNonInitialEmptyDocument() || GetSupplementable()->GetSecurityOrigin()->IsOpaque(); } @@ -751,8 +751,6 @@ NavigationApi::DispatchResult NavigationApi::DispatchNavigateEvent( auto* script_state = ToScriptStateForMainWorld(GetSupplementable()->GetFrame()); - if (!script_state) - return DispatchResult::kContinue; ScriptState::Scope scope(script_state); if (params.frame_load_type == WebFrameLoadType::kBackForward && @@ -877,6 +875,8 @@ void NavigationApi::InformAboutCanceledNavigation( has_dropped_navigation_ = true; return; } + if (HasEntriesAndEventsDisabled()) + return; if (ongoing_navigate_event_) { auto* script_state = diff --git a/third_party/blink/renderer/core/navigation_api/navigation_api_test.cc b/third_party/blink/renderer/core/navigation_api/navigation_api_test.cc index 92be0c364caa23..016ed612a272f6 100644 --- a/third_party/blink/renderer/core/navigation_api/navigation_api_test.cc +++ b/third_party/blink/renderer/core/navigation_api/navigation_api_test.cc @@ -84,4 +84,59 @@ TEST_F(NavigationApiTest, BrowserInitiatedSameDocumentBackForwardUncancelable) { EXPECT_EQ(result, mojom::blink::CommitResult::Ok); } +TEST_F(NavigationApiTest, DispatchNavigateEventAfterPurgeMemory) { + url_test_helpers::RegisterMockedURLLoad( + url_test_helpers::ToKURL("https://example.com/foo.html"), + test::CoreTestDataPath("foo.html")); + + frame_test_helpers::WebViewHelper web_view_helper; + web_view_helper.InitializeAndLoad("https://example.com/foo.html"); + LocalFrame* frame = web_view_helper.LocalMainFrame()->GetFrame(); + frame->ForciblyPurgeV8Memory(); + + KURL dest_url = url_test_helpers::ToKURL("https://example.com/foo.html#frag"); + // Should not crash. + NavigationApi::navigation(*frame->DomWindow()) + ->DispatchNavigateEvent(NavigationApi::DispatchParams( + dest_url, NavigateEventType::kFragment, WebFrameLoadType::kStandard)); +} + +TEST_F(NavigationApiTest, UpdateForNavigationAfterPurgeMemory) { + url_test_helpers::RegisterMockedURLLoad( + url_test_helpers::ToKURL("https://example.com/foo.html"), + test::CoreTestDataPath("foo.html")); + + frame_test_helpers::WebViewHelper web_view_helper; + web_view_helper.InitializeAndLoad("https://example.com/foo.html"); + LocalFrame* frame = web_view_helper.LocalMainFrame()->GetFrame(); + frame->ForciblyPurgeV8Memory(); + + HistoryItem* item = frame->Loader().GetDocumentLoader()->GetHistoryItem(); + // Should not crash. + NavigationApi::navigation(*frame->DomWindow()) + ->UpdateForNavigation(*item, WebFrameLoadType::kStandard); +} + +TEST_F(NavigationApiTest, InformAboutCanceledNavigationAfterPurgeMemory) { + url_test_helpers::RegisterMockedURLLoad( + url_test_helpers::ToKURL("https://example.com/foo.html"), + test::CoreTestDataPath("foo.html")); + + frame_test_helpers::WebViewHelper web_view_helper; + web_view_helper.InitializeAndLoad("https://example.com/foo.html"); + + LocalFrame* frame = web_view_helper.LocalMainFrame()->GetFrame(); + KURL dest_url = url_test_helpers::ToKURL("https://example.com/foo.html#frag"); + // DispatchNavigateEvent() will ensure NavigationApi::ongoing_navigate_event_ + // is non-null. + NavigationApi::navigation(*frame->DomWindow()) + ->DispatchNavigateEvent(NavigationApi::DispatchParams( + dest_url, NavigateEventType::kFragment, WebFrameLoadType::kStandard)); + // Purging memory will invalidate the v8::Context then call + // FrameLoader::StopAllLoaders(), which will in turn call + // NavigationApi::InformAboutCanceledNavigation. InformAboutCanceledNavigation + // shouldn't crash due to the invalid v8::Context. + frame->ForciblyPurgeV8Memory(); +} + } // namespace blink