Merge pull request #212 from deepak1556/devtools_network_controller_patch

Fix race initialising DevtoolsNetworkController

brightray/browser/browser_context.cc

@@ -141,7 +141,7 @@ net::URLRequestContextGetter* BrowserContext::CreateRequestContext(
   DCHECK(!url_request_getter_.get());
   url_request_getter_ = new URLRequestContextGetter(
       this,
-      GetDevToolsNetworkController(),
+      network_controller_handle(),
       net_log,
       GetPath(),
       in_memory_,
@@ -153,12 +153,6 @@ net::URLRequestContextGetter* BrowserContext::CreateRequestContext(
   return url_request_getter_.get();
 }
 
-DevToolsNetworkController* BrowserContext::GetDevToolsNetworkController() {
-  if (!controller_)
-    controller_.reset(new DevToolsNetworkController);
-  return controller_.get();
-}
-
 net::NetworkDelegate* BrowserContext::CreateNetworkDelegate() {
   return new NetworkDelegate;
 }

brightray/browser/browser_context.h

@@ -7,7 +7,7 @@
 
 #include <map>
 
-#include "browser/net/devtools_network_controller.h"
+#include "browser/net/devtools_network_controller_handle.h"
 #include "browser/permission_manager.h"
 #include "browser/url_request_context_getter.h"
 
@@ -63,12 +63,15 @@ class BrowserContext : public base::RefCounted<BrowserContext>,
       NetLog* net_log,
       content::ProtocolHandlerMap* protocol_handlers,
       content::URLRequestInterceptorScopedVector protocol_interceptors);
-  DevToolsNetworkController* GetDevToolsNetworkController();
 
   net::URLRequestContextGetter* url_request_context_getter() const {
     return url_request_getter_.get();
   }
 
+  DevToolsNetworkControllerHandle* network_controller_handle() {
+    return &network_controller_handle_;
+  }
+
   void InitPrefs();
   PrefService* prefs() { return prefs_.get(); }
 
@@ -114,8 +117,10 @@ class BrowserContext : public base::RefCounted<BrowserContext>,
 
   base::FilePath path_;
   bool in_memory_;
+
+  DevToolsNetworkControllerHandle network_controller_handle_;
+
   scoped_ptr<ResourceContext> resource_context_;
-  scoped_ptr<DevToolsNetworkController> controller_;
   scoped_refptr<URLRequestContextGetter> url_request_getter_;
   scoped_refptr<storage::SpecialStoragePolicy> storage_policy_;
   scoped_ptr<PrefService> prefs_;

brightray/browser/net/devtools_network_controller_handle.cc

@@ -0,0 +1,60 @@
+// Copyright (c) 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE-CHROMIUM file.
+
+#include "browser/net/devtools_network_controller_handle.h"
+
+#include "base/bind.h"
+#include "browser/net/devtools_network_conditions.h"
+#include "browser/net/devtools_network_controller.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
+
+namespace brightray {
+
+DevToolsNetworkControllerHandle::DevToolsNetworkControllerHandle() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+}
+
+DevToolsNetworkControllerHandle::~DevToolsNetworkControllerHandle() {
+  BrowserThread::DeleteSoon(BrowserThread::IO,
+                            FROM_HERE,
+                            controller_.release());
+}
+
+void DevToolsNetworkControllerHandle::SetNetworkState(
+    const std::string& client_id,
+    scoped_ptr<DevToolsNetworkConditions> conditions) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&DevToolsNetworkControllerHandle::SetNetworkStateOnIO,
+                 base::Unretained(this), client_id, base::Passed(&conditions)));
+}
+
+DevToolsNetworkController* DevToolsNetworkControllerHandle::GetController() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  LazyInitialize();
+  return controller_.get();
+}
+
+void DevToolsNetworkControllerHandle::LazyInitialize() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (!controller_)
+    controller_.reset(new DevToolsNetworkController);
+}
+
+void DevToolsNetworkControllerHandle::SetNetworkStateOnIO(
+    const std::string& client_id,
+    scoped_ptr<DevToolsNetworkConditions> conditions) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  LazyInitialize();
+  controller_->SetNetworkState(client_id, std::move(conditions));
+}
+
+}  // namespace brightray

