From de60acbfdeb98531c6057f714adc931c44e4c887 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Thu, 14 Apr 2016 06:59:14 +0530 Subject: [PATCH] Fix race initialising DevtoolsNetworkController --- brightray/browser/browser_context.cc | 8 +-- brightray/browser/browser_context.h | 11 +++- .../net/devtools_network_controller_handle.cc | 60 +++++++++++++++++++ .../net/devtools_network_controller_handle.h | 44 ++++++++++++++ .../net/devtools_network_interceptor.cc | 8 +-- .../net/devtools_network_protocol_handler.cc | 14 +---- .../net/devtools_network_transaction.cc | 2 +- .../browser/url_request_context_getter.cc | 10 ++-- .../browser/url_request_context_getter.h | 6 +- brightray/filenames.gypi | 2 + 10 files changed, 130 insertions(+), 35 deletions(-) create mode 100644 brightray/browser/net/devtools_network_controller_handle.cc create mode 100644 brightray/browser/net/devtools_network_controller_handle.h diff --git a/brightray/browser/browser_context.cc b/brightray/browser/browser_context.cc index 6181f8ec632..11214bd38ab 100644 --- a/brightray/browser/browser_context.cc +++ b/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; } diff --git a/brightray/browser/browser_context.h b/brightray/browser/browser_context.h index 9866732cbcb..5ec2b2bcede 100644 --- a/brightray/browser/browser_context.h +++ b/brightray/browser/browser_context.h @@ -7,7 +7,7 @@ #include -#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, 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, base::FilePath path_; bool in_memory_; + + DevToolsNetworkControllerHandle network_controller_handle_; + scoped_ptr resource_context_; - scoped_ptr controller_; scoped_refptr url_request_getter_; scoped_refptr storage_policy_; scoped_ptr prefs_; diff --git a/brightray/browser/net/devtools_network_controller_handle.cc b/brightray/browser/net/devtools_network_controller_handle.cc new file mode 100644 index 00000000000..efc53a23a17 --- /dev/null +++ b/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 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 conditions) { + DCHECK_CURRENTLY_ON(BrowserThread::IO); + + LazyInitialize(); + controller_->SetNetworkState(client_id, std::move(conditions)); +} + +} // namespace brightray diff --git a/brightray/browser/net/devtools_network_controller_handle.h b/brightray/browser/net/devtools_network_controller_handle.h new file mode 100644 index 00000000000..f009d273268 --- /dev/null +++ b/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 + +#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 conditions); + + // Called on the IO thread. + DevToolsNetworkController* GetController(); + + private: + void LazyInitialize(); + void SetNetworkStateOnIO(const std::string& client_id, + scoped_ptr conditions); + + scoped_ptr controller_; + + DISALLOW_COPY_AND_ASSIGN(DevToolsNetworkControllerHandle); +}; + +} // namespace brightray + +#endif // BROWSER_DEVTOOLS_NETWORK_CONTROLLER_HANDLE_H_ diff --git a/brightray/browser/net/devtools_network_interceptor.cc b/brightray/browser/net/devtools_network_interceptor.cc index 126077c5f72..f1f8c63ae55 100644 --- a/brightray/browser/net/devtools_network_interceptor.cc +++ b/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) { diff --git a/brightray/browser/net/devtools_network_protocol_handler.cc b/brightray/browser/net/devtools_network_protocol_handler.cc index 9febb2a677d..b5e723d06a6 100644 --- a/brightray/browser/net/devtools_network_protocol_handler.cc +++ b/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 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 conditions) { auto browser_context = static_cast(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 diff --git a/brightray/browser/net/devtools_network_transaction.cc b/brightray/browser/net/devtools_network_transaction.cc index 5e6bd59e866..48ed4232db8 100644 --- a/brightray/browser/net/devtools_network_transaction.cc +++ b/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(); } diff --git a/brightray/browser/url_request_context_getter.cc b/brightray/browser/url_request_context_getter.cc index dffc106ede5..ae0771974b0 100644 --- a/brightray/browser/url_request_context_getter.cc +++ b/brightray/browser/url_request_context_getter.cc @@ -6,7 +6,7 @@ #include -#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 { diff --git a/brightray/browser/url_request_context_getter.h b/brightray/browser/url_request_context_getter.h index aaf9855d2a5..3f06ba78ba2 100644 --- a/brightray/browser/url_request_context_getter.h +++ b/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_; diff --git a/brightray/filenames.gypi b/brightray/filenames.gypi index e65e87307c3..566c6972e29 100644 --- a/brightray/filenames.gypi +++ b/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',