Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibWeb: Merge NavigationParams' Empty and NullWithError into NullOrError #2330

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions Libraries/LibWeb/HTML/Navigable.cpp
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<GC::Ref<NavigationParams>>() && navigation_params.get<GC::Ref<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<GC::Ref<NonFetchSchemeNavigationParams>>())) {
&& (!navigation_params.has<NullOrError>() && navigation_params.has<GC::Ref<NonFetchSchemeNavigationParams>>())) {
// 1. Assert: navigationParams's request is not null.
VERIFY(navigation_params.has<GC::Ref<NavigationParams>>() && navigation_params.get<GC::Ref<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
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, GC::Ref<NavigationParams>, GC::Ref<NonFetchSchemeNavigationParams>>;
using NullOrError = Optional<StringView>;
using NavigationParamsVariant = Variant<NullOrError, GC::Ref<NavigationParams>, GC::Ref<NonFetchSchemeNavigationParams>>;

ErrorOr<void> initialize_navigable(GC::Ref<DocumentState> document_state, GC::Ptr<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,
GC::Ptr<GC::Function<void()>> completion_steps = {});
Expand Down
2 changes: 1 addition & 1 deletion Libraries/LibWeb/HTML/TraversableNavigable.cpp
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(GC::create_function(this->heap(), [populated_target_entry, potentially_target_specific_source_snapshot_params, target_snapshot_params, this, allow_POST, navigable, after_document_populated = GC::create_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, GC::create_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, GC::create_function(this->heap(), [this, after_document_populated, populated_target_entry]() mutable {
VERIFY(active_window());
queue_global_task(Task::Source::NavigationAndTraversal, *active_window(), GC::create_function(this->heap(), [after_document_populated, populated_target_entry]() mutable {
after_document_populated->function()(true, populated_target_entry);
Expand Down
Loading