LibWeb: Prefer GCPtr<T> over Optional<NonnullGCPtr<T>>

Author: https://github.com/shannonbooth
Commit: 70e2f51674
Pull-request: https://github.com/SerenityOS/serenity/pull/23877
Reviewed-by: https://github.com/kennethmyhra ✅

Userland/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp

@@ -204,7 +204,7 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<Infrastructure::FetchController>> fetch(JS:
 }
 
 // https://fetch.spec.whatwg.org/#concept-main-fetch
-WebIDL::ExceptionOr<Optional<JS::NonnullGCPtr<PendingResponse>>> main_fetch(JS::Realm& realm, Infrastructure::FetchParams const& fetch_params, Recursive recursive)
+WebIDL::ExceptionOr<JS::GCPtr<PendingResponse>> main_fetch(JS::Realm& realm, Infrastructure::FetchParams const& fetch_params, Recursive recursive)
 {
     dbgln_if(WEB_FETCH_DEBUG, "Fetch: Running 'main fetch' with: fetch_params @ {}", &fetch_params);
 
@@ -529,7 +529,7 @@ WebIDL::ExceptionOr<Optional<JS::NonnullGCPtr<PendingResponse>>> main_fetch(JS::
         });
     });
 
-    return Optional<JS::NonnullGCPtr<PendingResponse>> {};
+    return JS::GCPtr<PendingResponse> {};
 }
 
 // https://fetch.spec.whatwg.org/#fetch-finale
@@ -1013,7 +1013,7 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<PendingResponse>> http_fetch(JS::Realm& rea
             return;
         }
 
-        Optional<JS::NonnullGCPtr<PendingResponse>> inner_pending_response;
+        JS::GCPtr<PendingResponse> inner_pending_response;
 
         // 7. If actualResponse’s status is a redirect status, then:
         if (Infrastructure::is_redirect_status(actual_response->status())) {
@@ -1053,8 +1053,8 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<PendingResponse>> http_fetch(JS::Realm& rea
             }
         }
 
