Skip to content

Commit

Permalink
Make NavigationApi resilient to memory purging destroying its Executi…
Browse files Browse the repository at this point in the history
…onContext

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 e52842e)

Bug: 1316437, 1319341
Change-Id: Id621452da6fe1bb58f965c92c6afb5dafe5833ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3615532
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#997490}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3621615
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5005@{#358}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
  • Loading branch information
natechapin authored and Chromium LUCI CQ committed May 2, 2022
1 parent fc92165 commit 199390b
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 8 deletions.
16 changes: 8 additions & 8 deletions third_party/blink/renderer/core/navigation_api/navigation_api.cc
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -877,6 +875,8 @@ void NavigationApi::InformAboutCanceledNavigation(
has_dropped_navigation_ = true;
return;
}
if (HasEntriesAndEventsDisabled())
return;

if (ongoing_navigate_event_) {
auto* script_state =
Expand Down
Expand Up @@ -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

0 comments on commit 199390b

Please sign in to comment.