brightray/browser/net/devtools_network_controller_handle.h

@@ -0,0 +1,44 @@
+// Copyright (c) 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE-CHROMIUM file.
+
+#ifndef BROWSER_DEVTOOLS_NETWORK_CONTROLLER_HANDLE_H_
+#define BROWSER_DEVTOOLS_NETWORK_CONTROLLER_HANDLE_H_
+
+#include <string>
+
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+
+namespace brightray {
+
+class DevToolsNetworkConditions;
+class DevToolsNetworkController;
+
+// A handle to manage an IO-thread DevToolsNetworkController on the IO thread
+// while allowing SetNetworkState to be called from the UI thread.
+class DevToolsNetworkControllerHandle {
+ public:
+  DevToolsNetworkControllerHandle();
+  ~DevToolsNetworkControllerHandle();
+
+  // Called on the UI thread.
+  void SetNetworkState(const std::string& client_id,
+                       scoped_ptr<DevToolsNetworkConditions> conditions);
+
+  // Called on the IO thread.
+  DevToolsNetworkController* GetController();
+
+ private:
+  void LazyInitialize();
+  void SetNetworkStateOnIO(const std::string& client_id,
+                           scoped_ptr<DevToolsNetworkConditions> conditions);
+
+  scoped_ptr<DevToolsNetworkController> controller_;
+
+  DISALLOW_COPY_AND_ASSIGN(DevToolsNetworkControllerHandle);
+};
+
+}  // namespace brightray
+
+#endif  // BROWSER_DEVTOOLS_NETWORK_CONTROLLER_HANDLE_H_

brightray/browser/net/devtools_network_interceptor.cc

