NQE: Notify observer as soon as it is added

net/nqe/network_quality_estimator.cc

Index: net/nqe/network_quality_estimator.cc
diff --git a/net/nqe/network_quality_estimator.cc b/net/nqe/network_quality_estimator.cc
index 926ec14904722328db9e04c9acf81c5056081a48..724b8a5f53e67f015b14a87218d1f8842dcc8619 100644
--- a/net/nqe/network_quality_estimator.cc
+++ b/net/nqe/network_quality_estimator.cc
@@ -1147,7 +1147,13 @@ NetworkQualityEstimator::GetRecentEffectiveConnectionTypeUsingMetrics(
 void NetworkQualityEstimator::AddEffectiveConnectionTypeObserver(
     EffectiveConnectionTypeObserver* observer) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(observer);
   effective_connection_type_observer_list_.AddObserver(observer);
+
+  base::ThreadTaskRunnerHandle::Get()->PostTask(

[bengr, 2016/11/11 17:26:44]

The real question is why do you post tasks for all of these instead of calling
directly?

[RyanSturm, 2016/11/11 17:30:18]

I like avoiding weird re-entrancy issues. Seems like if adding the observer was
part of initialization, the observer might not be fully set up to handle the
callback immediately.

[tbansal1, 2016/11/11 18:20:45]

Right, the caller may not have been set up fully. 

[bengr, 2016/11/16 17:06:40]

Please add a comment somewhere that says you post so as not to require the
caller to be fully set up before registering an observer.

+      FROM_HERE, base::Bind(&NetworkQualityEstimator::
+                                NotifyEffectiveConnectionTypeObserverIfPresent,
+                            weak_ptr_factory_.GetWeakPtr(), observer));
 }
 
 void NetworkQualityEstimator::RemoveEffectiveConnectionTypeObserver(
@@ -1159,7 +1165,14 @@ void NetworkQualityEstimator::RemoveEffectiveConnectionTypeObserver(
 void NetworkQualityEstimator::AddRTTAndThroughputEstimatesObserver(
     RTTAndThroughputEstimatesObserver* observer) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(observer);
   rtt_and_throughput_estimates_observer_list_.AddObserver(observer);
+
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE,
+      base::Bind(&NetworkQualityEstimator::
+                     NotifyRTTAndThroughputEstimatesObserverIfPresent,
+                 weak_ptr_factory_.GetWeakPtr(), observer));
 }
 
 void NetworkQualityEstimator::RemoveRTTAndThroughputEstimatesObserver(
@@ -1501,6 +1514,28 @@ void NetworkQualityEstimator::NotifyObserversOfRTTOrThroughputComputed() const {
   }
 }
 
+void NetworkQualityEstimator::NotifyEffectiveConnectionTypeObserverIfPresent(
+    EffectiveConnectionTypeObserver* observer) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (!effective_connection_type_observer_list_.HasObserver(observer))
+    return;
+  if (effective_connection_type_ == EFFECTIVE_CONNECTION_TYPE_UNKNOWN)
+    return;
+  observer->OnEffectiveConnectionTypeChanged(effective_connection_type_);
+}
+
+void NetworkQualityEstimator::NotifyRTTAndThroughputEstimatesObserverIfPresent(
+    RTTAndThroughputEstimatesObserver* observer) const {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (!rtt_and_throughput_estimates_observer_list_.HasObserver(observer))
+    return;
+  observer->OnRTTOrThroughputEstimatesComputed(
+      network_quality_.http_rtt(), network_quality_.transport_rtt(),
+      network_quality_.downstream_throughput_kbps());
+}
+
 void NetworkQualityEstimator::AddNetworkQualitiesCacheObserver(
     nqe::internal::NetworkQualityStore::NetworkQualitiesCacheObserver*
         observer) {