Skip to content

Commit

Permalink
Rename NavigationApiNavigation to NavigationApiMethodTracker
Browse files Browse the repository at this point in the history 
This follows the new names in the HTML Standard, which are clearer, instead of the ones in the original WICG spec, which were a bit confusing.

Fixed: 1424318
Change-Id: I7fbe1a68008e0ae0f7e59f9c49dbde27d9a11eee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4789148
Commit-Queue: Nate Chapin <japhet@chromium.org>
Auto-Submit: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1184880}
  • Loading branch information
@domenic
domenic authored and Chromium LUCI CQ committed Aug 17, 2023
1 parent 840e293 commit 23023aa
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 80 deletions.
4 changes: 2 additions & 2 deletions third_party/blink/renderer/core/navigation_api/build.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ blink_core_sources_navigation_api = [
"navigate_event.h",
"navigate_event_dispatch_params.cc",
"navigate_event_dispatch_params.h",
"navigation_api_navigation.cc",
"navigation_api_navigation.h",
"navigation_api_method_tracker.cc",
"navigation_api_method_tracker.h",
"navigation_api.cc",
"navigation_api.h",
"navigation_current_entry_change_event.cc",
Expand Down
117 changes: 62 additions & 55 deletions third_party/blink/renderer/core/navigation_api/navigation_api.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
#include "third_party/blink/renderer/core/loader/document_loader.h"
#include "third_party/blink/renderer/core/loader/frame_load_request.h"
#include "third_party/blink/renderer/core/navigation_api/navigate_event.h"
#include "third_party/blink/renderer/core/navigation_api/navigation_api_navigation.h"
#include "third_party/blink/renderer/core/navigation_api/navigation_api_method_tracker.h"
#include "third_party/blink/renderer/core/navigation_api/navigation_current_entry_change_event.h"
#include "third_party/blink/renderer/core/navigation_api/navigation_destination.h"
#include "third_party/blink/renderer/core/navigation_api/navigation_history_entry.h"
Expand All @@ -42,7 +42,6 @@
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
#include "third_party/blink/renderer/platform/bindings/to_v8.h"
#include "third_party/blink/renderer/platform/scheduler/public/thread_scheduler.h"
#include "third_party/blink/renderer/platform/wtf/uuid.h"

namespace blink {

Expand Down Expand Up @@ -212,8 +211,8 @@ void NavigationApi::UpdateForNavigation(HistoryItem& item,
// NotifyAboutTheCommittedToEntry() leads to the committed promise rejecting,
// even though we have already committed and the promise should definitely
// fulfill.
if (ongoing_navigation_) {
ongoing_navigation_->NotifyAboutTheCommittedToEntry(
if (ongoing_api_method_tracker_) {
ongoing_api_method_tracker_->NotifyAboutTheCommittedToEntry(
entries_[current_entry_index_], type);
}

Expand Down Expand Up @@ -496,30 +495,31 @@ NavigationResult* NavigationApi::PerformNonTraverseNavigation(
PerformSharedNavigationChecks(method_name_for_error_message))
return EarlyErrorResult(script_state, maybe_ex);

NavigationApiNavigation* navigation =
MakeGarbageCollected<NavigationApiNavigation>(
NavigationApiMethodTracker* api_method_tracker =
MakeGarbageCollected<NavigationApiMethodTracker>(
script_state, options, String(), std::move(serialized_state));
if (HasEntriesAndEventsDisabled()) {
// If `HasEntriesAndEventsDisabled()` is true, we still allow the
// navigation, but the navigate event won't fire and we won't do anything
// with the promises, so we need to detach the promise resolvers.
navigation->CleanupForWillNeverSettle();
api_method_tracker->CleanupForWillNeverSettle();
} else {
upcoming_non_traversal_navigation_ = navigation;
upcoming_non_traverse_api_method_tracker_ = api_method_tracker;
}

window_->GetFrame()->MaybeLogAdClickNavigation();
window_->GetFrame()->Navigate(request, frame_load_type);

// DispatchNavigateEvent() will clear upcoming_non_traversal_navigation_ if we
// get that far. If the navigation is blocked before DispatchNavigateEvent()
// is called, reject the promise and cleanup here.
if (upcoming_non_traversal_navigation_ == navigation) {
upcoming_non_traversal_navigation_ = nullptr;
// DispatchNavigateEvent() will clear
// upcoming_non_traverse_api_method_tracker_ if we get that far. If the
// navigation is blocked before DispatchNavigateEvent() is called, reject the
// promise and cleanup here.
if (upcoming_non_traverse_api_method_tracker_ == api_method_tracker) {
upcoming_non_traverse_api_method_tracker_ = nullptr;
return EarlyErrorResult(script_state, DOMExceptionCode::kAbortError,
"Navigation was aborted");
}
return navigation->GetNavigationResult();
return api_method_tracker->GetNavigationResult();
}

NavigationResult* NavigationApi::traverseTo(ScriptState* script_state,
Expand All @@ -538,13 +538,17 @@ NavigationResult* NavigationApi::traverseTo(ScriptState* script_state,
return EarlySuccessResult(script_state, currentEntry());
}

auto previous_navigation = upcoming_traversals_.find(key);
if (previous_navigation != upcoming_traversals_.end())
return previous_navigation->value->GetNavigationResult();
auto previous_api_method_tracker =
upcoming_traverse_api_method_trackers_.find(key);
if (previous_api_method_tracker !=
upcoming_traverse_api_method_trackers_.end()) {
return previous_api_method_tracker->value->GetNavigationResult();
}

NavigationApiNavigation* ongoing_navigation =
MakeGarbageCollected<NavigationApiNavigation>(script_state, options, key);
upcoming_traversals_.insert(key, ongoing_navigation);
NavigationApiMethodTracker* api_method_tracker =
MakeGarbageCollected<NavigationApiMethodTracker>(script_state, options,
key);
upcoming_traverse_api_method_trackers_.insert(key, api_method_tracker);
if (window_->GetFrame()->IsMainFrame()) {
SoftNavigationHeuristics* heuristics =
SoftNavigationHeuristics::From(*window_);
Expand All @@ -558,7 +562,7 @@ NavigationResult* NavigationApi::traverseTo(ScriptState* script_state,
window_->GetFrame()->GetLocalFrameHostRemote().NavigateToNavigationApiKey(
key, LocalFrame::HasTransientUserActivation(window_->GetFrame()),
task_id);
return ongoing_navigation->GetNavigationResult();
return api_method_tracker->GetNavigationResult();
}

bool NavigationApi::canGoBack() const {
Expand Down Expand Up @@ -618,16 +622,17 @@ scoped_refptr<SerializedScriptValue> NavigationApi::SerializeState(
}

void NavigationApi::PromoteUpcomingNavigationToOngoing(const String& key) {
CHECK(!ongoing_navigation_);
CHECK(!ongoing_api_method_tracker_);
if (!key.IsNull()) {
CHECK(!upcoming_non_traversal_navigation_);
auto iter = upcoming_traversals_.find(key);
if (iter != upcoming_traversals_.end()) {
ongoing_navigation_ = iter->value;
upcoming_traversals_.erase(iter);
CHECK(!upcoming_non_traverse_api_method_tracker_);
auto iter = upcoming_traverse_api_method_trackers_.find(key);
if (iter != upcoming_traverse_api_method_trackers_.end()) {
ongoing_api_method_tracker_ = iter->value;
upcoming_traverse_api_method_trackers_.erase(iter);
}
} else {
ongoing_navigation_ = upcoming_non_traversal_navigation_.Release();
ongoing_api_method_tracker_ =
upcoming_non_traverse_api_method_tracker_.Release();
}
}

Expand Down Expand Up @@ -661,13 +666,14 @@ NavigationApi::DispatchResult NavigationApi::DispatchNavigateEvent(
// * back()/forward()/traverseTo() immediately fail when
// `HasEntriesAndEventsDisabled()` is false, because current_entry_index_
// will be permanently -1.
// * navigate()/reload() will not set `upcoming_non_traversal_navigation_`
// when `HasEntriesAndEventsDisabled()` is false, so there's nothing to
// promote to `ongoing_navigation_`.
// * navigate()/reload() will not set
// `upcoming_non_traverse_api_method_tracker_` when
// `HasEntriesAndEventsDisabled()` is false, so there's nothing to promote
// to `ongoing_navigation_`.
// * non-NavigationApi navigations never create an upcoming navigation.
CHECK(!ongoing_navigation_);
CHECK(!upcoming_non_traversal_navigation_);
CHECK(upcoming_traversals_.empty());
CHECK(!ongoing_api_method_tracker_);
CHECK(!upcoming_non_traverse_api_method_tracker_);
CHECK(upcoming_traverse_api_method_trackers_.empty());
return DispatchResult::kContinue;
}

Expand Down Expand Up @@ -699,8 +705,8 @@ NavigationApi::DispatchResult NavigationApi::DispatchNavigateEvent(
SerializedScriptValue* destination_state = nullptr;
if (params->destination_item) {
destination_state = params->destination_item->GetNavigationApiState();
} else if (ongoing_navigation_) {
destination_state = ongoing_navigation_->GetSerializedState();
} else if (ongoing_api_method_tracker_) {
destination_state = ongoing_api_method_tracker_->GetSerializedState();
} else if (navigation_type == "reload") {
HistoryItem* current_item = window_->document()->Loader()->GetHistoryItem();
destination_state = current_item->GetNavigationApiState();
Expand Down Expand Up @@ -741,8 +747,8 @@ NavigationApi::DispatchResult NavigationApi::DispatchNavigateEvent(
if (params->form && params->form->Method() == FormSubmission::kPostMethod) {
init->setFormData(FormData::Create(params->form, ASSERT_NO_EXCEPTION));
}
if (ongoing_navigation_) {
init->setInfo(ongoing_navigation_->GetInfo());
if (ongoing_api_method_tracker_) {
init->setInfo(ongoing_api_method_tracker_->GetInfo());
}
auto* controller = AbortController::Create(script_state);
init->setSignal(controller->signal());
Expand Down Expand Up @@ -816,26 +822,27 @@ void NavigationApi::InformAboutCanceledNavigation(
}

// If this function is being called as part of frame detach, also cleanup any
// upcoming_traversals_.
if (!upcoming_traversals_.empty() && window_->GetFrame() &&
// upcoming_traverse_api_method_trackers_.
if (!upcoming_traverse_api_method_trackers_.empty() && window_->GetFrame() &&
!window_->GetFrame()->IsAttached()) {
HeapVector<Member<NavigationApiNavigation>> traversals;
CopyValuesToVector(upcoming_traversals_, traversals);
HeapVector<Member<NavigationApiMethodTracker>> traversals;
CopyValuesToVector(upcoming_traverse_api_method_trackers_, traversals);
for (auto& traversal : traversals) {
TraverseCancelled(
traversal->GetKey(),
mojom::blink::TraverseCancelledReason::kAbortedBeforeCommit);
}
CHECK(upcoming_traversals_.empty());
CHECK(upcoming_traverse_api_method_trackers_.empty());
}
}

void NavigationApi::TraverseCancelled(
const String& key,
mojom::blink::TraverseCancelledReason reason) {
auto traversal = upcoming_traversals_.find(key);
if (traversal == upcoming_traversals_.end())
auto traversal = upcoming_traverse_api_method_trackers_.find(key);
if (traversal == upcoming_traverse_api_method_trackers_.end()) {
return;
}

auto* script_state = ToScriptStateForMainWorld(window_->GetFrame());
ScriptState::Scope scope(script_state);
Expand All @@ -858,7 +865,7 @@ void NavigationApi::TraverseCancelled(
CHECK(exception);
traversal->value->RejectFinishedPromise(
ScriptValue::From(script_state, exception));
upcoming_traversals_.erase(traversal);
upcoming_traverse_api_method_trackers_.erase(traversal);
}

bool NavigationApi::HasNonDroppedOngoingNavigation() const {
Expand All @@ -879,9 +886,9 @@ void NavigationApi::DidFailOngoingNavigation(ScriptValue value) {
event->SetType(event_type_names::kNavigateerror);
DispatchEvent(*event);

if (ongoing_navigation_) {
ongoing_navigation_->RejectFinishedPromise(value);
ongoing_navigation_ = nullptr;
if (ongoing_api_method_tracker_) {
ongoing_api_method_tracker_->RejectFinishedPromise(value);
ongoing_api_method_tracker_ = nullptr;
}

if (transition_) {
Expand All @@ -893,9 +900,9 @@ void NavigationApi::DidFailOngoingNavigation(ScriptValue value) {
void NavigationApi::DidFinishOngoingNavigation() {
DispatchEvent(*Event::Create(event_type_names::kNavigatesuccess));

if (ongoing_navigation_) {
ongoing_navigation_->ResolveFinishedPromise();
ongoing_navigation_ = nullptr;
if (ongoing_api_method_tracker_) {
ongoing_api_method_tracker_->ResolveFinishedPromise();
ongoing_api_method_tracker_ = nullptr;
}

if (transition_) {
Expand Down Expand Up @@ -959,9 +966,9 @@ void NavigationApi::Trace(Visitor* visitor) const {
visitor->Trace(window_);
visitor->Trace(entries_);
visitor->Trace(transition_);
visitor->Trace(ongoing_navigation_);
visitor->Trace(upcoming_traversals_);
visitor->Trace(upcoming_non_traversal_navigation_);
visitor->Trace(ongoing_api_method_tracker_);
visitor->Trace(upcoming_traverse_api_method_trackers_);
visitor->Trace(upcoming_non_traverse_api_method_tracker_);
visitor->Trace(ongoing_navigate_event_);
}

Expand Down
9 changes: 5 additions & 4 deletions third_party/blink/renderer/core/navigation_api/navigation_api.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace blink {

class DOMException;
class HistoryItem;
class NavigationApiNavigation;
class NavigationApiMethodTracker;
class NavigationUpdateCurrentEntryOptions;
class NavigationHistoryEntry;
class NavigateEvent;
Expand Down Expand Up @@ -167,9 +167,10 @@ class CORE_EXPORT NavigationApi final : public EventTarget {

Member<NavigationTransition> transition_;

Member<NavigationApiNavigation> ongoing_navigation_;
HeapHashMap<String, Member<NavigationApiNavigation>> upcoming_traversals_;
Member<NavigationApiNavigation> upcoming_non_traversal_navigation_;
Member<NavigationApiMethodTracker> ongoing_api_method_tracker_;
HeapHashMap<String, Member<NavigationApiMethodTracker>>
upcoming_traverse_api_method_trackers_;
Member<NavigationApiMethodTracker> upcoming_non_traverse_api_method_tracker_;

Member<NavigateEvent> ongoing_navigate_event_;

Expand Down
18 changes: 10 additions & 8 deletions ...vigation_api/navigation_api_navigation.cc → ...tion_api/navigation_api_method_tracker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "third_party/blink/renderer/core/navigation_api/navigation_api_navigation.h"
#include "third_party/blink/renderer/core/navigation_api/navigation_api_method_tracker.h"

#include "base/check_op.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
Expand All @@ -14,7 +14,7 @@

namespace blink {

NavigationApiNavigation::NavigationApiNavigation(
NavigationApiMethodTracker::NavigationApiMethodTracker(
ScriptState* script_state,
NavigationOptions* options,
const String& key,
Expand Down Expand Up @@ -47,23 +47,25 @@ NavigationApiNavigation::NavigationApiNavigation(
finished_resolver_->Promise().MarkAsHandled();
}

void NavigationApiNavigation::NotifyAboutTheCommittedToEntry(
void NavigationApiMethodTracker::NotifyAboutTheCommittedToEntry(
NavigationHistoryEntry* entry,
WebFrameLoadType type) {
CHECK_EQ(committed_to_entry_, nullptr);
committed_to_entry_ = entry;

if (type != WebFrameLoadType::kBackForward)
if (type != WebFrameLoadType::kBackForward) {
committed_to_entry_->SetAndSaveState(std::move(serialized_state_));
}

committed_resolver_->Resolve(committed_to_entry_);
}

void NavigationApiNavigation::ResolveFinishedPromise() {
void NavigationApiMethodTracker::ResolveFinishedPromise() {
finished_resolver_->Resolve(committed_to_entry_);
}

void NavigationApiNavigation::RejectFinishedPromise(const ScriptValue& value) {
void NavigationApiMethodTracker::RejectFinishedPromise(
const ScriptValue& value) {
if (committed_resolver_) {
// We never hit NotifyAboutTheCommittedToEntry(), so we should reject that
// too.
Expand All @@ -74,14 +76,14 @@ void NavigationApiNavigation::RejectFinishedPromise(const ScriptValue& value) {
serialized_state_.reset();
}

void NavigationApiNavigation::CleanupForWillNeverSettle() {
void NavigationApiMethodTracker::CleanupForWillNeverSettle() {
CHECK_EQ(committed_to_entry_, nullptr);
committed_resolver_->Detach();
finished_resolver_->Detach();
serialized_state_.reset();
}

void NavigationApiNavigation::Trace(Visitor* visitor) const {
void NavigationApiMethodTracker::Trace(Visitor* visitor) const {
visitor->Trace(info_);
visitor->Trace(committed_to_entry_);
visitor->Trace(committed_resolver_);
Expand Down
19 changes: 10 additions & 9 deletions ...avigation_api/navigation_api_navigation.h → ...ation_api/navigation_api_method_tracker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_NAVIGATION_API_NAVIGATION_API_NAVIGATION_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_NAVIGATION_API_NAVIGATION_API_NAVIGATION_H_
#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_NAVIGATION_API_NAVIGATION_API_METHOD_TRACKER_H_
#define THIRD_PARTY_BLINK_RENDERER_CORE_NAVIGATION_API_NAVIGATION_API_METHOD_TRACKER_H_

#include "base/memory/scoped_refptr.h"
#include "third_party/blink/public/web/web_frame_load_type.h"
Expand All @@ -20,13 +20,14 @@ class ScriptState;
class ScriptPromiseResolver;
class SerializedScriptValue;

class NavigationApiNavigation final
: public GarbageCollected<NavigationApiNavigation> {
class NavigationApiMethodTracker final
: public GarbageCollected<NavigationApiMethodTracker> {
public:
NavigationApiNavigation(ScriptState*,
NavigationOptions*,
const String& key,
scoped_refptr<SerializedScriptValue> state = nullptr);
NavigationApiMethodTracker(
ScriptState*,
NavigationOptions*,
const String& key,
scoped_refptr<SerializedScriptValue> state = nullptr);

void NotifyAboutTheCommittedToEntry(NavigationHistoryEntry*,
WebFrameLoadType);
Expand Down Expand Up @@ -60,4 +61,4 @@ class NavigationApiNavigation final

} // namespace blink

#endif // THIRD_PARTY_BLINK_RENDERER_CORE_NAVIGATION_API_NAVIGATION_API_NAVIGATION_H_
#endif // THIRD_PARTY_BLINK_RENDERER_CORE_NAVIGATION_API_NAVIGATION_API_METHOD_TRACKER_H_

0 comments on commit 23023aa

Please sign in to comment.