From 8a4f4bff9b73a1aa1c44193dad7f4e6d13038d48 Mon Sep 17 00:00:00 2001
From: Marshall Greenblatt <magreenblatt@gmail.com>
Date: Fri, 26 Apr 2019 14:22:42 -0400
Subject: [PATCH] Fix NetworkService load hang with default request handling
 (see issue #2622).

This change fixes a load hang when no custom handlers (CefResourceRequestHandler
or registered scheme handler) are found for a request.

To test: Run `cefsimple --enable-network-service` and all requests load. Test
expectations are unchanged.
---
 .../resource_request_handler_wrapper.cc       | 25 ++++++++++++++++---
 tests/ceftests/cookie_unittest.cc             | 23 ++++++++++++++++-
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/libcef/browser/net_service/resource_request_handler_wrapper.cc b/libcef/browser/net_service/resource_request_handler_wrapper.cc
index 19399e811..1fd674810 100644
--- a/libcef/browser/net_service/resource_request_handler_wrapper.cc
+++ b/libcef/browser/net_service/resource_request_handler_wrapper.cc
@@ -221,6 +221,14 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler {
 
     CefRefPtr<CefSchemeHandlerFactory> scheme_factory =
         resource_context_->GetSchemeHandlerFactory(request->url);
+    if (scheme_factory && !requestPtr) {
+      requestPtr = MakeRequest(request, id.hash(), true);
+    }
+
+    // True if there's a possibility that the client might handle the request.
+    const bool maybe_intercept_request = handler || scheme_factory;
+    if (!maybe_intercept_request && requestPtr)
+      requestPtr = nullptr;
 
     // May have a handler and/or scheme factory.
     state->Reset(handler, scheme_factory, requestPtr, request_was_redirected);
@@ -230,9 +238,15 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler {
           GetBrowser(), frame_, requestPtr.get());
     }
 
-    auto exec_callback = base::BindOnce(
-        std::move(callback), handler || scheme_factory /* intercept_request */,
-        is_external_ ? true : intercept_only);
+    auto exec_callback =
+        base::BindOnce(std::move(callback), maybe_intercept_request,
+                       is_external_ ? true : intercept_only);
+
+    if (!maybe_intercept_request) {
+      // Cookies will be handled by the NetworkService.
+      std::move(exec_callback).Run();
+      return;
+    }
 
     MaybeLoadCookies(state, request, std::move(exec_callback));
   }
@@ -438,8 +452,11 @@ class InterceptedRequestHandlerWrapper : public InterceptedRequestHandler {
 
     RequestState* state = GetState(id);
     DCHECK(state);
-    if (!state->handler_)
+    if (!state->handler_) {
+      InterceptedRequestHandler::OnRequestResponse(
+          id, request, head, redirect_info, std::move(callback));
       return;
+    }
 
     DCHECK(state->pending_request_);
     DCHECK(state->pending_response_);
diff --git a/tests/ceftests/cookie_unittest.cc b/tests/ceftests/cookie_unittest.cc
index 97a495dff..ca4c129e9 100644
--- a/tests/ceftests/cookie_unittest.cc
+++ b/tests/ceftests/cookie_unittest.cc
@@ -1458,6 +1458,9 @@ class CookieAccessTestHandler : public RoutingTestHandler,
     BLOCK_WRITE = 1 << 1,
     BLOCK_READ_WRITE = BLOCK_READ | BLOCK_WRITE,
     ALLOW_NO_FILTER = 1 << 2,
+
+    // Can only be used in combination with the SERVER backend.
+    ALLOW_NO_HANDLER = 1 << 3,
   };
 
   enum TestBackend {
@@ -1501,7 +1504,7 @@ class CookieAccessTestHandler : public RoutingTestHandler,
 
     EXPECT_FALSE(got_cookie_manager_);
 
-    if (test_mode_ == ALLOW_NO_FILTER) {
+    if (test_mode_ == ALLOW_NO_FILTER || test_mode_ == ALLOW_NO_HANDLER) {
       EXPECT_EQ(0, can_save_cookie1_ct_);
       EXPECT_EQ(0, can_send_cookie2_ct_);
     } else {
@@ -1569,6 +1572,22 @@ class CookieAccessTestHandler : public RoutingTestHandler,
     return this;
   }
 
+  CefRefPtr<CefResourceRequestHandler> GetResourceRequestHandler(
+      CefRefPtr<CefBrowser> browser,
+      CefRefPtr<CefFrame> frame,
+      CefRefPtr<CefRequest> request,
+      bool is_navigation,
+      bool is_download,
+      const CefString& request_initiator,
+      bool& disable_default_handling) override {
+    if (test_mode_ == ALLOW_NO_HANDLER) {
+      DCHECK_EQ(SERVER, test_backend_);
+      return nullptr;
+    }
+
+    return this;
+  }
+
   CefRefPtr<CefResourceHandler> GetResourceHandler(
       CefRefPtr<CefBrowser> browser,
       CefRefPtr<CefFrame> frame,
@@ -1848,6 +1867,8 @@ class CookieAccessTestHandler : public RoutingTestHandler,
   ACCESS_TEST(name##BlockWrite, BLOCK_WRITE, backend_mode)        \
   ACCESS_TEST(name##BlockReadWrite, BLOCK_READ_WRITE, backend_mode)
 
+ACCESS_TEST(ServerAllowNoHandler, ALLOW_NO_HANDLER, SERVER)
+
 ACCESS_TEST_ALL_MODES(Server, SERVER)
 ACCESS_TEST_ALL_MODES(Scheme, SCHEME_HANDLER)
 ACCESS_TEST_ALL_MODES(Resource, RESOURCE_HANDLER)