Network config: Only set AutoConnect to false

Setting 'AutoConnect' to true during configuration causes Shill to
attempt to connect immediately after configuration which breaks
the notification logic. By default Shill will set AutoConenct to
true on the first successful connect if not explicitly set to false.

This also adds yet more / better logging.

Bug: 809902
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ibace61a761b7adf7b5c87e8db93d54d3b3942a48
Reviewed-on: https://chromium-review.googlesource.com/940338
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539896}

chrome/browser/ui/ash/network/network_state_notifier.cc

@@ -172,7 +172,11 @@ void NetworkStateNotifier::ConnectFailed(const std::string& service_path,
   const NetworkState* network =
       NetworkHandler::Get()->network_state_handler()->GetNetworkState(
           service_path);
-  if (ShouldConnectFailedNotificationBeShown(error_name, network))
+  if (!ShouldConnectFailedNotificationBeShown(error_name, network)) {
+    NET_LOG(EVENT) << "Skipping notification for: " << service_path
+                   << " Error: " << error_name;
+    return;
+  }
   ShowNetworkConnectErrorForGuid(error_name, network ? network->guid() : "");
 }
 
@@ -348,6 +352,7 @@ void NetworkStateNotifier::ConnectErrorPropertiesSucceeded(
   shill_properties.GetStringWithoutPathExpansion(shill::kStateProperty, &state);
   if (NetworkState::StateIsConnected(state) ||
       NetworkState::StateIsConnecting(state)) {
+    NET_LOG(EVENT) << "Skipping connect error notification. State: " << state;
     // Network is no longer in an error state. This can happen if an
     // unexpected idle state transition occurs, see http://crbug.com/333955.
     return;

chromeos/network/network_connection_handler_impl.cc

@@ -237,7 +237,7 @@ void NetworkConnectionHandlerImpl::ConnectToNetwork(
   // Clear any existing queued connect request.
   queued_connect_.reset();
   if (HasConnectingNetwork(service_path)) {
-    NET_LOG_USER("Connect Request While Pending", service_path);
+    NET_LOG(USER) << "Connect Request while pending: " << service_path;
     InvokeConnectErrorCallback(service_path, error_callback, kErrorConnecting);
     return;
   }
@@ -252,6 +252,7 @@ void NetworkConnectionHandlerImpl::ConnectToNetwork(
   if (network) {
     // For existing networks, perform some immediate consistency checks.
     if (network->IsConnectedState()) {
+      NET_LOG(ERROR) << "Connect Request while connected: " << service_path;
       InvokeConnectErrorCallback(service_path, error_callback, kErrorConnected);
       return;
     }
@@ -742,10 +743,10 @@ void NetworkConnectionHandlerImpl::HandleShillConnectFailure(
   } else if (dbus_error_name == shill::kErrorResultInProgress) {
     error = kErrorConnecting;
   } else {
-    NET_LOG_ERROR("Connect Failure, Shill error: " + dbus_error_name,
-                  service_path);
     error = kErrorConnectFailed;
   }
+  NET_LOG(ERROR) << "Connect Failure: " << service_path << " Error: " << error
+                 << " Shill error: " << dbus_error_name;
   InvokeConnectErrorCallback(service_path, error_callback, error);
 }
 

ui/webui/resources/cr_components/chromeos/network/network_config.js

@@ -368,14 +368,14 @@ Polymer({
 
     var propertiesToSet = this.getPropertiesToSet_();
     if (this.getSource_() == CrOnc.Source.NONE) {
-      // New non VPN network configurations default to 'AutoConnect' unless
-      // prohibited by policy.
-      var prohibitAutoConnect = this.globalPolicy &&
-          this.globalPolicy.AllowOnlyPolicyNetworksToConnect;
-      CrOnc.setTypeProperty(
-          propertiesToSet, 'AutoConnect',
-          this.type != CrOnc.Type.VPN && !prohibitAutoConnect);
-
+      // Set 'AutoConnect' to false for VPN or if prohibited by policy.
+      // Note: Do not set AutoConnect to true, the connection manager will do
+      // that on a successful connection (unless set to false here).
+      if (this.type == CrOnc.Type.VPN ||
+          (this.globalPolicy &&
+           this.globalPolicy.AllowOnlyPolicyNetworksToConnect)) {
+        CrOnc.setTypeProperty(propertiesToSet, 'AutoConnect', false);
+      }
       // Create the configuration, then connect to it in the callback.
       this.networkingPrivate.createNetwork(
           this.shareNetwork_, propertiesToSet,