Skip to content

Commit

Permalink
Replace base::Timer with behavior-specific timers in net/
Browse files Browse the repository at this point in the history
This CL replaces base::Timer with its subclasses.
As base::Timer changes its behavior subject to its construction time
flags, that makes hard to see the actual timer behavior, especially
it's unclear when a timer is injected from other components.
Also, that OnceCallback support of base::Timer is hard to implement on
the dynamically determined invocation pattern.

Bug: 850247
Change-Id: Iaca4daa947e64a96a25a852ed1a1db332a0d2e5d
Reviewed-on: https://chromium-review.googlesource.com/1125712
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573212}
  • Loading branch information
tzik authored and Commit Bot committed Jul 9, 2018
1 parent 4be85b3 commit 08d8d6e
Show file tree
Hide file tree
Showing 21 changed files with 51 additions and 50 deletions.
2 changes: 1 addition & 1 deletion chrome/renderer/net/net_error_helper_core.cc
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ NetErrorHelperCore::NetErrorHelperCore(Delegate* delegate,
can_show_network_diagnostics_dialog_(false),
auto_reload_enabled_(auto_reload_enabled),
auto_reload_visible_only_(auto_reload_visible_only),
auto_reload_timer_(new base::Timer(false, false)),
auto_reload_timer_(new base::OneShotTimer()),
auto_reload_paused_(false),
auto_reload_in_flight_(false),
uncommitted_load_started_(false),
Expand Down
4 changes: 2 additions & 2 deletions chrome/renderer/net/net_error_helper_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class NetErrorHelperCore {

bool ShouldSuppressErrorPage(FrameType frame_type, const GURL& url);

void set_timer_for_testing(std::unique_ptr<base::Timer> timer) {
void set_timer_for_testing(std::unique_ptr<base::OneShotTimer> timer) {
auto_reload_timer_ = std::move(timer);
}

Expand Down Expand Up @@ -257,7 +257,7 @@ class NetErrorHelperCore {
const bool auto_reload_visible_only_;

// Timer used to wait for auto-reload attempts.
std::unique_ptr<base::Timer> auto_reload_timer_;
std::unique_ptr<base::OneShotTimer> auto_reload_timer_;

// True if the auto-reload timer would be running but is waiting for an
// offline->online network transition.
Expand Down
8 changes: 3 additions & 5 deletions net/base/network_throttle_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,7 @@ void NetworkThrottleManagerImpl::ThrottleImpl::NotifyUnblocked() {
NetworkThrottleManagerImpl::NetworkThrottleManagerImpl()
: lifetime_median_estimate_(PercentileEstimator::kMedianPercentile,
kInitialMedianInMs),
outstanding_recomputation_timer_(
std::make_unique<base::Timer>(false /* retain_user_task */,
false /* is_repeating */)),
outstanding_recomputation_timer_(std::make_unique<base::OneShotTimer>()),
tick_clock_(base::DefaultTickClock::GetInstance()),
weak_ptr_factory_(this) {}

Expand Down Expand Up @@ -191,8 +189,8 @@ void NetworkThrottleManagerImpl::SetTickClockForTesting(
const base::TickClock* tick_clock) {
tick_clock_ = tick_clock;
DCHECK(!outstanding_recomputation_timer_->IsRunning());
outstanding_recomputation_timer_ = std::make_unique<base::Timer>(
false /* retain_user_task */, false /* is_repeating */, tick_clock_);
outstanding_recomputation_timer_ =
std::make_unique<base::OneShotTimer>(tick_clock_);
}

bool NetworkThrottleManagerImpl::ConditionallyTriggerTimerForTesting() {
Expand Down
2 changes: 1 addition & 1 deletion net/base/network_throttle_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class NET_EXPORT NetworkThrottleManagerImpl : public NetworkThrottleManager {
// throttles are outstanding. This guarantees that the class will
// eventually detect aging out of outstanding throttles and unblock
// throttles blocked on those outstanding throttles.
std::unique_ptr<base::Timer> outstanding_recomputation_timer_;
std::unique_ptr<base::OneShotTimer> outstanding_recomputation_timer_;

// FIFO of OUTSTANDING throttles (ordered by time of entry into the
// OUTSTANDING state).
Expand Down
9 changes: 4 additions & 5 deletions net/dns/mdns_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,10 @@ void MDnsConnection::OnDatagramReceived(
delegate_->HandlePacket(response, bytes_read);
}

MDnsClientImpl::Core::Core(base::Clock* clock, base::Timer* timer)
MDnsClientImpl::Core::Core(base::Clock* clock, base::OneShotTimer* timer)
: clock_(clock),
cleanup_timer_(timer),
connection_(new MDnsConnection(this)) {
}
connection_(new MDnsConnection(this)) {}

MDnsClientImpl::Core::~Core() = default;

Expand Down Expand Up @@ -418,10 +417,10 @@ void MDnsClientImpl::Core::QueryCache(

MDnsClientImpl::MDnsClientImpl()
: clock_(base::DefaultClock::GetInstance()),
cleanup_timer_(new base::Timer(false, false)) {}
cleanup_timer_(new base::OneShotTimer()) {}

MDnsClientImpl::MDnsClientImpl(base::Clock* clock,
std::unique_ptr<base::Timer> timer)
std::unique_ptr<base::OneShotTimer> timer)
: clock_(clock), cleanup_timer_(std::move(timer)) {}

MDnsClientImpl::~MDnsClientImpl() = default;
Expand Down
10 changes: 5 additions & 5 deletions net/dns/mdns_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

namespace base {
class Clock;
class Timer;
class OneShotTimer;
} // namespace base

namespace net {
Expand Down Expand Up @@ -121,7 +121,7 @@ class NET_EXPORT_PRIVATE MDnsClientImpl : public MDnsClient {
// invalidate the core.
class Core : public base::SupportsWeakPtr<Core>, MDnsConnection::Delegate {
public:
Core(base::Clock* clock, base::Timer* timer);
Core(base::Clock* clock, base::OneShotTimer* timer);
~Core() override;

// Initialize the core. Returns true on success.
Expand Down Expand Up @@ -176,7 +176,7 @@ class NET_EXPORT_PRIVATE MDnsClientImpl : public MDnsClient {
MDnsCache cache_;

base::Clock* clock_;
base::Timer* cleanup_timer_;
base::OneShotTimer* cleanup_timer_;
base::Time scheduled_cleanup_;

std::unique_ptr<MDnsConnection> connection_;
Expand Down Expand Up @@ -210,11 +210,11 @@ class NET_EXPORT_PRIVATE MDnsClientImpl : public MDnsClient {

// Test constructor, takes a mock clock and mock timer.
MDnsClientImpl(base::Clock* clock,
std::unique_ptr<base::Timer> cleanup_timer);
std::unique_ptr<base::OneShotTimer> cleanup_timer);

std::unique_ptr<Core> core_;
base::Clock* clock_;
std::unique_ptr<base::Timer> cleanup_timer_;
std::unique_ptr<base::OneShotTimer> cleanup_timer_;

DISALLOW_COPY_AND_ASSIGN(MDnsClientImpl);
};
Expand Down
4 changes: 2 additions & 2 deletions net/http/bidirectional_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,14 @@ BidirectionalStream::BidirectionalStream(
session,
send_request_headers_automatically,
delegate,
std::make_unique<base::Timer>(false, false)) {}
std::make_unique<base::OneShotTimer>()) {}

BidirectionalStream::BidirectionalStream(
std::unique_ptr<BidirectionalStreamRequestInfo> request_info,
HttpNetworkSession* session,
bool send_request_headers_automatically,
Delegate* delegate,
std::unique_ptr<base::Timer> timer)
std::unique_ptr<base::OneShotTimer> timer)
: request_info_(std::move(request_info)),
net_log_(NetLogWithSource::Make(session->net_log(),
NetLogSourceType::BIDIRECTIONAL_STREAM)),
Expand Down
8 changes: 6 additions & 2 deletions net/http/bidirectional_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@
#include "net/http/http_stream_request.h"
#include "net/log/net_log_with_source.h"

namespace base {
class OneShotTimer;
} // namespace base

namespace spdy {
class SpdyHeaderBlock;
} // namespace spdy
Expand Down Expand Up @@ -120,7 +124,7 @@ class NET_EXPORT BidirectionalStream : public BidirectionalStreamImpl::Delegate,
HttpNetworkSession* session,
bool send_request_headers_automatically,
Delegate* delegate,
std::unique_ptr<base::Timer> timer);
std::unique_ptr<base::OneShotTimer> timer);

// Cancels |stream_request_| or |stream_impl_| if applicable.
// |this| should not be destroyed during Delegate::OnHeadersSent or
Expand Down Expand Up @@ -240,7 +244,7 @@ class NET_EXPORT BidirectionalStream : public BidirectionalStreamImpl::Delegate,

// Timer used to buffer data received in short time-spans and send a single
// read completion notification.
std::unique_ptr<base::Timer> timer_;
std::unique_ptr<base::OneShotTimer> timer_;
// HttpStreamRequest used to request a BidirectionalStreamImpl. This is NULL
// if the request has been canceled or completed.
std::unique_ptr<HttpStreamRequest> stream_request_;
Expand Down
4 changes: 2 additions & 2 deletions net/http/bidirectional_stream_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include "net/traffic_annotation/network_traffic_annotation.h"

namespace base {
class Timer;
class OneShotTimer;
} // namespace base

namespace spdy {
Expand Down Expand Up @@ -108,7 +108,7 @@ class NET_EXPORT_PRIVATE BidirectionalStreamImpl {
const NetLogWithSource& net_log,
bool send_request_headers_automatically,
BidirectionalStreamImpl::Delegate* delegate,
std::unique_ptr<base::Timer> timer,
std::unique_ptr<base::OneShotTimer> timer,
const NetworkTrafficAnnotationTag& traffic_annotation) = 0;

// Sends request headers to server.
Expand Down
6 changes: 3 additions & 3 deletions net/http/bidirectional_stream_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,11 @@ class TestDelegateBase : public BidirectionalStream::Delegate {
TestDelegateBase(IOBuffer* read_buf, int read_buf_len)
: TestDelegateBase(read_buf,
read_buf_len,
std::make_unique<base::Timer>(false, false)) {}
std::make_unique<base::OneShotTimer>()) {}

TestDelegateBase(IOBuffer* read_buf,
int read_buf_len,
std::unique_ptr<base::Timer> timer)
std::unique_ptr<base::OneShotTimer> timer)
: read_buf_(read_buf),
read_buf_len_(read_buf_len),
timer_(std::move(timer)),
Expand Down Expand Up @@ -286,7 +286,7 @@ class TestDelegateBase : public BidirectionalStream::Delegate {
std::unique_ptr<BidirectionalStream> stream_;
scoped_refptr<IOBuffer> read_buf_;
int read_buf_len_;
std::unique_ptr<base::Timer> timer_;
std::unique_ptr<base::OneShotTimer> timer_;
std::string data_received_;
std::unique_ptr<base::RunLoop> loop_;
spdy::SpdyHeaderBlock response_headers_;
Expand Down
2 changes: 1 addition & 1 deletion net/quic/chromium/bidirectional_stream_quic_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void BidirectionalStreamQuicImpl::Start(
const NetLogWithSource& net_log,
bool send_request_headers_automatically,
BidirectionalStreamImpl::Delegate* delegate,
std::unique_ptr<base::Timer> timer,
std::unique_ptr<base::OneShotTimer> timer,
const NetworkTrafficAnnotationTag& traffic_annotation) {
ScopedBoolSaver saver(&may_invoke_callbacks_, false);
DCHECK(!stream_);
Expand Down
4 changes: 2 additions & 2 deletions net/quic/chromium/bidirectional_stream_quic_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
#include "net/third_party/spdy/core/spdy_header_block.h"

namespace base {
class Timer;
class OneShotTimer;
} // namespace base

namespace net {
Expand All @@ -41,7 +41,7 @@ class NET_EXPORT_PRIVATE BidirectionalStreamQuicImpl
const NetLogWithSource& net_log,
bool send_request_headers_automatically,
BidirectionalStreamImpl::Delegate* delegate,
std::unique_ptr<base::Timer> timer,
std::unique_ptr<base::OneShotTimer> timer,
const NetworkTrafficAnnotationTag& traffic_annotation) override;
void SendRequestHeaders() override;
int ReadData(IOBuffer* buffer, int buffer_len) override;
Expand Down
6 changes: 3 additions & 3 deletions net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,11 @@ class TestDelegateBase : public BidirectionalStreamImpl::Delegate {
TestDelegateBase(IOBuffer* read_buf, int read_buf_len)
: TestDelegateBase(read_buf,
read_buf_len,
std::make_unique<base::Timer>(false, false)) {}
std::make_unique<base::OneShotTimer>()) {}

TestDelegateBase(IOBuffer* read_buf,
int read_buf_len,
std::unique_ptr<base::Timer> timer)
std::unique_ptr<base::OneShotTimer> timer)
: read_buf_(read_buf),
read_buf_len_(read_buf_len),
timer_(std::move(timer)),
Expand Down Expand Up @@ -293,7 +293,7 @@ class TestDelegateBase : public BidirectionalStreamImpl::Delegate {
std::unique_ptr<BidirectionalStreamQuicImpl> stream_;
scoped_refptr<IOBuffer> read_buf_;
int read_buf_len_;
std::unique_ptr<base::Timer> timer_;
std::unique_ptr<base::OneShotTimer> timer_;
std::string data_received_;
std::unique_ptr<base::RunLoop> loop_;
spdy::SpdyHeaderBlock response_headers_;
Expand Down
2 changes: 1 addition & 1 deletion net/spdy/bidirectional_stream_spdy_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void BidirectionalStreamSpdyImpl::Start(
const NetLogWithSource& net_log,
bool /*send_request_headers_automatically*/,
BidirectionalStreamImpl::Delegate* delegate,
std::unique_ptr<base::Timer> timer,
std::unique_ptr<base::OneShotTimer> timer,
const NetworkTrafficAnnotationTag& traffic_annotation) {
DCHECK(!stream_);
DCHECK(timer);
Expand Down
6 changes: 3 additions & 3 deletions net/spdy/bidirectional_stream_spdy_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#include "net/spdy/spdy_stream.h"

namespace base {
class Timer;
class OneShotTimer;
} // namespace base

namespace spdy {
Expand All @@ -50,7 +50,7 @@ class NET_EXPORT_PRIVATE BidirectionalStreamSpdyImpl
const NetLogWithSource& net_log,
bool send_request_headers_automatically,
BidirectionalStreamImpl::Delegate* delegate,
std::unique_ptr<base::Timer> timer,
std::unique_ptr<base::OneShotTimer> timer,
const NetworkTrafficAnnotationTag& traffic_annotation) override;
void SendRequestHeaders() override;
int ReadData(IOBuffer* buf, int buf_len) override;
Expand Down Expand Up @@ -90,7 +90,7 @@ class NET_EXPORT_PRIVATE BidirectionalStreamSpdyImpl
const base::WeakPtr<SpdySession> spdy_session_;
const BidirectionalStreamRequestInfo* request_info_;
BidirectionalStreamImpl::Delegate* delegate_;
std::unique_ptr<base::Timer> timer_;
std::unique_ptr<base::OneShotTimer> timer_;
SpdyStreamRequest stream_request_;
base::WeakPtr<SpdyStream> stream_;
const NetLogSource source_dependency_;
Expand Down
2 changes: 1 addition & 1 deletion net/spdy/bidirectional_stream_spdy_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ class TestDelegateBase : public BidirectionalStreamImpl::Delegate {
const NetLogWithSource& net_log) {
stream_->Start(request, net_log,
/*send_request_headers_automatically=*/false, this,
std::make_unique<base::Timer>(false, false),
std::make_unique<base::OneShotTimer>(),
TRAFFIC_ANNOTATION_FOR_TESTS);
not_expect_callback_ = false;
}
Expand Down
8 changes: 4 additions & 4 deletions net/websockets/websocket_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ class WebSocketStreamRequestImpl : public WebSocketStreamRequestAPI {
failure_message_ = message;
}

void Start(std::unique_ptr<base::Timer> timer) {
void Start(std::unique_ptr<base::OneShotTimer> timer) {
DCHECK(timer);
base::TimeDelta timeout(base::TimeDelta::FromSeconds(
kHandshakeTimeoutIntervalInSeconds));
Expand Down Expand Up @@ -309,7 +309,7 @@ class WebSocketStreamRequestImpl : public WebSocketStreamRequestAPI {
std::string failure_message_;

// A timer for handshake timeout.
std::unique_ptr<base::Timer> timer_;
std::unique_ptr<base::OneShotTimer> timer_;

// A delegate for On*HandshakeCreated and OnFailure calls.
std::unique_ptr<WebSocketStreamRequestAPI> api_delegate_;
Expand Down Expand Up @@ -489,7 +489,7 @@ std::unique_ptr<WebSocketStreamRequest> WebSocketStream::CreateAndConnectStream(
socket_url, url_request_context, origin, site_for_cookies,
additional_headers, std::move(connect_delegate), std::move(create_helper),
nullptr);
request->Start(std::make_unique<base::Timer>(false, false));
request->Start(std::make_unique<base::OneShotTimer>());
return std::move(request);
}

Expand All @@ -503,7 +503,7 @@ WebSocketStream::CreateAndConnectStreamForTesting(
URLRequestContext* url_request_context,
const NetLogWithSource& net_log,
std::unique_ptr<WebSocketStream::ConnectDelegate> connect_delegate,
std::unique_ptr<base::Timer> timer,
std::unique_ptr<base::OneShotTimer> timer,
std::unique_ptr<WebSocketStreamRequestAPI> api_delegate) {
auto request = std::make_unique<WebSocketStreamRequestImpl>(
socket_url, url_request_context, origin, site_for_cookies,
Expand Down
4 changes: 2 additions & 2 deletions net/websockets/websocket_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
class GURL;

namespace base {
class Timer;
class OneShotTimer;
}

namespace url {
Expand Down Expand Up @@ -173,7 +173,7 @@ class NET_EXPORT_PRIVATE WebSocketStream {
URLRequestContext* url_request_context,
const NetLogWithSource& net_log,
std::unique_ptr<ConnectDelegate> connect_delegate,
std::unique_ptr<base::Timer> timer,
std::unique_ptr<base::OneShotTimer> timer,
std::unique_ptr<WebSocketStreamRequestAPI> api_delegate);

// Derived classes must make sure Close() is called when the stream is not
Expand Down
4 changes: 2 additions & 2 deletions net/websockets/websocket_stream_create_test_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ void WebSocketStreamCreateTestBase::CreateAndConnectStream(
const url::Origin& origin,
const GURL& site_for_cookies,
const HttpRequestHeaders& additional_headers,
std::unique_ptr<base::Timer> timer) {
std::unique_ptr<base::OneShotTimer> timer) {
auto connect_delegate = std::make_unique<TestConnectDelegate>(
this, connect_run_loop_.QuitClosure());
auto create_helper = std::make_unique<WebSocketHandshakeStreamCreateHelper>(
Expand All @@ -108,7 +108,7 @@ void WebSocketStreamCreateTestBase::CreateAndConnectStream(
socket_url, std::move(create_helper), origin, site_for_cookies,
additional_headers, url_request_context_host_.GetURLRequestContext(),
NetLogWithSource(), std::move(connect_delegate),
timer ? std::move(timer) : std::make_unique<base::Timer>(false, false),
timer ? std::move(timer) : std::make_unique<base::OneShotTimer>(),
std::move(api_delegate));
}

Expand Down
2 changes: 1 addition & 1 deletion net/websockets/websocket_stream_create_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class WebSocketStreamCreateTestBase : public WithScopedTaskEnvironment {
const url::Origin& origin,
const GURL& site_for_cookies,
const HttpRequestHeaders& additional_headers,
std::unique_ptr<base::Timer> timer);
std::unique_ptr<base::OneShotTimer> timer);

static std::vector<HeaderKeyValuePair> RequestHeadersToVector(
const HttpRequestHeaders& headers);
Expand Down
Loading

0 comments on commit 08d8d6e

Please sign in to comment.