arc: update proxy settings from UI and ONC policy.

chrome/browser/chromeos/arc/arc_settings_service.cc

Index: chrome/browser/chromeos/arc/arc_settings_service.cc
diff --git a/chrome/browser/chromeos/arc/arc_settings_service.cc b/chrome/browser/chromeos/arc/arc_settings_service.cc
index 7d2e4f4a0599fe255810ab0dcc4500f186ef062b..2d6e123ea0a58707cf3297fda5554598e62986b1 100644
--- a/chrome/browser/chromeos/arc/arc_settings_service.cc
+++ b/chrome/browser/chromeos/arc/arc_settings_service.cc
@@ -10,15 +10,23 @@
 #include "base/json/json_writer.h"
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/arc/arc_auth_service.h"
+#include "chrome/browser/chromeos/net/onc_utils.h"
+#include "chrome/browser/chromeos/proxy_config_service_impl.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/common/pref_names.h"
+#include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
+#include "chromeos/network/network_state_handler_observer.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "chromeos/settings/timezone_settings.h"
 #include "components/arc/intent_helper/font_size_util.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/prefs/pref_service.h"
+#include "components/proxy_config/pref_proxy_config_tracker_impl.h"
 #include "components/proxy_config/proxy_config_dictionary.h"
 #include "components/proxy_config/proxy_config_pref_names.h"
 #include "device/bluetooth/bluetooth_adapter.h"
