Skip to content

Commit

Permalink
LibWeb: Merge NavigationParams' Empty and NullWithError into `Nul…
Browse files Browse the repository at this point in the history 
…lOrError`

To check whether a NavigationParams is null, we have to check whether
it's `Empty` or `NullWithError`. Instead, we can merge both of these
possible variants into an optional error. If `NullOrError` has no
value it's null, otherwise it contains an error message.
  • Loading branch information
@dlarocque
dlarocque committed Nov 14, 2024
1 parent ce56bc2 commit e2dcc9b
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 14 deletions.
20 changes: 10 additions & 10 deletions Libraries/LibWeb/HTML/Navigable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,7 @@ static WebIDL::ExceptionOr<Navigable::NavigationParamsVariant> create_navigation
// If the latter condition occurs, then abort fetchController, and return. Otherwise, proceed onward.
if (navigation_id.has_value() && (!navigable->ongoing_navigation().has<String>() || navigable->ongoing_navigation().get<String>() != *navigation_id)) {
fetch_controller->abort(realm, {});
return Empty {};
return Navigable::NullOrError {};
}

// 8. If request's body is null, then set entry's document state's resource to null.
Expand Down Expand Up @@ -1010,9 +1010,9 @@ static WebIDL::ExceptionOr<Navigable::NavigationParamsVariant> create_navigation
if (response_holder->response()->network_error_message().has_value() && !response_holder->response()->network_error_message().value().is_null())
return response_holder->response()->network_error_message().value();
else
return Empty {};
return Navigable::NullOrError {};
} else if (location_url.is_error() || (location_url.value().has_value() && Fetch::Infrastructure::is_fetch_scheme(location_url.value().value().scheme())))
return Empty {};
return Navigable::NullOrError {};

// 22. Assert: locationURL is null and response is not a network error.
VERIFY(!location_url.value().has_value());
Expand Down Expand Up @@ -1079,7 +1079,7 @@ WebIDL::ExceptionOr<void> Navigable::populate_session_history_entry_document(
// FIXME: 1. Assert: this is running in parallel.

// 2. Assert: if navigationParams is non-null, then navigationParams's response is non-null.
if (!navigation_params.has<Empty>() && !navigation_params.has<NullWithError>())
if (!navigation_params.has<NullOrError>())
VERIFY(navigation_params.has<JS::NonnullGCPtr<NavigationParams>>() && navigation_params.get<JS::NonnullGCPtr<NavigationParams>>()->response);

// 3. Let currentBrowsingContext be navigable's active browsing context.
Expand All @@ -1089,7 +1089,7 @@ WebIDL::ExceptionOr<void> Navigable::populate_session_history_entry_document(
auto document_resource = entry->document_state()->resource();

// 5. If navigationParams is null, then:
if (navigation_params.has<Empty>() || navigation_params.has<NullWithError>()) {
if (navigation_params.has<NullOrError>()) {
// 1. If documentResource is a string, then set navigationParams to the result
// of creating navigation params from a srcdoc resource given entry, navigable,
// targetSnapshotParams, navigationId, and navTimingType.
Expand Down Expand Up @@ -1160,9 +1160,9 @@ WebIDL::ExceptionOr<void> Navigable::populate_session_history_entry_document(
// - FIXME: the result of should navigation response to navigation request of type in target be blocked by Content Security Policy? given navigationParams's request, navigationParams's response, navigationParams's policy container's CSP list, cspNavigationType, and navigable is "Blocked";
// - FIXME: navigationParams's reserved environment is non-null and the result of checking a navigation response's adherence to its embedder policy given navigationParams's response, navigable, and navigationParams's policy container's embedder policy is false; or
// - FIXME: the result of checking a navigation response's adherence to `X-Frame-Options` given navigationParams's response, navigable, navigationParams's policy container's CSP list, and navigationParams's origin is false,
if (navigation_params.has<Empty>() || navigation_params.has<NullWithError>()) {
if (navigation_params.has<NullOrError>()) {
// 1. Set entry's document state's document to the result of creating a document for inline content that doesn't have a DOM, given navigable, null, and navTimingType. The inline content should indicate to the user the sort of error that occurred.
auto error_message = navigation_params.has<NullWithError>() ? navigation_params.get<NullWithError>() : "Unknown error"sv;
auto error_message = navigation_params.get<NullOrError>().value_or("Unknown error"sv);

auto error_html = load_error_page(entry->url(), error_message).release_value_but_fixme_should_propagate_errors();
entry->document_state()->set_document(create_document_for_inline_content(this, navigation_id, [error_html](auto& document) {
Expand All @@ -1178,7 +1178,7 @@ WebIDL::ExceptionOr<void> Navigable::populate_session_history_entry_document(
saveExtraDocumentState = false;

// 4. If navigationParams is not null, then:
if (!navigation_params.has<Empty>() && !navigation_params.has<NullWithError>()) {
if (!navigation_params.has<NullOrError>()) {
// FIXME: 1. Run the environment discarding steps for navigationParams's reserved environment.
// FIXME: 2. Invoke WebDriver BiDi navigation failed with currentBrowsingContext and a new WebDriver BiDi navigation status whose id is navigationId, status is "canceled", and url is navigationParams's response's URL.
}
Expand Down Expand Up @@ -1210,7 +1210,7 @@ WebIDL::ExceptionOr<void> Navigable::populate_session_history_entry_document(

// 3. If entry's document state's request referrer is "client", and navigationParams is a navigation params (i.e., neither null nor a non-fetch scheme navigation params), then:
if (entry->document_state()->request_referrer() == Fetch::Infrastructure::Request::Referrer::Client
&& (!navigation_params.has<Empty>() && !navigation_params.has<NullWithError>() && navigation_params.has<JS::NonnullGCPtr<NonFetchSchemeNavigationParams>>())) {
&& (!navigation_params.has<NullOrError>() && navigation_params.has<JS::NonnullGCPtr<NonFetchSchemeNavigationParams>>())) {
// 1. Assert: navigationParams's request is not null.
VERIFY(navigation_params.has<JS::NonnullGCPtr<NavigationParams>>() && navigation_params.get<JS::NonnullGCPtr<NavigationParams>>()->request);

Expand Down Expand Up @@ -1463,7 +1463,7 @@ WebIDL::ExceptionOr<void> Navigable::navigate(NavigateParams params)
history_entry->set_document_state(document_state);

// 7. Let navigationParams be null.
NavigationParamsVariant navigation_params = Empty {};
NavigationParamsVariant navigation_params = Navigable::NullOrError {};

// FIXME: 8. If response is non-null:
if (response) {
Expand Down
6 changes: 3 additions & 3 deletions Libraries/LibWeb/HTML/Navigable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ class Navigable : public JS::Cell {
public:
virtual ~Navigable() override;

using NullWithError = StringView;
using NavigationParamsVariant = Variant<Empty, NullWithError, JS::NonnullGCPtr<NavigationParams>, JS::NonnullGCPtr<NonFetchSchemeNavigationParams>>;
using NullOrError = Optional<StringView>;
using NavigationParamsVariant = Variant<NullOrError, JS::NonnullGCPtr<NavigationParams>, JS::NonnullGCPtr<NonFetchSchemeNavigationParams>>;

ErrorOr<void> initialize_navigable(JS::NonnullGCPtr<DocumentState> document_state, JS::GCPtr<Navigable> parent);

Expand Down Expand Up @@ -132,7 +132,7 @@ class Navigable : public JS::Cell {
SourceSnapshotParams const& source_snapshot_params,
TargetSnapshotParams const& target_snapshot_params,
Optional<String> navigation_id = {},
NavigationParamsVariant navigation_params = Empty {},
NavigationParamsVariant navigation_params = Navigable::NullOrError {},
CSPNavigationType csp_navigation_type = CSPNavigationType::Other,
bool allow_POST = false,
JS::GCPtr<JS::HeapFunction<void()>> completion_steps = {});
Expand Down
2 changes: 1 addition & 1 deletion Libraries/LibWeb/HTML/TraversableNavigable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ TraversableNavigable::HistoryStepResult TraversableNavigable::apply_the_history_
// targetSnapshotParams, with allowPOST set to allowPOST and completionSteps set to queue a global task on the navigation and traversal task source given
// navigable's active window to run afterDocumentPopulated.
Platform::EventLoopPlugin::the().deferred_invoke(JS::create_heap_function(this->heap(), [populated_target_entry, potentially_target_specific_source_snapshot_params, target_snapshot_params, this, allow_POST, navigable, after_document_populated = JS::create_heap_function(this->heap(), move(after_document_populated))] {
navigable->populate_session_history_entry_document(populated_target_entry, *potentially_target_specific_source_snapshot_params, target_snapshot_params, {}, Empty {}, CSPNavigationType::Other, allow_POST, JS::create_heap_function(this->heap(), [this, after_document_populated, populated_target_entry]() mutable {
navigable->populate_session_history_entry_document(populated_target_entry, *potentially_target_specific_source_snapshot_params, target_snapshot_params, {}, Navigable::NullOrError {}, CSPNavigationType::Other, allow_POST, JS::create_heap_function(this->heap(), [this, after_document_populated, populated_target_entry]() mutable {
VERIFY(active_window());
queue_global_task(Task::Source::NavigationAndTraversal, *active_window(), JS::create_heap_function(this->heap(), [after_document_populated, populated_target_entry]() mutable {
after_document_populated->function()(true, populated_target_entry);
Expand Down

0 comments on commit e2dcc9b

Please sign in to comment.