Exposing NQE on the Browser UI thread

chrome/browser/net/nqe/ui_network_quality_estimator_service.cc

Index: chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
diff --git a/chrome/browser/net/nqe/ui_network_quality_estimator_service.cc b/chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0ccbb5e295ae8677c319bdd5d16b150d23f6caa6
--- /dev/null
+++ b/chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
@@ -0,0 +1,116 @@
+// Copyright 2016 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 "chrome/browser/net/nqe/ui_network_quality_estimator_service.h"
+
+#include "base/bind.h"
+#include "base/message_loop/message_loop.h"
+#include "base/task_runner.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/io_thread.h"
+
+namespace chrome_browser_net {
+
+// A class that sets itself as an observer of the EffectiveconnectionType for
+// the browser IO thread. It reports any change in EffectiveConnectionType back
+// to the UI service.
+// It is created on the UI thread, but used and deleted on the IO thread.
+class UINetworkQualityEstimatorService::IONetworkQualityObserver
+    : public net::NetworkQualityEstimator::EffectiveConnectionTypeObserver {
+ public:
+  IONetworkQualityObserver(
+      const scoped_refptr<base::SingleThreadTaskRunner>& ui_task_runner,
+      base::WeakPtr<UINetworkQualityEstimatorService> service)
+      : ui_task_runner_(ui_task_runner), service_(service) {
+    DCHECK(ui_task_runner_);
+    // This is created on the UI thread, but used only on the IO thread.
+    thread_checker_.DetachFromThread();
+  }
+
+  ~IONetworkQualityObserver() override {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    if (network_quality_estimator_)
+      network_quality_estimator_->RemoveEffectiveConnectionTypeObserver(this);
+  }
+
+  void InitializeOnIOThread(IOThread* io_thread) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    if (!io_thread->globals()->network_quality_estimator)

[mmenke, 2016/07/14 19:10:12]

Hrm...Is there any plan to use per-profile data instead?  Wonder if this really
gets anything from being a Profile keyed service (BrowserContextKeyedService? 
Whatever)..

[RyanSturm, 2016/07/14 21:42:12]

Because network prefs will be stored per profile, an estimate of the initial
EffectiveConnectionType would be available synchronously during start up for
some profiles and not others. This matches the decision to use per-profile
prefs, which was made to have clear ownership of this information in a profile.
If a user wants to clear their cache, it will clear the prefs for that user, for
example.

[mmenke, 2016/07/18 21:34:18]

ok, as long as there's a reason, SGTM.

+      return;
+    network_quality_estimator_ =
+        io_thread->globals()->network_quality_estimator.get();
+    if (!network_quality_estimator_)
+      return;
+    network_quality_estimator_->AddEffectiveConnectionTypeObserver(this);
+    ui_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &UINetworkQualityEstimatorService::EffectiveConnectionTypeChanged,
+            service_,
+            network_quality_estimator_->GetEffectiveConnectionType()));
+  }
+
+  // net::NetworkQualityEstimator::EffectiveConnectionTypeObserver
+  // implementation:
+  void OnEffectiveConnectionTypeChanged(
+      net::NetworkQualityEstimator::EffectiveConnectionType type) override {
+    DCHECK(thread_checker_.CalledOnValidThread());

[mmenke, 2016/07/14 19:10:12]

Seems much clearer to DCHECK against browser thread instead - as-is, it's a bit
confusing what's run on what thread.

[RyanSturm, 2016/07/14 21:42:12]

Done.

+    ui_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &UINetworkQualityEstimatorService::EffectiveConnectionTypeChanged,
+            service_, type));
+  }
+
+ private:
+  scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
+  base::WeakPtr<UINetworkQualityEstimatorService> service_;
+  net::NetworkQualityEstimator* network_quality_estimator_;
+  base::ThreadChecker thread_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(IONetworkQualityObserver);
+};
+
+UINetworkQualityEstimatorService::UINetworkQualityEstimatorService(
+    Profile* profile)
+    : type_(net::NetworkQualityEstimator::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
+      io_observer_(nullptr),
+      weak_factory_(this) {}
+
+UINetworkQualityEstimatorService::~UINetworkQualityEstimatorService() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(io_task_runner_->DeleteSoon(FROM_HERE, io_observer_));
+}
+
+void UINetworkQualityEstimatorService::Initialize(
+    const scoped_refptr<base::SingleThreadTaskRunner>& ui_task_runner,
+    const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  io_task_runner_ = io_task_runner;
+  io_observer_ =
+      new IONetworkQualityObserver(ui_task_runner, weak_factory_.GetWeakPtr());
+  io_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&IONetworkQualityObserver::InitializeOnIOThread,
+                            base::Unretained(io_observer_),
+                            g_browser_process->io_thread()));
+}
+
+void UINetworkQualityEstimatorService::Shutdown() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  weak_factory_.InvalidateWeakPtrs();
+}
+
+void UINetworkQualityEstimatorService::EffectiveConnectionTypeChanged(
+    net::NetworkQualityEstimator::EffectiveConnectionType type) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  type_ = type;
+}
+
+net::NetworkQualityEstimator::EffectiveConnectionType
+UINetworkQualityEstimatorService::type() const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  return type_;
+}
+
+}  // namespace chrome_browser_net