@@ -75,7 +75,7 @@ void DevToolsNetworkInterceptor::UpdateConditions(
   conditions_ = std::move(conditions);
 
   bool offline = conditions_->offline();
-  if (offline || conditions_->IsThrottling()) {
+  if (offline || !conditions_->IsThrottling()) {
     timer_.Stop();
     FinishRecords(&download_, offline);
     FinishRecords(&upload_, offline);
@@ -98,7 +98,7 @@ void DevToolsNetworkInterceptor::UpdateConditions(
   latency_length_ = base::TimeDelta();
   double latency = conditions_->latency();
   if (latency > 0)
-    latency_length_ = base::TimeDelta::FromMilliseconds(latency);
+    latency_length_ = base::TimeDelta::FromMillisecondsD(latency);
   ArmTimer(now);
 }
 
@@ -124,7 +124,7 @@ uint64_t DevToolsNetworkInterceptor::UpdateThrottledRecords(
     (*records)[i].bytes -=
         (ticks / length) * kPacketSize + (i < shift ? kPacketSize : 0);
   }
-  std::rotate(records->begin(), records->end() + shift, records->end());
+  std::rotate(records->begin(), records->begin() + shift, records->end());
   return new_tick;
 }
 
@@ -138,7 +138,7 @@ void DevToolsNetworkInterceptor::UpdateThrottled(base::TimeTicks now) {
 
 void DevToolsNetworkInterceptor::UpdateSuspended(base::TimeTicks now) {
   int64_t activation_baseline =
-      (now - latency_length_ - base::TimeTicks::Now()).InMicroseconds();
+      (now - latency_length_ - base::TimeTicks()).InMicroseconds();
   ThrottleRecords suspended;
   for (const ThrottleRecord& record : suspended_) {
     if (record.send_end <= activation_baseline) {

brightray/browser/net/devtools_network_protocol_handler.cc

@@ -78,13 +78,6 @@ CreateFailureResponse(int id, const std::string& param) {
   return response;
 }
 
-void UpdateNetworkStateInIO(brightray::DevToolsNetworkController* controller,
-                            const std::string& client_id,
-                            scoped_ptr<brightray::DevToolsNetworkConditions> conditions) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  controller->SetNetworkState(client_id, std::move(conditions));
-}
-
 }  // namespace
 
 DevToolsNetworkProtocolHandler::DevToolsNetworkProtocolHandler() {
@@ -172,11 +165,8 @@ void DevToolsNetworkProtocolHandler::UpdateNetworkState(
     scoped_ptr<DevToolsNetworkConditions> conditions) {
   auto browser_context =
       static_cast<brightray::BrowserContext*>(agent_host->GetBrowserContext());
-  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
-                          base::Bind(&UpdateNetworkStateInIO,
-                                     browser_context->GetDevToolsNetworkController(),
-                                     agent_host->GetId(),
-                                     base::Passed(&conditions)));
+  browser_context->network_controller_handle()->SetNetworkState(
+      agent_host->GetId(), std::move(conditions));
 }
 
 }  // namespace brightray

brightray/browser/net/devtools_network_transaction.cc

@@ -290,7 +290,7 @@ void DevToolsNetworkTransaction::SetBeforeProxyHeadersSentCallback(
 }
 
 int DevToolsNetworkTransaction::ResumeNetworkStart() {
-  if (failed_)
+  if (CheckFailed())
     return net::ERR_INTERNET_DISCONNECTED;
   return transaction_->ResumeNetworkStart();
 }

brightray/browser/url_request_context_getter.cc

@@ -6,7 +6,7 @@
 
 #include <algorithm>
 
-#include "browser/net/devtools_network_controller.h"
+#include "browser/net/devtools_network_controller_handle.h"
 #include "browser/net/devtools_network_transaction_factory.h"
 #include "browser/net_log.h"
 #include "browser/network_delegate.h"
@@ -182,7 +182,7 @@ bool URLRequestContextGetter::Delegate::CanDelegateURLSecurity(const GURL& auth_
 
 URLRequestContextGetter::URLRequestContextGetter(
     Delegate* delegate,
-    DevToolsNetworkController* controller,
+    DevToolsNetworkControllerHandle* handle,
     NetLog* net_log,
     const base::FilePath& base_path,
     bool in_memory,
@@ -191,7 +191,7 @@ URLRequestContextGetter::URLRequestContextGetter(
     content::ProtocolHandlerMap* protocol_handlers,
     content::URLRequestInterceptorScopedVector protocol_interceptors)
     : delegate_(delegate),
-      controller_(controller),
+      network_controller_handle_(handle),
       net_log_(net_log),
       base_path_(base_path),
       in_memory_(in_memory),
@@ -370,11 +370,11 @@ net::URLRequestContext* URLRequestContextGetter::GetURLRequestContext() {
       backend.reset(delegate_->CreateHttpCacheBackendFactory(base_path_));
     }
 
-    if (controller_) {
+    if (network_controller_handle_) {
       storage_->set_http_transaction_factory(make_scoped_ptr(
           new net::HttpCache(
               make_scoped_ptr(new DevToolsNetworkTransactionFactory(
-                  controller_, http_network_session_.get())),
+                  network_controller_handle_->GetController(), http_network_session_.get())),
               std::move(backend),
               false)));
     } else {

brightray/browser/url_request_context_getter.h

@@ -28,7 +28,7 @@ class URLRequestJobFactory;
 
 namespace brightray {
 
-class DevToolsNetworkController;
+class DevToolsNetworkControllerHandle;
 class NetLog;
 
 class URLRequestContextGetter : public net::URLRequestContextGetter {
@@ -73,7 +73,7 @@ class URLRequestContextGetter : public net::URLRequestContextGetter {
 
   URLRequestContextGetter(
       Delegate* delegate,
-      DevToolsNetworkController* controller,
+      DevToolsNetworkControllerHandle* handle,
       NetLog* net_log,
       const base::FilePath& base_path,
       bool in_memory,
@@ -92,7 +92,7 @@ class URLRequestContextGetter : public net::URLRequestContextGetter {
  private:
   Delegate* delegate_;
 
-  DevToolsNetworkController* controller_;
+  DevToolsNetworkControllerHandle* network_controller_handle_;
   NetLog* net_log_;
   base::FilePath base_path_;
   bool in_memory_;

brightray/filenames.gypi

@@ -45,6 +45,8 @@
       'browser/net/devtools_network_conditions.h',
       'browser/net/devtools_network_controller.cc',
       'browser/net/devtools_network_controller.h',
+      'browser/net/devtools_network_controller_handle.cc',
+      'browser/net/devtools_network_controller_handle.h',
       'browser/net/devtools_network_interceptor.cc',
       'browser/net/devtools_network_interceptor.h',
       'browser/net/devtools_network_protocol_handler.cc',