-        if (inner_pending_response.has_value()) {
-            inner_pending_response.value()->when_loaded([returned_pending_response](JS::NonnullGCPtr<Infrastructure::Response> response) {
+        if (inner_pending_response) {
+            inner_pending_response->when_loaded([returned_pending_response](JS::NonnullGCPtr<Infrastructure::Response> response) {
                 dbgln_if(WEB_FETCH_DEBUG, "Fetch: Running 'HTTP fetch' inner_pending_response load callback");
                 returned_pending_response->resolve(response);
             });
@@ -1069,7 +1069,7 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<PendingResponse>> http_fetch(JS::Realm& rea
 }
 
 // https://fetch.spec.whatwg.org/#concept-http-redirect-fetch
-WebIDL::ExceptionOr<Optional<JS::NonnullGCPtr<PendingResponse>>> http_redirect_fetch(JS::Realm& realm, Infrastructure::FetchParams const& fetch_params, Infrastructure::Response& response)
+WebIDL::ExceptionOr<JS::GCPtr<PendingResponse>> http_redirect_fetch(JS::Realm& realm, Infrastructure::FetchParams const& fetch_params, Infrastructure::Response& response)
 {
     dbgln_if(WEB_FETCH_DEBUG, "Fetch: Running 'HTTP-redirect fetch' with: fetch_params @ {}, response = {}", &fetch_params, &response);
 

Userland/Libraries/LibWeb/Fetch/Fetching/Fetching.h

@@ -30,11 +30,11 @@ ENUMERATE_BOOL_PARAMS
 #undef __ENUMERATE_BOOL_PARAM
 
 WebIDL::ExceptionOr<JS::NonnullGCPtr<Infrastructure::FetchController>> fetch(JS::Realm&, Infrastructure::Request&, Infrastructure::FetchAlgorithms const&, UseParallelQueue use_parallel_queue = UseParallelQueue::No);
-WebIDL::ExceptionOr<Optional<JS::NonnullGCPtr<PendingResponse>>> main_fetch(JS::Realm&, Infrastructure::FetchParams const&, Recursive recursive = Recursive::No);
+WebIDL::ExceptionOr<JS::GCPtr<PendingResponse>> main_fetch(JS::Realm&, Infrastructure::FetchParams const&, Recursive recursive = Recursive::No);
 WebIDL::ExceptionOr<void> fetch_response_handover(JS::Realm&, Infrastructure::FetchParams const&, Infrastructure::Response&);
 WebIDL::ExceptionOr<JS::NonnullGCPtr<PendingResponse>> scheme_fetch(JS::Realm&, Infrastructure::FetchParams const&);
 WebIDL::ExceptionOr<JS::NonnullGCPtr<PendingResponse>> http_fetch(JS::Realm&, Infrastructure::FetchParams const&, MakeCORSPreflight make_cors_preflight = MakeCORSPreflight::No);
-WebIDL::ExceptionOr<Optional<JS::NonnullGCPtr<PendingResponse>>> http_redirect_fetch(JS::Realm&, Infrastructure::FetchParams const&, Infrastructure::Response&);
+WebIDL::ExceptionOr<JS::GCPtr<PendingResponse>> http_redirect_fetch(JS::Realm&, Infrastructure::FetchParams const&, Infrastructure::Response&);
 WebIDL::ExceptionOr<JS::NonnullGCPtr<PendingResponse>> http_network_or_cache_fetch(JS::Realm&, Infrastructure::FetchParams const&, IsAuthenticationFetch is_authentication_fetch = IsAuthenticationFetch::No, IsNewConnectionFetch is_new_connection_fetch = IsNewConnectionFetch::No);
 WebIDL::ExceptionOr<JS::NonnullGCPtr<PendingResponse>> nonstandard_resource_loader_file_or_http_network_fetch(JS::Realm&, Infrastructure::FetchParams const&, IncludeCredentials include_credentials = IncludeCredentials::No, IsNewConnectionFetch is_new_connection_fetch = IsNewConnectionFetch::No);
 WebIDL::ExceptionOr<JS::NonnullGCPtr<PendingResponse>> cors_preflight_fetch(JS::Realm&, Infrastructure::Request&);

Userland/Libraries/LibWeb/HTML/Navigation.cpp

@@ -735,7 +735,7 @@ WebIDL::ExceptionOr<NavigationResult> Navigation::perform_a_navigation_api_trave
 }
 
 // https://html.spec.whatwg.org/multipage/nav-history-apis.html#abort-the-ongoing-navigation
-void Navigation::abort_the_ongoing_navigation(Optional<JS::NonnullGCPtr<WebIDL::DOMException>> error)
+void Navigation::abort_the_ongoing_navigation(JS::GCPtr<WebIDL::DOMException> error)
 {
     auto& realm = relevant_realm(*this);
 
@@ -754,17 +754,17 @@ void Navigation::abort_the_ongoing_navigation(Optional<JS::NonnullGCPtr<WebIDL::
     m_suppress_scroll_restoration_during_ongoing_navigation = false;
 
     // 5. If error was not given, then let error be a new "AbortError" DOMException created in navigation's relevant realm.
-    if (!error.has_value())
+    if (!error)
         error = WebIDL::AbortError::create(realm, "Navigation aborted"_fly_string);
 
-    VERIFY(error.has_value());
+    VERIFY(error);
 
     // 6. If event's dispatch flag is set, then set event's canceled flag to true.
     if (event->dispatched())
         event->set_cancelled(true);
 
     // 7. Signal abort on event's abort controller given error.
-    event->abort_controller()->abort(error.value());
+    event->abort_controller()->abort(error);
 
     // 8. Set navigation's ongoing navigate event to null.
     m_ongoing_navigate_event = nullptr;
@@ -773,7 +773,7 @@ void Navigation::abort_the_ongoing_navigation(Optional<JS::NonnullGCPtr<WebIDL::
     //   and message, filename, lineno, and colno initialized to appropriate values that can be extracted
     //   from error and the current JavaScript stack in the same underspecified way that the report the exception algorithm does.
     ErrorEventInit event_init = {};
-    event_init.error = error.value();
+    event_init.error = error;
     // FIXME: Extract information from the exception and the JS context in the wishy-washy way the spec says here.
     event_init.filename = String {};
     event_init.colno = 0;
@@ -784,12 +784,12 @@ void Navigation::abort_the_ongoing_navigation(Optional<JS::NonnullGCPtr<WebIDL::
 
     // 10. If navigation's ongoing API method tracker is non-null, then reject the finished promise for apiMethodTracker with error.
     if (m_ongoing_api_method_tracker != nullptr)
-        WebIDL::reject_promise(realm, m_ongoing_api_method_tracker->finished_promise, error.value());
+        WebIDL::reject_promise(realm, m_ongoing_api_method_tracker->finished_promise, error);
 
     // 11. If navigation's transition is not null, then:
     if (m_transition != nullptr) {
         // 1. Reject navigation's transition's finished promise with error.
-        m_transition->finished()->reject(error.value());
+        m_transition->finished()->reject(error);
 
         // 2. Set navigation's transition to null.
         m_transition = nullptr;

Userland/Libraries/LibWeb/HTML/Navigation.h

@@ -109,7 +109,7 @@ public:
     // Abstract Operations
     bool has_entries_and_events_disabled() const;
     i64 get_the_navigation_api_entry_index(SessionHistoryEntry const&) const;
-    void abort_the_ongoing_navigation(Optional<JS::NonnullGCPtr<WebIDL::DOMException>> error = {});
+    void abort_the_ongoing_navigation(JS::GCPtr<WebIDL::DOMException> error = {});
     bool fire_a_traverse_navigate_event(JS::NonnullGCPtr<SessionHistoryEntry> destination_she, UserNavigationInvolvement = UserNavigationInvolvement::None);
     bool fire_a_push_replace_reload_navigate_event(
         Bindings::NavigationType,

Userland/Libraries/LibWeb/HTML/NavigatorBeacon.cpp

@@ -42,7 +42,7 @@ WebIDL::ExceptionOr<bool> NavigatorBeaconMixin::send_beacon(String const& url, O
     auto cors_mode = Fetch::Infrastructure::Request::Mode::NoCORS;
 
     // 6. If data is not null:
-    Optional<JS::NonnullGCPtr<Fetch::Infrastructure::Body>> transmitted_data;
+    JS::GCPtr<Fetch::Infrastructure::Body> transmitted_data;
     if (data.has_value()) {
         // 6.1 Set transmittedData and contentType to the result of extracting data's byte stream with the keepalive flag set.
         auto body_with_type = TRY(Fetch::extract_body(realm, data.value(), true));
@@ -77,11 +77,11 @@ WebIDL::ExceptionOr<bool> NavigatorBeaconMixin::send_beacon(String const& url, O
     req->set_header_list(header_list);                         // header list: headerList
     req->set_origin(origin);                                   // origin: origin
     req->set_keepalive(true);                                  // keepalive: true
-    if (transmitted_data.has_value())
-        req->set_body(transmitted_data.value());                                         // body: transmittedData
-    req->set_mode(cors_mode);                                                            // mode: corsMode
-    req->set_credentials_mode(Fetch::Infrastructure::Request::CredentialsMode::Include); // credentials mode: include
-    req->set_initiator_type(Fetch::Infrastructure::Request::InitiatorType::Beacon);      // initiator type: "beacon"
+    if (transmitted_data)
+        req->set_body(JS::NonnullGCPtr<Fetch::Infrastructure::Body> { *transmitted_data }); // body: transmittedData
+    req->set_mode(cors_mode);                                                               // mode: corsMode
+    req->set_credentials_mode(Fetch::Infrastructure::Request::CredentialsMode::Include);    // credentials mode: include
+    req->set_initiator_type(Fetch::Infrastructure::Request::InitiatorType::Beacon);         // initiator type: "beacon"
 
     // 7.2 Fetch req.
     (void)Fetch::Fetching::fetch(realm, req, Fetch::Infrastructure::FetchAlgorithms::create(vm, {}));