Initial CL for Data Saver (Flywheel) prompt when cellular network detected

chromeos/network/network_state_handler.h

 // Copyright (c) 2012 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 CHROMEOS_NETWORK_NETWORK_STATE_HANDLER_H_
 #define CHROMEOS_NETWORK_NETWORK_STATE_HANDLER_H_
 
 #include <map>
 #include <set>
 #include <string>
@@ skipping 209 matching lines @@
   // connected using EAP, returns NULL.
   const NetworkState* GetEAPForEthernet(const std::string& service_path);
 
   const std::string& default_network_path() const {
     return default_network_path_;
   }
 
   // Constructs and initializes an instance for testing.
   static NetworkStateHandler* InitializeForTest();
 
+  // Used to simulate change in network properties

[Daniel Erat, 2015/03/23 03:49:21]

nit: add trailing period

[Greg Levin, 2015/03/24 20:35:32]

Done.

+  void DebugNotifyNetworkPropertiesUpdated();

[Daniel Erat, 2015/03/23 03:49:21]

adding a debug method here doesn't seem right.

[Greg Levin, 2015/03/24 20:35:32]

Could you elaborate on what seems wrong about this?  Do you have any suggestions
for a better place to put it, or a different approach to triggering the prompt
from an accelerator?  Here's my thinking for putting this here:

* CTRL+ALT+SHIFT+3 is meant to simulate a switch to cellular data (since this
isn't always easy to do in practice), which will trigger the Data Saver prompt
for debugging and demo purposes.  NetworkStateHandler is where
NetworkPropertiesUpdated notifications are dispatched, so this seems about like
what my accelerator is trying to fake.

* There didn't seem to be a good way to put the accelerator handler closer to
the source in DataPromoNotification.  This class is only instantiated as a
member of ChromeBrowserMainPartsChromeos, and is never accessed directly. 
DataPromoNotification registers itself on construction as a NetworkStateHandler
observer, and the NetworkPropertiesUpdated and DefaultNetworkChanged
notifications are currently the only entry points into DataPromoNotification
code.  So the accelerator handler has to go through NetworkStateHandler (unless
I built some new framework), and that class can be accessed directly from
debug_commands.cc.

* Not that this is an excuse for bad design, but this code is temporary.  The
debug accelerator and this function will be removed once the Data Saver prompt
is taken out from behind its experimental flag.

Of course, I don't pretend to understand all the surrounding code, so there
could easily be a better solution that I'm not seeing.

[Daniel Erat, 2015/03/24 20:45:45]

thanks for the explanation. there are stub implementations of many of the
chromeos/ classes that are used only when running a chrome os build on the
desktop, although i'm not sure whether that applies to this code as well.
stevenjb@ is the best person to comment on this. if it needs to go here, please
add a comment stating that it's temporary and linking to the bug that's tracking
the feature (to make it less likely that we'll forget to remove it later).

+
   // Default set of comma separated interfaces on which to enable
   // portal checking.
   static const char kDefaultCheckPortalList[];
 
  protected:
   friend class NetworkHandler;
   NetworkStateHandler();
 
   // ShillPropertyHandler::Listener overrides.
 
@@ skipping 139 matching lines @@
   // Map of network specifiers to guids. Contains an entry for each
   // NetworkState that is not saved in a profile.
   SpecifierGuidMap specifier_guid_map_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkStateHandler);
 };
 
 }  // namespace chromeos
 
 #endif  // CHROMEOS_NETWORK_NETWORK_STATE_HANDLER_H_