[NetworkChangeNotifier] Run Windows connection type computation on other thread

net/base/network_change_notifier_win.h

Index: net/base/network_change_notifier_win.h
diff --git a/net/base/network_change_notifier_win.h b/net/base/network_change_notifier_win.h
index 94bab7f8d100213744e72863ebb4a2b0aa6b51b8..6c29d76b666b0d96ed62a79bbdc88f8952ce61df 100644
--- a/net/base/network_change_notifier_win.h
+++ b/net/base/network_change_notifier_win.h
@@ -9,8 +9,10 @@
 
 #include <memory>
 
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/non_thread_safe.h"
 #include "base/timer/timer.h"
@@ -18,6 +20,10 @@
 #include "net/base/net_export.h"
 #include "net/base/network_change_notifier.h"
 
+namespace base {
+class SingleThreadTaskRunner;
+}  // namespace base
+
 namespace net {
 
 // NetworkChangeNotifierWin inherits from NonThreadSafe, as all its internal
@@ -50,6 +56,7 @@ class NET_EXPORT_PRIVATE NetworkChangeNotifierWin
  private:
   class DnsConfigServiceThread;
   friend class NetworkChangeNotifierWinTest;
+  friend class TestNetworkChangeNotifierWin;
 
   // NetworkChangeNotifier methods:
   ConnectionType GetCurrentConnectionType() const override;
@@ -60,17 +67,18 @@ class NET_EXPORT_PRIVATE NetworkChangeNotifierWin
 
   // Does the actual work to determine the current connection type.
   // It is not thread safe, see crbug.com/324913.
-  virtual ConnectionType RecomputeCurrentConnectionType() const;
+  static ConnectionType RecomputeCurrentConnectionType();

[pauljensen, 2017/05/23 17:57:40]

can we leave this virtual since NCNWin outlives dns thread? then tests can
overload it, and we can get rid of calculate_connection_type_

[jkarlin, 2017/05/23 18:02:15]

NCNWin outlives dns thread, but there is no guarantee that the
NetworkChangeNotifierWin will still be alive by the time the reply task is run.

E.g.,

1. On main thread NCN PostTaskAndReply's to dns thread
2. dns thread does its thing and posts a task on the main thread
3. main thread runs a task to delete the NCN
4. main thread runs the reply task, but the NCN is gone

[pauljensen, 2017/05/23 18:05:47]

can it post back with a weakptr?

[pauljensen, 2017/05/23 18:06:47]

I think this is very similar to the example here:
https://chromium.googlesource.com/chromium/src/+/master/docs/threading_and_ta...

[jkarlin, 2017/05/26 15:09:55]

Back to the original.

 
   void SetCurrentConnectionType(ConnectionType connection_type);
 
   // Notifies IP address change observers of a change immediately, and notifies
   // network state change observers on a delay.  Must only be called on the
   // thread |this| was created on.
-  void NotifyObservers();
+  void NotifyObservers(ConnectionType connection_type);
 
   // Forwards connection type notifications to parent class.
   void NotifyParentOfConnectionTypeChange();
+  void NotifyParentOfConnectionTypeChangeImpl(ConnectionType connection_type);
 
   // Tries to start listening for a single subsequent address change.  Returns
   // false on failure.  The caller is responsible for updating |is_watching_|.
@@ -98,6 +106,7 @@ class NET_EXPORT_PRIVATE NetworkChangeNotifierWin
 
   // Thread on which we can run DnsConfigService.
   std::unique_ptr<DnsConfigServiceThread> dns_config_service_thread_;
+  scoped_refptr<base::SingleThreadTaskRunner> dns_task_runner_;
 
   mutable base::Lock last_computed_connection_type_lock_;
   ConnectionType last_computed_connection_type_;
@@ -108,6 +117,9 @@ class NET_EXPORT_PRIVATE NetworkChangeNotifierWin
   // Number of times polled to check if still offline.
   int offline_polls_;
 
+  // The callback run to calculate connection type.
+  base::Callback<ConnectionType()> calculate_connection_type_;
+
   // Used for calling WatchForAddressChange again on failure.
   base::WeakPtrFactory<NetworkChangeNotifierWin> weak_factory_;