Fix allow_os_execution=true (fixes #2715, fixes #3851, fixes #3889)

When setting allow_os_execution=true in OnProtocolExecution the
confirmation dialog should display consistently, the load should
be canceled with ERR_ABORTED, and no interstitial error page
should be displayed.

libcef/browser/chrome/chrome_content_browser_client_cef.cc

@@ -132,7 +132,7 @@ void HandleExternalProtocolHelper(
     ChromeContentBrowserClientCef* self,
     content::WebContents::Getter web_contents_getter,
     content::FrameTreeNodeId frame_tree_node_id,
-    content::NavigationUIData* navigation_data,
+    std::unique_ptr<content::NavigationUIData> navigation_data,
     bool is_primary_main_frame,
     bool is_in_fenced_frame_tree,
     network::mojom::WebSandboxFlags sandbox_flags,
@@ -153,7 +153,7 @@ void HandleExternalProtocolHelper(
   // NavigationURLLoaderImpl::PrepareForNonInterceptedRequest.
   self->HandleExternalProtocol(
       resource_request.url, web_contents_getter, frame_tree_node_id,
-      navigation_data, is_primary_main_frame, is_in_fenced_frame_tree,
+      navigation_data.get(), is_primary_main_frame, is_in_fenced_frame_tree,
       sandbox_flags,
       static_cast<ui::PageTransition>(resource_request.transition_type),
       resource_request.has_user_gesture, initiating_origin, initiator_rfh,
@@ -544,10 +544,10 @@ bool ChromeContentBrowserClientCef::HandleExternalProtocol(
       web_contents_getter, frame_tree_node_id, request,
       base::BindRepeating(HandleExternalProtocolHelper, base::Unretained(this),
                           web_contents_getter, frame_tree_node_id,
-                          navigation_data, is_primary_main_frame,
-                          is_in_fenced_frame_tree, sandbox_flags, request,
-                          initiating_origin, std::move(weak_initiator_document),
-                          isolation_info));
+                          base::Passed(navigation_data->Clone()),
+                          is_primary_main_frame, is_in_fenced_frame_tree,
+                          sandbox_flags, request, initiating_origin,
+                          std::move(weak_initiator_document), isolation_info));
 
   net_service::ProxyURLLoaderFactory::CreateProxy(
       web_contents_getter, std::move(receiver), std::move(request_handler));

libcef/browser/net_service/proxy_url_loader_factory.cc

@@ -341,6 +341,7 @@ class InterceptedRequest : public network::mojom::URLLoader,
 
   network::URLLoaderCompletionStatus status_;
   bool got_loader_error_ = false;
+  bool completed_ = false;
 
   // Used for rate limiting OnUploadProgress callbacks.
   bool waiting_for_upload_progress_ack_ = false;
@@ -1141,7 +1142,17 @@ void InterceptedRequest::OnDestroy() {
   // We don't want any callbacks after this point.
   weak_factory_.InvalidateWeakPtrs();
 
-  factory_->request_handler_->OnRequestComplete(id_, request_, status_);
+  bool handled_externally = false;
+  factory_->request_handler_->OnRequestComplete(id_, request_, status_,
+                                                handled_externally);
+
+  // Don't call OnComplete() if an unhandled request might be handled
+  // externally. The request will instead be canceled implicitly with
+  // ERR_ABORTED.
+  if (!handled_externally && target_client_ && !completed_) {
+    target_client_->OnComplete(status_);
+    completed_ = true;
+  }
 
   // Destroys |this|.
   factory_->RemoveRequest(this);
@@ -1193,6 +1204,7 @@ void InterceptedRequest::CallOnComplete(
 
   if (target_client_) {
     target_client_->OnComplete(status);
+    completed_ = true;
   }
 
   if (proxied_loader_receiver_.is_bound() &&
@@ -1227,7 +1239,6 @@ void InterceptedRequest::SendErrorStatusAndCompleteImmediately(
     const network::URLLoaderCompletionStatus& status) {
   status_ = status;
   SendErrorCallback(status_.error_code, false);
-  target_client_->OnComplete(status_);
   OnDestroy();
 }
 

libcef/browser/net_service/proxy_url_loader_factory.h

@@ -121,7 +121,8 @@ class InterceptedRequestHandler {
   virtual void OnRequestComplete(
       int32_t request_id,
       const network::ResourceRequest& request,
-      const network::URLLoaderCompletionStatus& status) {}
+      const network::URLLoaderCompletionStatus& status,
+      bool& handled_externally) {}
 
   // Called on error.
   virtual void OnRequestError(int32_t request_id,

libcef/browser/net_service/resource_request_handler_wrapper.cc

@@ -1069,10 +1069,10 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler {
     }
   }
 
-  void OnRequestComplete(
-      int32_t request_id,
-      const network::ResourceRequest& request,
-      const network::URLLoaderCompletionStatus& status) override {
+  void OnRequestComplete(int32_t request_id,
+                         const network::ResourceRequest& request,
+                         const network::URLLoaderCompletionStatus& status,
+                         bool& handled_externally) override {
     CEF_REQUIRE_IOT();
 
     RequestState* state = GetState(request_id);
@@ -1116,6 +1116,7 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler {
             init_state_->browser_, init_state_->frame_,
             state->pending_request_.get(), allow_os_execution);
         if (allow_os_execution && init_state_->unhandled_request_callback_) {
+          handled_externally = true;
           CEF_POST_TASK(TID_UI, init_state_->unhandled_request_callback_);
         }
       }

tests/cefclient/browser/client_handler.cc

@@ -1074,28 +1074,6 @@ void ClientHandler::OnLoadingStateChange(CefRefPtr<CefBrowser> browser,
   NotifyLoadingState(isLoading, canGoBack, canGoForward);
 }
 
-void ClientHandler::OnLoadError(CefRefPtr<CefBrowser> browser,
-                                CefRefPtr<CefFrame> frame,
-                                ErrorCode errorCode,
-                                const CefString& errorText,
-                                const CefString& failedUrl) {
-  CEF_REQUIRE_UI_THREAD();
-
-  // Don't display an error for downloaded files.
-  if (errorCode == ERR_ABORTED) {
-    return;
-  }
-
-  // Don't display an error for external protocols that we allow the OS to
-  // handle. See OnProtocolExecution().
-  if (errorCode == ERR_UNKNOWN_URL_SCHEME) {
-    std::string urlStr = frame->GetURL();
-    if (urlStr.find("spotify:") == 0) {
-      return;
-    }
-  }
-}
-
 bool ClientHandler::OnRequestMediaAccessPermission(
     CefRefPtr<CefBrowser> browser,
     CefRefPtr<CefFrame> frame,

tests/cefclient/browser/client_handler.h

@@ -240,11 +240,6 @@ class ClientHandler : public BaseClientHandler,
                             bool isLoading,
                             bool canGoBack,
                             bool canGoForward) override;
-  void OnLoadError(CefRefPtr<CefBrowser> browser,
-                   CefRefPtr<CefFrame> frame,
-                   ErrorCode errorCode,
-                   const CefString& errorText,
-                   const CefString& failedUrl) override;
 
   // CefPermissionHandler methods
   bool OnRequestMediaAccessPermission(