Use DevToolsStatus enum to distinguish DevTools-handled redirects from request...

Use DevToolsStatus enum to distinguish DevTools-handled redirects from request failures. Fix some typos.

Change-Id: If32f061c76f44fa830ec701ffbf01d88963d1b9c
Reviewed-on: https://chromium-review.googlesource.com/959587
Commit-Queue: Hidy Han <hidyhan@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542912}

content/browser/devtools/devtools_url_interceptor_request_job.cc

@@ -448,6 +448,8 @@ int DevToolsURLInterceptorRequestJob::MockResponseDetails::ReadRawData(
 
 namespace {
 
+using DevToolsStatus = ResourceRequestInfoImpl::DevToolsStatus;
+
 void SendPendingBodyRequestsOnUiThread(
     std::vector<std::unique_ptr<
         protocol::Network::Backend::GetResponseBodyForInterceptionCallback>>
@@ -522,6 +524,14 @@ std::unique_ptr<net::UploadDataStream> GetUploadData(net::URLRequest* request) {
       std::move(proxy_readers), 0);
 }
 
+void SetDevToolsStatus(net::URLRequest* request,
+                       DevToolsStatus devtools_status) {
+  ResourceRequestInfoImpl* resource_request_info =
+      ResourceRequestInfoImpl::ForRequest(request);
+  DCHECK(resource_request_info);
+  resource_request_info->set_devtools_status(devtools_status);
+}
+
 }  // namespace
 
 DevToolsURLInterceptorRequestJob::DevToolsURLInterceptorRequestJob(
@@ -711,7 +721,7 @@ void DevToolsURLInterceptorRequestJob::OnSubRequestAuthRequired(
 
   if (stage_to_intercept_ == InterceptionStage::DONT_INTERCEPT) {
     // This should trigger default auth behavior.
-    // See comment in ProcessAuthRespose.
+    // See comment in ProcessAuthResponse.
     NotifyHeadersComplete();
     return;
   }
@@ -756,6 +766,8 @@ void DevToolsURLInterceptorRequestJob::OnSubRequestRedirectReceived(
     bool* defer_redirect) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(sub_request_);
+  SetDevToolsStatus(sub_request_->request(),
+                    DevToolsStatus::kCanceledAsRedirect);
 
   // If we're not intercepting results or are a response then cancel this
   // redirect and tell the parent request it was redirected through |redirect_|.
@@ -873,7 +885,7 @@ void DevToolsURLInterceptorRequestJob::StopIntercepting() {
     case WaitingForUserResponse::WAITING_FOR_RESPONSE_ACK:
     // Fallthough.
     case WaitingForUserResponse::WAITING_FOR_REQUEST_ACK:
-      ProcessInterceptionRespose(
+      ProcessInterceptionResponse(
           std::make_unique<DevToolsNetworkInterceptor::Modifications>(
               base::nullopt, base::nullopt, protocol::Maybe<std::string>(),
               protocol::Maybe<std::string>(), protocol::Maybe<std::string>(),
@@ -887,7 +899,7 @@ void DevToolsURLInterceptorRequestJob::StopIntercepting() {
               .SetResponse(protocol::Network::AuthChallengeResponse::
                                ResponseEnum::Default)
               .Build();
-      ProcessAuthRespose(
+      ProcessAuthResponse(
           std::make_unique<DevToolsNetworkInterceptor::Modifications>(
               base::nullopt, base::nullopt, protocol::Maybe<std::string>(),
               protocol::Maybe<std::string>(), protocol::Maybe<std::string>(),
@@ -928,7 +940,7 @@ void DevToolsURLInterceptorRequestJob::ContinueInterceptedRequest(
                                "authChallengeResponse not expected.")));
         break;
       }
-      ProcessInterceptionRespose(std::move(modifications));
+      ProcessInterceptionResponse(std::move(modifications));
       BrowserThread::PostTask(
           BrowserThread::UI, FROM_HERE,
           base::BindOnce(&ContinueInterceptedRequestCallback::sendSuccess,
@@ -945,7 +957,7 @@ void DevToolsURLInterceptorRequestJob::ContinueInterceptedRequest(
                                "authChallengeResponse required.")));
         break;
       }
-      if (ProcessAuthRespose(std::move(modifications))) {
+      if (ProcessAuthResponse(std::move(modifications))) {
         BrowserThread::PostTask(
             BrowserThread::UI, FROM_HERE,
             base::BindOnce(&ContinueInterceptedRequestCallback::sendSuccess,
@@ -1025,18 +1037,14 @@ DevToolsURLInterceptorRequestJob::BuildRequestInfo() {
   return result;
 }
 
-void DevToolsURLInterceptorRequestJob::ProcessInterceptionRespose(
+void DevToolsURLInterceptorRequestJob::ProcessInterceptionResponse(
     std::unique_ptr<DevToolsNetworkInterceptor::Modifications> modifications) {
   bool is_response_ack = waiting_for_user_response_ ==
                          WaitingForUserResponse::WAITING_FOR_RESPONSE_ACK;
   waiting_for_user_response_ = WaitingForUserResponse::NOT_WAITING;
 
-  if (modifications->mark_as_canceled) {
-    ResourceRequestInfoImpl* resource_request_info =
-        ResourceRequestInfoImpl::ForRequest(request());
-    DCHECK(resource_request_info);
-    resource_request_info->set_canceled_by_devtools(true);
-  }
+  if (modifications->mark_as_canceled)
+    SetDevToolsStatus(request(), DevToolsStatus::kCanceled);
 
   if (modifications->error_reason) {
     if (sub_request_) {
@@ -1122,7 +1130,7 @@ void DevToolsURLInterceptorRequestJob::ProcessInterceptionRespose(
   }
 }
 
-bool DevToolsURLInterceptorRequestJob::ProcessAuthRespose(
+bool DevToolsURLInterceptorRequestJob::ProcessAuthResponse(
     std::unique_ptr<DevToolsNetworkInterceptor::Modifications> modifications) {
   waiting_for_user_response_ = WaitingForUserResponse::NOT_WAITING;
 

content/browser/devtools/devtools_url_interceptor_request_job.h

@@ -128,10 +128,10 @@ class DevToolsURLInterceptorRequestJob : public net::URLRequestJob {
   const net::HttpResponseHeaders* GetHttpResponseHeaders() const;
 
   void ProcessRedirect(int status_code, const std::string& new_url);
-  void ProcessInterceptionRespose(
+  void ProcessInterceptionResponse(
       std::unique_ptr<DevToolsNetworkInterceptor::Modifications> modification);
 
-  bool ProcessAuthRespose(
+  bool ProcessAuthResponse(
       std::unique_ptr<DevToolsNetworkInterceptor::Modifications> modification);
 
   enum class WaitingForUserResponse {

content/browser/loader/resource_request_info_impl.cc

@@ -182,7 +182,7 @@ ResourceRequestInfoImpl::ResourceRequestInfoImpl(
       context_(context),
       report_raw_headers_(report_raw_headers),
       is_async_(is_async),
-      canceled_by_devtools_(false),
+      devtools_status_(DevToolsStatus::kNotCanceled),
       previews_state_(previews_state),
       body_(body),
       initiated_in_secure_context_(initiated_in_secure_context),
@@ -319,8 +319,9 @@ NavigationUIData* ResourceRequestInfoImpl::GetNavigationUIData() const {
   return navigation_ui_data_.get();
 }
 
-bool ResourceRequestInfoImpl::CanceledByDevTools() const {
-  return canceled_by_devtools_;
+ResourceRequestInfo::DevToolsStatus ResourceRequestInfoImpl::GetDevToolsStatus()
+    const {
+  return devtools_status_;
 }
 
 base::StringPiece ResourceRequestInfoImpl::GetCustomCancelReason() const {

content/browser/loader/resource_request_info_impl.h

@@ -103,7 +103,7 @@ class ResourceRequestInfoImpl : public ResourceRequestInfo,
   PreviewsState GetPreviewsState() const override;
   bool ShouldReportRawHeaders() const;
   NavigationUIData* GetNavigationUIData() const override;
-  bool CanceledByDevTools() const override;
+  DevToolsStatus GetDevToolsStatus() const override;
   base::StringPiece GetCustomCancelReason() const override;
 
   CONTENT_EXPORT void AssociateWithRequest(net::URLRequest* request);
@@ -202,8 +202,8 @@ class ResourceRequestInfoImpl : public ResourceRequestInfo,
     on_transfer_ = on_transfer;
   }
 
-  void set_canceled_by_devtools(bool canceled_by_devtools) {
-    canceled_by_devtools_ = canceled_by_devtools;
+  void set_devtools_status(DevToolsStatus devtools_status) {
+    devtools_status_ = devtools_status;
   }
 
   void SetBlobHandles(BlobHandles blob_handles);
@@ -262,7 +262,7 @@ class ResourceRequestInfoImpl : public ResourceRequestInfo,
   ResourceContext* context_;
   bool report_raw_headers_;
   bool is_async_;
-  bool canceled_by_devtools_;
+  DevToolsStatus devtools_status_;
   PreviewsState previews_state_;
   scoped_refptr<network::ResourceRequestBody> body_;
   bool initiated_in_secure_context_;

content/public/browser/resource_request_info.h

@@ -175,8 +175,16 @@ class ResourceRequestInfo {
   // UI thread at the beginning of navigation.
   virtual NavigationUIData* GetNavigationUIData() const = 0;
 
-  // Whether this request was canceled by DevTools.
-  virtual bool CanceledByDevTools() const = 0;
+  enum class DevToolsStatus {
+    kCanceled,
+    // DevTools can internally handle a redirect, so the url request may
+    // appear never done. Mark these cases.
+    kCanceledAsRedirect,
+    kNotCanceled,
+  };
+
+  // If and why this request was canceled by DevTools.
+  virtual DevToolsStatus GetDevToolsStatus() const = 0;
 
   // When the client of a request decides to cancel it, it may optionally
   // provide an application-defined description of the canncellation reason.

headless/lib/browser/headless_browser_context_impl.cc

@@ -396,10 +396,10 @@ void HeadlessBrowserContextImpl::NotifyChildContentsCreated(
 void HeadlessBrowserContextImpl::NotifyUrlRequestFailed(
     net::URLRequest* request,
     int net_error,
-    bool canceled_by_devtools) {
+    DevToolsStatus devtools_status) {
   base::AutoLock lock(observers_lock_);
   for (auto& observer : observers_)
-    observer.UrlRequestFailed(request, net_error, canceled_by_devtools);
+    observer.UrlRequestFailed(request, net_error, devtools_status);
 }
 
 void HeadlessBrowserContextImpl::SetNetworkConditions(

headless/lib/browser/headless_browser_context_impl.h

@@ -116,7 +116,7 @@ class HeadlessBrowserContextImpl : public HeadlessBrowserContext,
   // This will be called on the IO thread.
   void NotifyUrlRequestFailed(net::URLRequest* request,
                               int net_error,
-                              bool canceled_by_devtools);
+                              DevToolsStatus devtools_status);
 
   void SetNetworkConditions(HeadlessNetworkConditions conditions);
   HeadlessNetworkConditions GetNetworkConditions() override;

headless/lib/browser/headless_network_delegate.cc

@@ -87,10 +87,10 @@ void HeadlessNetworkDelegate::OnCompleted(net::URLRequest* request,
   if (!resource_request_info)
     return;
 
-  bool canceled_by_devtools = resource_request_info->CanceledByDevTools();
-  if (canceled_by_devtools || net_error != net::OK) {
+  DevToolsStatus devtools_status = resource_request_info->GetDevToolsStatus();
+  if (devtools_status != DevToolsStatus::kNotCanceled || net_error != net::OK) {
     headless_browser_context_->NotifyUrlRequestFailed(request, net_error,
-                                                      canceled_by_devtools);
+                                                      devtools_status);
   }
 }
 

headless/lib/headless_devtools_client_browsertest.cc

@@ -1469,10 +1469,10 @@ class UrlRequestFailedTest : public HeadlessAsyncDevTooledBrowserTest,
 
   void UrlRequestFailed(net::URLRequest* request,
                         int net_error,
-                        bool canceled_by_devtools) override {
+                        DevToolsStatus devtools_status) override {
     base::AutoLock lock(lock_);
     urls_that_failed_to_load_.push_back(request->url().ExtractFileName());
-    EXPECT_TRUE(canceled_by_devtools);
+    EXPECT_TRUE(devtools_status != DevToolsStatus::kNotCanceled);
   }
 
   virtual network::ErrorReason GetErrorReason() = 0;

headless/public/headless_browser_context.h

@@ -13,6 +13,7 @@
 
 #include "base/callback.h"
 #include "base/optional.h"
+#include "content/public/browser/resource_request_info.h"
 #include "content/public/common/web_preferences.h"
 #include "headless/lib/browser/headless_network_conditions.h"
 #include "headless/public/headless_export.h"
@@ -32,6 +33,8 @@ class HeadlessBrowserContextOptions;
 // Builder::SetOverrideWebPreferencesCallback().
 using content::WebPreferences;
 
+using DevToolsStatus = content::ResourceRequestInfo::DevToolsStatus;
+
 using ProtocolHandlerMap = std::unordered_map<
     std::string,
     std::unique_ptr<net::URLRequestJobFactory::ProtocolHandler>>;
@@ -91,7 +94,7 @@ class HEADLESS_EXPORT HeadlessBrowserContext::Observer {
   // delivered on the IO thread.
   virtual void UrlRequestFailed(net::URLRequest* request,
                                 int net_error,
-                                bool canceled_by_devtools) {}
+                                DevToolsStatus devtools_status) {}
 
   // Indicates the HeadlessBrowserContext is about to be deleted.
   virtual void OnHeadlessBrowserContextDestruct() {}

headless/test/headless_render_test.cc

@@ -161,8 +161,8 @@ void HeadlessRenderTest::OverrideWebPreferences(WebPreferences* preferences) {
 
 void HeadlessRenderTest::UrlRequestFailed(net::URLRequest* request,
                                           int net_error,
-                                          bool canceled_by_devtools) {
-  if (canceled_by_devtools)
+                                          DevToolsStatus devtools_status) {
+  if (devtools_status != DevToolsStatus::kNotCanceled)
     return;
   ADD_FAILURE() << "Network request failed: " << net_error << " for "
                 << request->url().spec();

headless/test/headless_render_test.h

@@ -90,7 +90,7 @@ class HeadlessRenderTest : public HeadlessAsyncDevTooledBrowserTest,
   // HeadlessBrowserContext::Observer
   void UrlRequestFailed(net::URLRequest* request,
                         int net_error,
-                        bool canceled_by_devtools) override;
+                        DevToolsStatus devtools_status) override;
 
   // page::ExperimentalObserver implementation:
   void OnLoadEventFired(const page::LoadEventFiredParams& params) override;