Revert of Use networkingPrivate.startConnect (patchset #12 id:240001 of https://codereview.chromium.org/1043343002/)

Reason for revert:
Causes browser_test failure on Win7 bot.
https://build.chromium.org/p/chromium.win/builders/Win%207%20Tests%20x64%20(1)

[ RUN      ] WebNavigationApiTest.Failures
[4444:4508:0414/144708:WARNING:data_reduction_proxy_config.cc(319)] SPDY proxy OFF at startup
[4444:2028:0414/144708:ERROR:bluetooth_adapter_win.cc(102)] NOT IMPLEMENTED
[180:3848:0414/144709:WARNING:raw_channel_win.cc(473)] WriteFile: Error (0x5) while retrieving error. (0xE8)
[180:3848:0414/144709:WARNING:channel.cc(549)] Failed to send message to ack remove remote endpoint (local ID 2147483650, remote ID 3)
[180:3848:0414/144709:WARNING:channel.cc(315)] RawChannel write error
[180:3848:0414/144709:WARNING:raw_channel_win.cc(473)] WriteFile: Error (0x3B01) while retrieving error. (0xE8)
[180:3848:0414/144709:WARNING:channel.cc(549)] Failed to send message to ack remove remote endpoint (local ID 1, remote ID 1)
[180:3848:0414/144709:WARNING:channel.cc(315)] RawChannel write error
[4444:2028:0414/144709:INFO:CONSOLE(0)] "[SUCCESS] nonExistentIframe", source: chrome-extension://ollhgkgdokpphndfogkepkmblokamlkf/test_failures.html (0)
[4444:2028:0414/144710:INFO:CONSOLE(0)] "[SUCCESS] nonExistentIframeNavigation", source: chrome-extension://ollhgkgdokpphndfogkepkmblokamlkf/test_failures.html (0)
[4444:2028:0414/144710:INFO:CONSOLE(0)] "[FAIL] cancel: Received unexpected event 'onCompleted':{"frameId":0,"processId":0,"tabId":0,"timeStamp":0,"url":"chrome-extension://ollhgkgdokpphndfogkepkmblokamlkf/e.html"}
Error
    at captureEvent (chrome-extension://ollhgkgdokpphndfogkepkmblokamlkf/framework.js:194:17)
    at chrome-extension://ollhgkgdokpphndfogkepkmblokamlkf/framework.js:219:5
    at EventImpl.dispatchToListener (extensions::event_bindings:395:22)
    at Event.publicClass.(anonymous function) [as dispatchToListener] (extensions::utils:94:26)
    at EventImpl.dispatch_ (extensions::event_bindings:379:35)
    at dispatchArgs (extensions::event_bindings:247:26)
    at dispatchEvent (extensions::event_bindings:256:7)", source: chrome-extension://ollhgkgdokpphndfogkepkmblokamlkf/test_failures.html (0)
[4444:2028:0414/144710:INFO:CONSOLE(0)] "[SUCCESS] nonExistent", source: chrome-extension://ollhgkgdokpphndfogkepkmblokamlkf/test_failures.html (0)
c:\b\build\slave\win_x64_builder\build\src\chrome\browser\extensions\api\web_navigation\web_navigation_apitest.cc(468): error: Value of: RunExtensionTest("webnavigation/failures")
  Actual: false
Expected: true
Failed 1 of 4 tests
[4444:2028:0414/144710:ERROR:browser_thread.h(263)] DeleteSoon failed on thread 0
[  FAILED  ] WebNavigationApiTest.Failures, where TypeParam =  and GetParam() =  (2465 ms)

Original issue's description:
> Use networkingPrivate.startConnect
>
> This adds checks for unconfigured or non-activated networks to internet_details.js since it no longer uses the checks in network_connect.cc.
> It also moves some notification handling from network_connect.cc to network_state_notifier.cc so that it does not rely on network_connect.cc.
>
> BUG=430115
>
> Committed: https://crrev.com/e22e5bcbad2cea941993b32e6cc3b082822a42fc
> Cr-Commit-Position: refs/heads/master@{#325108}

TBR=armansito@chromium.org,michaelpg@chromium.org,pneubeck@chromium.org,stevenjb@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=430115

Review URL: https://codereview.chromium.org/1085993002

Cr-Commit-Position: refs/heads/master@{#325145}

12 files changed, 49 insertions, 99 deletions

diff --git a/chromeos/dbus/fake_shill_service_client.cc b/chromeos/dbus/fake_shill_service_client.cc
index fdae9db..d4e789c3 100644
--- a/chromeos/dbus/fake_shill_service_client.cc
+++ b/chromeos/dbus/fake_shill_service_client.cc
@@ -178,8 +178,8 @@ void FakeShillServiceClient::Connect(const dbus::ObjectPath& service_path,
                                      const ErrorCallback& error_callback) {
   VLOG(1) << "FakeShillServiceClient::Connect: " << service_path.value();
   base::DictionaryValue* service_properties = NULL;
-  if (!stub_services_.GetDictionary(service_path.value(),
-                                    &service_properties)) {
+  if (!stub_services_.GetDictionary(
+          service_path.value(), &service_properties)) {
     LOG(ERROR) << "Service not found: " << service_path.value();
     error_callback.Run("Error.InvalidService", "Invalid Service");
     return;
@@ -190,22 +190,21 @@ void FakeShillServiceClient::Connect(const dbus::ObjectPath& service_path,
   // sending an update.
   SetOtherServicesOffline(service_path.value());
 
-  // Clear Error.
-  service_properties->SetStringWithoutPathExpansion(shill::kErrorProperty, "");
-
   // Set Associating.
   base::StringValue associating_value(shill::kStateAssociation);
-  SetServiceProperty(service_path.value(), shill::kStateProperty,
+  SetServiceProperty(service_path.value(),
+                     shill::kStateProperty,
                      associating_value);
 
   // Stay Associating until the state is changed again after a delay.
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&FakeShillServiceClient::ContinueConnect,
-                 weak_ptr_factory_.GetWeakPtr(), service_path.value()),
+                 weak_ptr_factory_.GetWeakPtr(),
+                 service_path.value()),
       base::TimeDelta::FromSeconds(GetInteractiveDelay()));
 
-  base::MessageLoop::current()->PostTask(FROM_HERE, callback);
+  callback.Run();
 }
 
 void FakeShillServiceClient::Disconnect(const dbus::ObjectPath& service_path,
@@ -607,7 +606,8 @@ void FakeShillServiceClient::SetCellularActivated(
               error_callback);
 }
 
-void FakeShillServiceClient::ContinueConnect(const std::string& service_path) {
+void FakeShillServiceClient::ContinueConnect(
+    const std::string& service_path) {
   VLOG(1) << "FakeShillServiceClient::ContinueConnect: " << service_path;
   base::DictionaryValue* service_properties = NULL;
   if (!stub_services_.GetDictionary(service_path, &service_properties)) {
@@ -625,24 +625,26 @@ void FakeShillServiceClient::ContinueConnect(const std::string& service_path) {
 
   // No custom connect behavior set, continue with the default connect behavior.
   std::string passphrase;
-  service_properties->GetStringWithoutPathExpansion(shill::kPassphraseProperty,
-                                                    &passphrase);
+  service_properties->GetStringWithoutPathExpansion(
+      shill::kPassphraseProperty, &passphrase);
   if (passphrase == "failure") {
     // Simulate a password failure.
-    SetServiceProperty(service_path, shill::kErrorProperty,
-                       base::StringValue(shill::kErrorBadPassphrase));
-    SetServiceProperty(service_path, shill::kStateProperty,
+    SetServiceProperty(service_path,
+                       shill::kStateProperty,
                        base::StringValue(shill::kStateFailure));
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(
             base::IgnoreResult(&FakeShillServiceClient::SetServiceProperty),
-            weak_ptr_factory_.GetWeakPtr(), service_path, shill::kErrorProperty,
+            weak_ptr_factory_.GetWeakPtr(),
+            service_path,
+            shill::kErrorProperty,
             base::StringValue(shill::kErrorBadPassphrase)));
   } else {
     // Set Online.
     VLOG(1) << "Setting state to Online " << service_path;
-    SetServiceProperty(service_path, shill::kStateProperty,
+    SetServiceProperty(service_path,
+                       shill::kStateProperty,
                        base::StringValue(shill::kStateOnline));
   }
 }
diff --git a/chromeos/network/managed_network_configuration_handler_impl.cc b/chromeos/network/managed_network_configuration_handler_impl.cc
index f20cdcc3..a92be66 100644
--- a/chromeos/network/managed_network_configuration_handler_impl.cc
+++ b/chromeos/network/managed_network_configuration_handler_impl.cc
@@ -164,12 +164,9 @@ void ManagedNetworkConfigurationHandlerImpl::SendManagedProperties(
 
   ::onc::ONCSource onc_source;
   FindPolicyByGUID(userhash, guid, &onc_source);
-  const NetworkState* network_state =
-      network_state_handler_->GetNetworkState(service_path);
   scoped_ptr<base::DictionaryValue> active_settings(
-      onc::TranslateShillServiceToONCPart(*shill_properties, onc_source,
-                                          &onc::kNetworkWithStateSignature,
-                                          network_state));
+      onc::TranslateShillServiceToONCPart(
+          *shill_properties, onc_source, &onc::kNetworkWithStateSignature));
 
   const base::DictionaryValue* network_policy = NULL;
   const base::DictionaryValue* global_policy = NULL;
@@ -216,12 +213,10 @@ void ManagedNetworkConfigurationHandlerImpl::SendProperties(
     const network_handler::ErrorCallback& error_callback,
     const std::string& service_path,
     scoped_ptr<base::DictionaryValue> shill_properties) {
-  const NetworkState* network_state =
-      network_state_handler_->GetNetworkState(service_path);
   scoped_ptr<base::DictionaryValue> onc_network(
       onc::TranslateShillServiceToONCPart(
           *shill_properties, ::onc::ONC_SOURCE_UNKNOWN,
-          &onc::kNetworkWithStateSignature, network_state));
+          &onc::kNetworkWithStateSignature));
   callback.Run(service_path, *onc_network);
 }
 
diff --git a/chromeos/network/network_state.cc b/chromeos/network/network_state.cc
index 93c12e4..f4ad959 100644
--- a/chromeos/network/network_state.cc
+++ b/chromeos/network/network_state.cc
@@ -231,6 +231,8 @@ void NetworkState::GetStateProperties(base::DictionaryValue* dictionary) const {
                                             profile_path());
 
   if (visible()) {
+    if (!error().empty())
+      dictionary->SetStringWithoutPathExpansion(shill::kErrorProperty, error());
     dictionary->SetStringWithoutPathExpansion(shill::kStateProperty,
                                               connection_state());
   }
@@ -396,12 +398,6 @@ bool NetworkState::UpdateName(const base::DictionaryValue& properties) {
   return false;
 }
 
-std::string NetworkState::GetErrorState() const {
-  if (ErrorIsValid(error()))
-    return error();
-  return last_error();
-}
-
 // static
 bool NetworkState::StateIsConnected(const std::string& connection_state) {
   return (connection_state == shill::kStateReady ||
diff --git a/chromeos/network/network_state.h b/chromeos/network/network_state.h
index 518e568..e4d9cce 100644
--- a/chromeos/network/network_state.h
+++ b/chromeos/network/network_state.h
@@ -126,9 +126,6 @@ class CHROMEOS_EXPORT NetworkState : public ManagedState {
   // Set the GUID. Called exclusively by NetworkStateHandler.
   void SetGuid(const std::string& guid);
 
-  // Returns |error_| if valid, otherwise returns |last_error_|.
-  std::string GetErrorState() const;
-
   // Helpers (used e.g. when a state, error, or shill dictionary is cached)
   static bool StateIsConnected(const std::string& connection_state);
   static bool StateIsConnecting(const std::string& connection_state);
diff --git a/chromeos/network/network_state_handler.cc b/chromeos/network/network_state_handler.cc
index a8a9ebf..236789c 100644
--- a/chromeos/network/network_state_handler.cc
+++ b/chromeos/network/network_state_handler.cc
@@ -417,16 +417,6 @@ const NetworkState* NetworkStateHandler::GetEAPForEthernet(
   return list.front();
 }
 
-void NetworkStateHandler::SetLastErrorForTest(const std::string& service_path,
-                                              const std::string& error) {
-  NetworkState* network_state = GetModifiableNetworkState(service_path);
-  if (!network_state) {
-    LOG(ERROR) << "No matching NetworkState for: " << service_path;
-    return;
-  }
-  network_state->last_error_ = error;
-}
-
 //------------------------------------------------------------------------------
 // ShillPropertyHandler::Delegate overrides
 
diff --git a/chromeos/network/network_state_handler.h b/chromeos/network/network_state_handler.h
index f032f77..135d225 100644
--- a/chromeos/network/network_state_handler.h
+++ b/chromeos/network/network_state_handler.h
@@ -224,10 +224,6 @@ class CHROMEOS_EXPORT NetworkStateHandler
     return default_network_path_;
   }
 
-  // Sets the |last_error_| property of the matching NetworkState for tests.
-  void SetLastErrorForTest(const std::string& service_path,
-                           const std::string& error);
-
   // Constructs and initializes an instance for testing.
   static NetworkStateHandler* InitializeForTest();
 
diff --git a/chromeos/network/network_util.cc b/chromeos/network/network_util.cc
index 5fa7de5..b5d50b3 100644
--- a/chromeos/network/network_util.cc
+++ b/chromeos/network/network_util.cc
@@ -179,7 +179,7 @@ scoped_ptr<base::DictionaryValue> TranslateNetworkStateToONC(
 
   scoped_ptr<base::DictionaryValue> onc_dictionary =
       TranslateShillServiceToONCPart(*shill_dictionary, onc_source,
-                                     &onc::kNetworkWithStateSignature, network);
+                                     &onc::kNetworkWithStateSignature);
   return onc_dictionary.Pass();
 }
 
diff --git a/chromeos/network/onc/onc_translation_tables.cc b/chromeos/network/onc/onc_translation_tables.cc
index ad7eef8..285a7e1 100644
--- a/chromeos/network/onc/onc_translation_tables.cc
+++ b/chromeos/network/onc/onc_translation_tables.cc
@@ -187,23 +187,24 @@ const FieldTranslationEntry cellular_fields[] = {
 const FieldTranslationEntry network_fields[] = {
     {::onc::network_config::kGUID, shill::kGuidProperty},
     {::onc::network_config::kConnectable, shill::kConnectableProperty},
+    {::onc::network_config::kErrorState, shill::kErrorProperty},
     {::onc::network_config::kPriority, shill::kPriorityProperty},
 
     // Shill doesn't allow setting the name for non-VPN networks.
     // Name is conditionally translated, see onc_translator_*.
-    // {::onc::network_config::kName, shill::kNameProperty },
+    // { ::onc::network_config::kName, shill::kNameProperty },
 
     // Type is converted during translation, see onc_translator_*.
-    // {::onc::network_config::kType, shill::kTypeProperty },
+    // { ::onc::network_config::kType, shill::kTypeProperty },
 
     // These fields are converted during translation, see
     // onc_translator_shill_to_onc.cc. They are only converted when going from
     // Shill->ONC, and ignored otherwise.
-    // {::onc::network_config::kConnectionState, shill::kStateProperty },
-    // {::onc::network_config::kErrorState, shill::kErrorProperty},
-    // {::onc::network_config::kRestrictedConnectivity, shill::kStateProperty },
-    // {::onc::network_config::kSource, shill::kProfileProperty },
-    // {::onc::network_config::kMacAddress, shill::kAddressProperty },
+    // { ::onc::network_config::kConnectionState, shill::kStateProperty },
+    // { ::onc::network_config::kRestrictedConnectivity,
+    //   shill::kStateProperty },
+    // { ::onc::network_config::kSource, shill::kProfileProperty },
+    // { ::onc::network_config::kMacAddress, shill::kAddressProperty },
     {NULL}};
 
 const FieldTranslationEntry ipconfig_fields[] = {
diff --git a/chromeos/network/onc/onc_translator.h b/chromeos/network/onc/onc_translator.h
index db1cb71..dc5afa3 100644
--- a/chromeos/network/onc/onc_translator.h
+++ b/chromeos/network/onc/onc_translator.h
@@ -14,9 +14,6 @@ class DictionaryValue;
 }
 
 namespace chromeos {
-
-class NetworkState;
-
 namespace onc {
 
 struct OncValueSignature;
@@ -42,15 +39,12 @@ scoped_ptr<base::DictionaryValue> TranslateONCObjectToShill(
 // only a subset of it and includes only the values that are actually required
 // by the UI. If |onc_source| != ONC_SOURCE_UNKNOWN then the 'Source' property
 // of the ONC dictionary will be set accordingly. Note: ONC_SOURCE_USER_IMPORT
-// is treated the same as ONC_SOURCE_NONE. If |network_state| is provided it
-// will be used to set the ErrorState property. Otherwise ErrorState will not
-// be set.
+// is treated the same as ONC_SOURCE_NONE.
 CHROMEOS_EXPORT
 scoped_ptr<base::DictionaryValue> TranslateShillServiceToONCPart(
     const base::DictionaryValue& shill_dictionary,
     ::onc::ONCSource onc_source,
-    const OncValueSignature* onc_signature,
-    const NetworkState* network_state);
+    const OncValueSignature* onc_signature);
 
 }  // namespace onc
 }  // namespace chromeos
diff --git a/chromeos/network/onc/onc_translator_shill_to_onc.cc b/chromeos/network/onc/onc_translator_shill_to_onc.cc
index aca9133..d918156 100644
--- a/chromeos/network/onc/onc_translator_shill_to_onc.cc
+++ b/chromeos/network/onc/onc_translator_shill_to_onc.cc
@@ -52,25 +52,21 @@ class ShillToONCTranslator {
  public:
   ShillToONCTranslator(const base::DictionaryValue& shill_dictionary,
                        ::onc::ONCSource onc_source,
-                       const OncValueSignature& onc_signature,
-                       const NetworkState* network_state)
+                       const OncValueSignature& onc_signature)
       : shill_dictionary_(&shill_dictionary),
         onc_source_(onc_source),
-        onc_signature_(&onc_signature),
-        network_state_(network_state) {
+        onc_signature_(&onc_signature) {
     field_translation_table_ = GetFieldTranslationTable(onc_signature);
   }
 
   ShillToONCTranslator(const base::DictionaryValue& shill_dictionary,
                        ::onc::ONCSource onc_source,
                        const OncValueSignature& onc_signature,
-                       const FieldTranslationEntry* field_translation_table,
-                       const NetworkState* network_state)
+                       const FieldTranslationEntry* field_translation_table)
       : shill_dictionary_(&shill_dictionary),
         onc_source_(onc_source),
         onc_signature_(&onc_signature),
-        field_translation_table_(field_translation_table),
-        network_state_(network_state) {}
+        field_translation_table_(field_translation_table) {}
 
   // Translates the associated Shill dictionary and creates an ONC object of the
   // given signature.
@@ -146,7 +142,6 @@ class ShillToONCTranslator {
   const OncValueSignature* onc_signature_;
   const FieldTranslationEntry* field_translation_table_;
   scoped_ptr<base::DictionaryValue> onc_object_;
-  const NetworkState* network_state_;
 
   DISALLOW_COPY_AND_ASSIGN(ShillToONCTranslator);
 };
@@ -387,9 +382,9 @@ void ShillToONCTranslator::TranslateCellularWithState() {
   if (device_dictionary) {
     // Merge the Device dictionary with this one (Cellular) using the
     // CellularDevice signature.
-    ShillToONCTranslator nested_translator(
-        *device_dictionary, onc_source_, kCellularWithStateSignature,
-        kCellularDeviceTable, network_state_);
+    ShillToONCTranslator nested_translator(*device_dictionary, onc_source_,
+                                           kCellularWithStateSignature,
+                                           kCellularDeviceTable);
     scoped_ptr<base::DictionaryValue> nested_object =
         nested_translator.CreateTranslatedONCObject();
     onc_object_->MergeDictionary(nested_object.get());
@@ -479,17 +474,6 @@ void ShillToONCTranslator::TranslateNetworkWithState() {
     }
   }
 
-  // 'ErrorState' reflects the most recent error maintained in NetworkState
-  // (which may not match Shill's Error or PreviousError properties). Non
-  // visible networks (with null network_state_) do not set ErrorState.
-  if (network_state_) {
-    std::string error_state = network_state_->GetErrorState();
-    if (!error_state.empty()) {
-      onc_object_->SetStringWithoutPathExpansion(
-          ::onc::network_config::kErrorState, error_state);
-    }
-  }
-
   std::string profile_path;
   if (onc_source_ != ::onc::ONC_SOURCE_UNKNOWN &&
       shill_dictionary_->GetStringWithoutPathExpansion(shill::kProfileProperty,
@@ -617,8 +601,7 @@ void ShillToONCTranslator::TranslateAndAddNestedObject(
     return;
   }
   ShillToONCTranslator nested_translator(dictionary, onc_source_,
-                                         *field_signature->value_signature,
-                                         network_state_);
+                                         *field_signature->value_signature);
   scoped_ptr<base::DictionaryValue> nested_object =
       nested_translator.CreateTranslatedONCObject();
   if (nested_object->empty())
@@ -659,8 +642,7 @@ void ShillToONCTranslator::TranslateAndAddListOfObjects(
       continue;
     ShillToONCTranslator nested_translator(
         *shill_value, onc_source_,
-        *field_signature->value_signature->onc_array_entry_signature,
-        network_state_);
+        *field_signature->value_signature->onc_array_entry_signature);
     scoped_ptr<base::DictionaryValue> nested_object =
         nested_translator.CreateTranslatedONCObject();
     // If the nested object couldn't be parsed, simply omit it.
@@ -746,12 +728,10 @@ std::string ShillToONCTranslator::GetName() {
 scoped_ptr<base::DictionaryValue> TranslateShillServiceToONCPart(
     const base::DictionaryValue& shill_dictionary,
     ::onc::ONCSource onc_source,
-    const OncValueSignature* onc_signature,
-    const NetworkState* network_state) {
+    const OncValueSignature* onc_signature) {
   CHECK(onc_signature != NULL);
 
-  ShillToONCTranslator translator(shill_dictionary, onc_source, *onc_signature,
-                                  network_state);
+  ShillToONCTranslator translator(shill_dictionary, onc_source, *onc_signature);
   return translator.CreateTranslatedONCObject();
 }
 
diff --git a/chromeos/network/onc/onc_translator_unittest.cc b/chromeos/network/onc/onc_translator_unittest.cc
index 45701c6..f637761 100644
--- a/chromeos/network/onc/onc_translator_unittest.cc
+++ b/chromeos/network/onc/onc_translator_unittest.cc
@@ -83,8 +83,7 @@ TEST_P(ONCTranslatorShillToOncTest, Translate) {
       test_utils::ReadTestDictionary(result_onc_filename));
 
   scoped_ptr<base::DictionaryValue> translation(TranslateShillServiceToONCPart(
-      *shill_network, ::onc::ONC_SOURCE_NONE, &kNetworkWithStateSignature,
-      nullptr /* network_state */));
+      *shill_network, ::onc::ONC_SOURCE_NONE, &kNetworkWithStateSignature));
 
   EXPECT_TRUE(test_utils::Equals(expected_onc_network.get(),
                                  translation.get()));
diff --git a/chromeos/network/policy_applicator.cc b/chromeos/network/policy_applicator.cc
index a5dd9ff..53ff817 100644
--- a/chromeos/network/policy_applicator.cc
+++ b/chromeos/network/policy_applicator.cc
@@ -131,9 +131,9 @@ void PolicyApplicator::GetEntryCallback(
           << profile_.ToDebugString();
 
   scoped_ptr<base::DictionaryValue> onc_part(
-      onc::TranslateShillServiceToONCPart(
-          entry_properties, ::onc::ONC_SOURCE_UNKNOWN,
-          &onc::kNetworkWithStateSignature, nullptr /* network_state */));
+      onc::TranslateShillServiceToONCPart(entry_properties,
+                                          ::onc::ONC_SOURCE_UNKNOWN,
+                                          &onc::kNetworkWithStateSignature));
 
   std::string old_guid;
   if (!onc_part->GetStringWithoutPathExpansion(::onc::network_config::kGUID,