@@ -30,11 +38,11 @@ using ::chromeos::system::TimezoneSettings;
 
 namespace {
 
-bool GetHttpProxyServer(const ProxyConfigDictionary& proxy_config_dict,
+bool GetHttpProxyServer(const ProxyConfigDictionary* proxy_config_dict,
                         std::string* host,
                         int* port) {
   std::string proxy_rules_string;
-  if (!proxy_config_dict.GetProxyServer(&proxy_rules_string))
+  if (!proxy_config_dict->GetProxyServer(&proxy_rules_string))
     return false;
 
   net::ProxyConfig::ProxyRules proxy_rules;
@@ -65,7 +73,8 @@ namespace arc {
 class ArcSettingsServiceImpl
     : public chromeos::system::TimezoneSettings::Observer,
       public device::BluetoothAdapter::Observer,
-      public ArcAuthService::Observer {
+      public ArcAuthService::Observer,
+      public chromeos::NetworkStateHandlerObserver {
  public:
   explicit ArcSettingsServiceImpl(ArcBridgeService* arc_bridge_service);
   ~ArcSettingsServiceImpl() override;
@@ -84,6 +93,9 @@ class ArcSettingsServiceImpl
   // ArcAuthService::Observer:
   void OnInitialStart() override;
 
+  // NetworkStateHandlerObserver:
+  void DefaultNetworkChanged(const chromeos::NetworkState* network) override;
+
  private:
   // Registers to observe changes for Chrome settings we care about.
   void StartObservingSettingsChanges();
@@ -168,8 +180,10 @@ void ArcSettingsServiceImpl::StartObservingSettingsChanges() {
   AddPrefToObserve(prefs::kWebKitMinimumFontSize);
   AddPrefToObserve(prefs::kAccessibilitySpokenFeedbackEnabled);
   AddPrefToObserve(prefs::kUse24HourClock);
-  AddPrefToObserve(proxy_config::prefs::kProxy);
   AddPrefToObserve(prefs::kArcBackupRestoreEnabled);
+  AddPrefToObserve(proxy_config::prefs::kProxy);
+  AddPrefToObserve(prefs::kDeviceOpenNetworkConfiguration);
+  AddPrefToObserve(prefs::kOpenNetworkConfiguration);
 
   reporting_consent_subscription_ = CrosSettings::Get()->AddSettingsObserver(
       chromeos::kStatsReportingPref,
@@ -183,6 +197,9 @@ void ArcSettingsServiceImpl::StartObservingSettingsChanges() {
         base::Bind(&ArcSettingsServiceImpl::OnBluetoothAdapterInitialized,
                    weak_factory_.GetWeakPtr()));
   }
+
+  chromeos::NetworkHandler::Get()->network_state_handler()->AddObserver(
+      this, FROM_HERE);
 }
 
 void ArcSettingsServiceImpl::OnBluetoothAdapterInitialized(
@@ -225,6 +242,8 @@ void ArcSettingsServiceImpl::StopObservingSettingsChanges() {
   reporting_consent_subscription_.reset();
 
   TimezoneSettings::GetInstance()->RemoveObserver(this);
+  chromeos::NetworkHandler::Get()->network_state_handler()->RemoveObserver(
+      this, FROM_HERE);
 }
 
 void ArcSettingsServiceImpl::AddPrefToObserve(const std::string& pref_name) {
@@ -252,6 +271,16 @@ void ArcSettingsServiceImpl::OnPrefChanged(const std::string& pref_name) const {
     SyncUse24HourClock();
   } else if (pref_name == proxy_config::prefs::kProxy) {
     SyncProxySettings();
+  } else if (pref_name == prefs::kDeviceOpenNetworkConfiguration ||
+             pref_name == prefs::kOpenNetworkConfiguration) {
+    // Do not update proxy settings if kProxy pref is applied.

[stevenjb, 2016/09/06 15:49:06]

Rephrase this: // Only update proxy settings if kProxy pref is applied.

[Polina Bondarenko, 2016/09/06 19:38:46]

Fixed to: "Only update proxy settings if kProxy pref is not applied."

+    net::ProxyConfig config;
+    Profile* profile = ProfileManager::GetActiveUserProfile();
+    if (!PrefProxyConfigTrackerImpl::PrefPrecedes(
+            PrefProxyConfigTrackerImpl::ReadPrefConfig(profile->GetPrefs(),
+                                                       &config))) {
+      SyncProxySettings();
+    }
   } else {
     LOG(ERROR) << "Unknown pref changed.";
   }
@@ -349,16 +378,14 @@ void ArcSettingsServiceImpl::SyncUse24HourClock() const {
 }
 
 void ArcSettingsServiceImpl::SyncProxySettings() const {
-  const PrefService::Preference* const pref =
-      registrar_.prefs()->FindPreference(proxy_config::prefs::kProxy);
-  const base::DictionaryValue* proxy_config_value;
-  bool value_exists = pref->GetValue()->GetAsDictionary(&proxy_config_value);
-  DCHECK(value_exists);
-
-  ProxyConfigDictionary proxy_config_dict(proxy_config_value);
+  std::unique_ptr<ProxyConfigDictionary> proxy_config_dict =
+      chromeos::ProxyConfigServiceImpl::GetDefaultProxyConfigDictionary(
+          ProfileManager::GetActiveUserProfile()->GetPrefs());
+  if (!proxy_config_dict)
+    return;
 
   ProxyPrefs::ProxyMode mode;
-  if (!proxy_config_dict.GetMode(&mode))
+  if (!proxy_config_dict || !proxy_config_dict->GetMode(&mode))
     mode = ProxyPrefs::MODE_DIRECT;
 
   base::DictionaryValue extras;
@@ -375,7 +402,7 @@ void ArcSettingsServiceImpl::SyncProxySettings() const {
       break;
     case ProxyPrefs::MODE_PAC_SCRIPT: {
       std::string pac_url;
-      if (!proxy_config_dict.GetPacUrl(&pac_url)) {
+      if (!proxy_config_dict->GetPacUrl(&pac_url)) {
         LOG(ERROR) << "No pac URL for pac_script proxy mode.";
         return;
       }
@@ -385,7 +412,7 @@ void ArcSettingsServiceImpl::SyncProxySettings() const {
     case ProxyPrefs::MODE_FIXED_SERVERS: {
       std::string host;
       int port = 0;
-      if (!GetHttpProxyServer(proxy_config_dict, &host, &port)) {
+      if (!GetHttpProxyServer(proxy_config_dict.get(), &host, &port)) {
         LOG(ERROR) << "No Http proxy server is sent.";
         return;
       }
@@ -393,7 +420,7 @@ void ArcSettingsServiceImpl::SyncProxySettings() const {
       extras.SetInteger("port", port);
 
       std::string bypass_list;
-      if (proxy_config_dict.GetBypassList(&bypass_list) &&
+      if (proxy_config_dict->GetBypassList(&bypass_list) &&
           !bypass_list.empty()) {
         extras.SetString("bypassList", bypass_list);
       }
@@ -438,6 +465,21 @@ void ArcSettingsServiceImpl::SendSettingsBroadcast(
   }
 }
 
+void ArcSettingsServiceImpl::DefaultNetworkChanged(
+    const chromeos::NetworkState* network) {
+  // kProxy pref and ONC policy have more priority than the default network
+  // update.
+  net::ProxyConfig config;
+  Profile* profile = ProfileManager::GetActiveUserProfile();
+  if (!chromeos::onc::HasPolicyForNetwork(
+          profile->GetPrefs(), g_browser_process->local_state(), *network) &&
+      !PrefProxyConfigTrackerImpl::PrefPrecedes(
+          PrefProxyConfigTrackerImpl::ReadPrefConfig(profile->GetPrefs(),
+                                                     &config))) {

[stevenjb, 2016/09/06 15:49:06]

Since we call PrefPrecedes twice, ignoring config, maybe wrap this call into a
helper function with a comment describing what we are checking (I am a little
fuzzy on that).

[Polina Bondarenko, 2016/09/06 19:38:46]

Wrapped into a local function.

+    SyncProxySettings();
+  }
+}
+
 ArcSettingsService::ArcSettingsService(ArcBridgeService* bridge_service)
     : ArcService(bridge_service) {
   arc_bridge_service()->intent_helper()->AddObserver(this);