Commit e94db9af authored by Chris Whelan's avatar Chris Whelan Committed by Commit Bot

Decouples time sync from network connectivity

On fuchsia, network connectivity does not guarantee that the time has
synced. This change adds TimeSyncTracker in order to give fuchsia a
means to block global connectivity until this process is complete.

Bug: 162553428
Test: Manual

Change-Id: Ied2fa293247a860ddc85b62dbbf554abb2c75be5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2503406
Commit-Queue: Christopher Whelan <chwhelan@google.com>
Reviewed-by: default avatarMichael Spang <spang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822017}
parent 8cd57b8c
...@@ -46,6 +46,23 @@ cast_source_set("net_switches") { ...@@ -46,6 +46,23 @@ cast_source_set("net_switches") {
] ]
} }
cast_source_set("time_sync_tracker") {
sources = [
"time_sync_tracker.cc",
"time_sync_tracker.h",
]
deps = [
"//base",
]
if (is_fuchsia) {
sources += [
"time_sync_tracker_fuchsia.cc",
"time_sync_tracker_fuchsia.h",
]
}
}
cast_source_set("connectivity_checker") { cast_source_set("connectivity_checker") {
sources = [ sources = [
"connectivity_checker.cc", "connectivity_checker.cc",
...@@ -57,6 +74,7 @@ cast_source_set("connectivity_checker") { ...@@ -57,6 +74,7 @@ cast_source_set("connectivity_checker") {
] ]
deps = [ deps = [
":net_switches", ":net_switches",
":time_sync_tracker",
"//base", "//base",
"//chromecast:chromecast_buildflags", "//chromecast:chromecast_buildflags",
"//chromecast/base/metrics", "//chromecast/base/metrics",
......
...@@ -41,10 +41,12 @@ scoped_refptr<ConnectivityChecker> ConnectivityChecker::Create( ...@@ -41,10 +41,12 @@ scoped_refptr<ConnectivityChecker> ConnectivityChecker::Create(
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
std::unique_ptr<network::PendingSharedURLLoaderFactory> std::unique_ptr<network::PendingSharedURLLoaderFactory>
pending_url_loader_factory, pending_url_loader_factory,
network::NetworkConnectionTracker* network_connection_tracker) { network::NetworkConnectionTracker* network_connection_tracker,
TimeSyncTracker* time_sync_tracker) {
return ConnectivityCheckerImpl::Create(task_runner, return ConnectivityCheckerImpl::Create(task_runner,
std::move(pending_url_loader_factory), std::move(pending_url_loader_factory),
network_connection_tracker); network_connection_tracker,
time_sync_tracker);
} }
} // namespace chromecast } // namespace chromecast
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "base/memory/ref_counted_delete_on_sequence.h" #include "base/memory/ref_counted_delete_on_sequence.h"
#include "base/observer_list_threadsafe.h" #include "base/observer_list_threadsafe.h"
#include "base/sequenced_task_runner_helpers.h" #include "base/sequenced_task_runner_helpers.h"
#include "chromecast/net/time_sync_tracker.h"
namespace base { namespace base {
class SingleThreadTaskRunner; class SingleThreadTaskRunner;
...@@ -44,7 +45,8 @@ class ConnectivityChecker ...@@ -44,7 +45,8 @@ class ConnectivityChecker
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
std::unique_ptr<network::PendingSharedURLLoaderFactory> std::unique_ptr<network::PendingSharedURLLoaderFactory>
pending_url_loader_factory, pending_url_loader_factory,
network::NetworkConnectionTracker* network_connection_tracker); network::NetworkConnectionTracker* network_connection_tracker,
TimeSyncTracker* time_sync_tracker = nullptr);
void AddConnectivityObserver(ConnectivityObserver* observer); void AddConnectivityObserver(ConnectivityObserver* observer);
void RemoveConnectivityObserver(ConnectivityObserver* observer); void RemoveConnectivityObserver(ConnectivityObserver* observer);
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "chromecast/base/metrics/cast_metrics_helper.h" #include "chromecast/base/metrics/cast_metrics_helper.h"
#include "chromecast/chromecast_buildflags.h" #include "chromecast/chromecast_buildflags.h"
#include "chromecast/net/net_switches.h" #include "chromecast/net/net_switches.h"
#include "chromecast/net/time_sync_tracker.h"
#include "net/base/request_priority.h" #include "net/base/request_priority.h"
#include "net/http/http_network_session.h" #include "net/http/http_network_session.h"
#include "net/http/http_response_headers.h" #include "net/http/http_response_headers.h"
...@@ -67,11 +68,13 @@ scoped_refptr<ConnectivityCheckerImpl> ConnectivityCheckerImpl::Create( ...@@ -67,11 +68,13 @@ scoped_refptr<ConnectivityCheckerImpl> ConnectivityCheckerImpl::Create(
scoped_refptr<base::SingleThreadTaskRunner> task_runner, scoped_refptr<base::SingleThreadTaskRunner> task_runner,
std::unique_ptr<network::PendingSharedURLLoaderFactory> std::unique_ptr<network::PendingSharedURLLoaderFactory>
pending_url_loader_factory, pending_url_loader_factory,
network::NetworkConnectionTracker* network_connection_tracker) { network::NetworkConnectionTracker* network_connection_tracker,
TimeSyncTracker* time_sync_tracker) {
DCHECK(task_runner); DCHECK(task_runner);
auto connectivity_checker = base::WrapRefCounted( auto connectivity_checker = base::WrapRefCounted(
new ConnectivityCheckerImpl(task_runner, network_connection_tracker)); new ConnectivityCheckerImpl(task_runner, network_connection_tracker,
time_sync_tracker));
task_runner->PostTask( task_runner->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce(&ConnectivityCheckerImpl::Initialize, connectivity_checker, base::BindOnce(&ConnectivityCheckerImpl::Initialize, connectivity_checker,
...@@ -81,11 +84,14 @@ scoped_refptr<ConnectivityCheckerImpl> ConnectivityCheckerImpl::Create( ...@@ -81,11 +84,14 @@ scoped_refptr<ConnectivityCheckerImpl> ConnectivityCheckerImpl::Create(
ConnectivityCheckerImpl::ConnectivityCheckerImpl( ConnectivityCheckerImpl::ConnectivityCheckerImpl(
scoped_refptr<base::SingleThreadTaskRunner> task_runner, scoped_refptr<base::SingleThreadTaskRunner> task_runner,
network::NetworkConnectionTracker* network_connection_tracker) network::NetworkConnectionTracker* network_connection_tracker,
TimeSyncTracker* time_sync_tracker)
: ConnectivityChecker(task_runner), : ConnectivityChecker(task_runner),
task_runner_(std::move(task_runner)), task_runner_(std::move(task_runner)),
network_connection_tracker_(network_connection_tracker), network_connection_tracker_(network_connection_tracker),
connected_(false), time_sync_tracker_(time_sync_tracker),
connected_and_time_synced_(false),
network_connected_(false),
connection_type_(network::mojom::ConnectionType::CONNECTION_NONE), connection_type_(network::mojom::ConnectionType::CONNECTION_NONE),
check_errors_(0), check_errors_(0),
network_changed_pending_(false), network_changed_pending_(false),
...@@ -93,6 +99,10 @@ ConnectivityCheckerImpl::ConnectivityCheckerImpl( ...@@ -93,6 +99,10 @@ ConnectivityCheckerImpl::ConnectivityCheckerImpl(
DCHECK(task_runner_); DCHECK(task_runner_);
DCHECK(network_connection_tracker_); DCHECK(network_connection_tracker_);
weak_this_ = weak_factory_.GetWeakPtr(); weak_this_ = weak_factory_.GetWeakPtr();
if (time_sync_tracker_) {
time_sync_tracker_->AddObserver(this);
}
} }
void ConnectivityCheckerImpl::Initialize( void ConnectivityCheckerImpl::Initialize(
...@@ -122,17 +132,27 @@ ConnectivityCheckerImpl::~ConnectivityCheckerImpl() { ...@@ -122,17 +132,27 @@ ConnectivityCheckerImpl::~ConnectivityCheckerImpl() {
bool ConnectivityCheckerImpl::Connected() const { bool ConnectivityCheckerImpl::Connected() const {
base::AutoLock auto_lock(connected_lock_); base::AutoLock auto_lock(connected_lock_);
return connected_; return connected_and_time_synced_;
} }
void ConnectivityCheckerImpl::SetConnected(bool connected) { void ConnectivityCheckerImpl::SetConnected(bool connected) {
DCHECK(task_runner_->BelongsToCurrentThread()); DCHECK(task_runner_->BelongsToCurrentThread());
{ {
base::AutoLock auto_lock(connected_lock_); base::AutoLock auto_lock(connected_lock_);
if (connected_ == connected) { network_connected_ = connected;
// If a time_sync_tracker is not provided, is it assumed that network
// connectivity is equivalent to time being synced.
bool connected_and_time_synced = network_connected_;
if (time_sync_tracker_) {
connected_and_time_synced &= time_sync_tracker_->IsTimeSynced();
}
if (connected_and_time_synced_ == connected_and_time_synced) {
return; return;
} }
connected_ = connected;
connected_and_time_synced_ = connected_and_time_synced;
} }
base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
...@@ -140,12 +160,14 @@ void ConnectivityCheckerImpl::SetConnected(bool connected) { ...@@ -140,12 +160,14 @@ void ConnectivityCheckerImpl::SetConnected(bool connected) {
command_line->GetSwitchValueNative(switches::kConnectivityCheckUrl); command_line->GetSwitchValueNative(switches::kConnectivityCheckUrl);
if (check_url_str.empty()) { if (check_url_str.empty()) {
connectivity_check_url_.reset(new GURL( connectivity_check_url_.reset(new GURL(
connected ? kHttpConnectivityCheckUrl : kDefaultConnectivityCheckUrl)); connected_and_time_synced_ ? kHttpConnectivityCheckUrl
: kDefaultConnectivityCheckUrl));
LOG(INFO) << "Change check url=" << *connectivity_check_url_; LOG(INFO) << "Change check url=" << *connectivity_check_url_;
} }
Notify(connected); Notify(connected_and_time_synced_);
LOG(INFO) << "Global connection is: " << (connected ? "Up" : "Down"); LOG(INFO) << "Global connection is: "
<< (connected_and_time_synced_ ? "Up" : "Down");
} }
void ConnectivityCheckerImpl::Check() { void ConnectivityCheckerImpl::Check() {
...@@ -230,6 +252,10 @@ void ConnectivityCheckerImpl::OnConnectionChangedInternal() { ...@@ -230,6 +252,10 @@ void ConnectivityCheckerImpl::OnConnectionChangedInternal() {
Check(); Check();
} }
void ConnectivityCheckerImpl::OnTimeSynced() {
SetConnected(network_connected_);
}
void ConnectivityCheckerImpl::OnConnectivityCheckComplete( void ConnectivityCheckerImpl::OnConnectivityCheckComplete(
scoped_refptr<net::HttpResponseHeaders> headers) { scoped_refptr<net::HttpResponseHeaders> headers) {
DCHECK(task_runner_->BelongsToCurrentThread()); DCHECK(task_runner_->BelongsToCurrentThread());
...@@ -254,6 +280,9 @@ void ConnectivityCheckerImpl::OnConnectivityCheckComplete( ...@@ -254,6 +280,9 @@ void ConnectivityCheckerImpl::OnConnectivityCheckComplete(
DVLOG(1) << "Connectivity check succeeded"; DVLOG(1) << "Connectivity check succeeded";
check_errors_ = 0; check_errors_ = 0;
SetConnected(true); SetConnected(true);
if (time_sync_tracker_) {
time_sync_tracker_->OnNetworkConnected();
}
// Some products don't have an idle screen that makes periodic network // Some products don't have an idle screen that makes periodic network
// requests. Schedule another check to ensure connectivity hasn't dropped. // requests. Schedule another check to ensure connectivity hasn't dropped.
task_runner_->PostDelayedTask( task_runner_->PostDelayedTask(
...@@ -271,7 +300,7 @@ void ConnectivityCheckerImpl::OnUrlRequestError(ErrorType type) { ...@@ -271,7 +300,7 @@ void ConnectivityCheckerImpl::OnUrlRequestError(ErrorType type) {
++check_errors_; ++check_errors_;
if (check_errors_ > kNumErrorsToNotifyOffline) { if (check_errors_ > kNumErrorsToNotifyOffline) {
// Only record event on the connectivity transition. // Only record event on the connectivity transition.
if (connected_) { if (connected_and_time_synced_) {
metrics::CastMetricsHelper::GetInstance()->RecordEventWithValue( metrics::CastMetricsHelper::GetInstance()->RecordEventWithValue(
kMetricNameNetworkConnectivityCheckingErrorType, kMetricNameNetworkConnectivityCheckingErrorType,
static_cast<int>(type)); static_cast<int>(type));
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "chromecast/net/connectivity_checker.h" #include "chromecast/net/connectivity_checker.h"
#include "chromecast/net/time_sync_tracker.h"
#include "services/network/public/cpp/network_connection_tracker.h" #include "services/network/public/cpp/network_connection_tracker.h"
class GURL; class GURL;
...@@ -35,14 +36,16 @@ namespace chromecast { ...@@ -35,14 +36,16 @@ namespace chromecast {
// to given url. // to given url.
class ConnectivityCheckerImpl class ConnectivityCheckerImpl
: public ConnectivityChecker, : public ConnectivityChecker,
public network::NetworkConnectionTracker::NetworkConnectionObserver { public network::NetworkConnectionTracker::NetworkConnectionObserver,
public TimeSyncTracker::Observer {
public: public:
// Connectivity checking and initialization will run on task_runner. // Connectivity checking and initialization will run on task_runner.
static scoped_refptr<ConnectivityCheckerImpl> Create( static scoped_refptr<ConnectivityCheckerImpl> Create(
scoped_refptr<base::SingleThreadTaskRunner> task_runner, scoped_refptr<base::SingleThreadTaskRunner> task_runner,
std::unique_ptr<network::PendingSharedURLLoaderFactory> std::unique_ptr<network::PendingSharedURLLoaderFactory>
pending_url_loader_factory, pending_url_loader_factory,
network::NetworkConnectionTracker* network_connection_tracker); network::NetworkConnectionTracker* network_connection_tracker,
TimeSyncTracker* time_sync_tracker);
// ConnectivityChecker implementation: // ConnectivityChecker implementation:
bool Connected() const override; bool Connected() const override;
...@@ -51,7 +54,8 @@ class ConnectivityCheckerImpl ...@@ -51,7 +54,8 @@ class ConnectivityCheckerImpl
protected: protected:
explicit ConnectivityCheckerImpl( explicit ConnectivityCheckerImpl(
scoped_refptr<base::SingleThreadTaskRunner> task_runner, scoped_refptr<base::SingleThreadTaskRunner> task_runner,
network::NetworkConnectionTracker* network_connection_tracker); network::NetworkConnectionTracker* network_connection_tracker,
TimeSyncTracker* time_sync_tracker);
~ConnectivityCheckerImpl() override; ~ConnectivityCheckerImpl() override;
private: private:
...@@ -63,6 +67,9 @@ class ConnectivityCheckerImpl ...@@ -63,6 +67,9 @@ class ConnectivityCheckerImpl
// implementation: // implementation:
void OnConnectionChanged(network::mojom::ConnectionType type) override; void OnConnectionChanged(network::mojom::ConnectionType type) override;
// TimeSyncTracker::Observer implementation:
void OnTimeSynced() override;
void OnConnectionChangedInternal(); void OnConnectionChangedInternal();
void OnConnectivityCheckComplete( void OnConnectivityCheckComplete(
...@@ -93,11 +100,17 @@ class ConnectivityCheckerImpl ...@@ -93,11 +100,17 @@ class ConnectivityCheckerImpl
std::unique_ptr<network::SimpleURLLoader> url_loader_; std::unique_ptr<network::SimpleURLLoader> url_loader_;
const scoped_refptr<base::SingleThreadTaskRunner> task_runner_; const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
network::NetworkConnectionTracker* const network_connection_tracker_; network::NetworkConnectionTracker* const network_connection_tracker_;
TimeSyncTracker* const time_sync_tracker_;
// connected_lock_ protects access to connected_ which is shared across // connected_lock_ protects access to connected_ which is shared across
// threads. // threads.
mutable base::Lock connected_lock_; mutable base::Lock connected_lock_;
bool connected_; // Represents that the device has network connectivity and that time has
// synced.
bool connected_and_time_synced_;
// If the device has network connectivity.
bool network_connected_;
network::mojom::ConnectionType connection_type_; network::mojom::ConnectionType connection_type_;
// Number of connectivity check errors. // Number of connectivity check errors.
......
// Copyright 2020 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 file.
#include "chromecast/net/time_sync_tracker.h"
namespace chromecast {
TimeSyncTracker::TimeSyncTracker() {}
TimeSyncTracker::~TimeSyncTracker() = default;
void TimeSyncTracker::AddObserver(Observer* observer) {
observer_list_.AddObserver(observer);
}
void TimeSyncTracker::RemoveObserver(Observer* observer) {
observer_list_.RemoveObserver(observer);
}
void TimeSyncTracker::Notify() {
for (Observer& observer : observer_list_) {
observer.OnTimeSynced();
}
}
} // namespace chromecast
// Copyright 2020 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 file.
#ifndef CHROMECAST_NET_TIME_SYNC_TRACKER_H_
#define CHROMECAST_NET_TIME_SYNC_TRACKER_H_
#include "base/observer_list.h"
#include "base/observer_list_types.h"
namespace chromecast {
// Tracks whether or not time has synced on the device.
//
// In cases where general network connectivity does not include whether or not
// time has synced, this class provides that information.
class TimeSyncTracker {
public:
class Observer : public base::CheckedObserver {
public:
virtual void OnTimeSynced() = 0;
protected:
Observer() {}
~Observer() override = default;
};
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
// Informs the tracker that the network is connected.
virtual void OnNetworkConnected() = 0;
// Returns if the time has been synced.
virtual bool IsTimeSynced() const = 0;
protected:
TimeSyncTracker();
virtual ~TimeSyncTracker();
// Notifies observer that time has been synced.
void Notify();
private:
base::ObserverList<Observer> observer_list_;
};
} // namespace chromecast
#endif // CHROMECAST_NET_TIME_SYNC_TRACKER_H_
#include "chromecast/net/time_sync_tracker_fuchsia.h"
#include <lib/zx/clock.h>
#include <zircon/utc.h>
#include "base/bind.h"
#include "base/fuchsia/fuchsia_logging.h"
#include "base/logging.h"
#include "base/single_thread_task_runner.h"
#include "base/time/time.h"
namespace chromecast {
namespace {
// How often zx::clock is polled in seconds.
const unsigned int kPollPeriodSeconds = 1;
zx_handle_t GetUtcClockHandle() {
zx_handle_t clock_handle = zx_utc_reference_get();
DCHECK(clock_handle != ZX_HANDLE_INVALID);
return clock_handle;
}
} // namespace
TimeSyncTrackerFuchsia::TimeSyncTrackerFuchsia(
scoped_refptr<base::SingleThreadTaskRunner> task_runner)
: task_runner_(std::move(task_runner)),
utc_clock_(GetUtcClockHandle()),
weak_factory_(this) {
DCHECK(task_runner_);
weak_this_ = weak_factory_.GetWeakPtr();
}
TimeSyncTrackerFuchsia::~TimeSyncTrackerFuchsia() = default;
void TimeSyncTrackerFuchsia::OnNetworkConnected() {
if (!is_polling_ && !is_time_synced_) {
is_polling_ = true;
task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&TimeSyncTrackerFuchsia::Poll, weak_this_));
}
}
bool TimeSyncTrackerFuchsia::IsTimeSynced() const {
return is_time_synced_;
}
void TimeSyncTrackerFuchsia::Poll() {
DCHECK(task_runner_->BelongsToCurrentThread());
DCHECK(is_polling_);
zx_clock_details_v1_t details;
zx_status_t status = utc_clock_->get_details(&details);
ZX_CHECK(status == ZX_OK, status) << "zx_clock_get_details";
is_time_synced_ =
details.backstop_time != details.ticks_to_synthetic.synthetic_offset;
DVLOG(2) << __func__ << ": backstop_time=" << details.backstop_time
<< ", synthetic_offset=" << details.ticks_to_synthetic.synthetic_offset
<< ", synced=" << is_time_synced_;
if (!is_time_synced_) {
task_runner_->PostDelayedTask(
FROM_HERE, base::BindOnce(&TimeSyncTrackerFuchsia::Poll, weak_this_),
base::TimeDelta::FromSeconds(kPollPeriodSeconds));
return;
}
is_polling_ = false;
Notify();
}
} // namespace chromecast
// Copyright 2020 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 file.
#ifndef CHROMECAST_NET_TIME_SYNC_TRACKER_FUCHSIA_H_
#define CHROMECAST_NET_TIME_SYNC_TRACKER_FUCHSIA_H_
#include <lib/zx/clock.h>
#include "base/memory/scoped_refptr.h"
#include "base/memory/weak_ptr.h"
#include "chromecast/net/time_sync_tracker.h"
namespace base {
class SingleThreadTaskRunner;
} // namespace base
namespace chromecast {
class TimeSyncTrackerFuchsia : public TimeSyncTracker {
public:
explicit TimeSyncTrackerFuchsia(
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
TimeSyncTrackerFuchsia(const TimeSyncTrackerFuchsia&) = delete;
TimeSyncTrackerFuchsia& operator=(const TimeSyncTrackerFuchsia&) = delete;
~TimeSyncTrackerFuchsia() override;
// TimeSyncTracker implementation:
void OnNetworkConnected() final;
bool IsTimeSynced() const final;
private:
void Poll();
const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
zx::unowned_clock utc_clock_;
bool is_polling_ = false;
bool is_time_synced_ = false;
base::WeakPtr<TimeSyncTrackerFuchsia> weak_this_;
base::WeakPtrFactory<TimeSyncTrackerFuchsia> weak_factory_;
};
} // namespace chromecast
#endif // CHROMECAST_NET_TIME_SYNC_TRACKER_FUCHSIA_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment