Fix update over cellular network on guest account

Device policy is not loaded for guest account. When a user check for
update over cellular network, the update engine does not allow so.

Changes:
Allow updates over cellular in IsUpdateAllowedOver() if device policy is
not loaded, because this should be treated the same as device policy
having not update setting.
Overwrite decision made by IsUpdateAllowedOver() with user preferences
if device policy is not loaded or does not have update setting.

BUG=chromium:799185
TEST=ConnectionManagerTest.AllowUpdatesOver3GIfPolicyFailsToBeLoaded
Change-Id: Ica96384f87a07a03caafbe000ee62edf8132ce6d
Reviewed-on: https://chromium-review.googlesource.com/851733
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Weidong Guo <weidongg@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>

3 files changed, 22 insertions, 6 deletions

diff --git a/connection_manager.cc b/connection_manager.cc
index f0a2c923..5085842e 100644
--- a/connection_manager.cc
+++ b/connection_manager.cc
@@ -74,9 +74,12 @@ bool ConnectionManager::IsUpdateAllowedOver(
       }
 
       if (!device_policy) {
-        LOG(INFO) << "Disabling updates over cellular as device policy "
+        // Device policy fails to be loaded (possibly due to guest account). We
+        // do not check the local user setting here, which should be checked by
+        // |OmahaRequestAction| during checking for update.
+        LOG(INFO) << "Allowing updates over cellular as device policy "
                      "fails to be loaded.";
-        return false;
+        return true;
       }
 
       if (device_policy->GetAllowedConnectionTypesForUpdate(&allowed_types)) {
@@ -88,11 +91,14 @@ bool ConnectionManager::IsUpdateAllowedOver(
         }
 
         LOG(INFO) << "Allowing updates over cellular per device policy.";
+        return true;
       }
 
       // If there's no update setting in the device policy, we do not check
       // the local user setting here, which should be checked by
       // |OmahaRequestAction| during checking for update.
+      LOG(INFO) << "Allowing updates over cellular as device policy does "
+                   "not include update setting.";
       return true;
     }
 
diff --git a/connection_manager_unittest.cc b/connection_manager_unittest.cc
index 547539bd..0005b2e0 100644
--- a/connection_manager_unittest.cc
+++ b/connection_manager_unittest.cc
@@ -334,6 +334,14 @@ TEST_F(ConnectionManagerTest, AllowUpdatesOver3GIfPolicyIsNotSet) {
                                         ConnectionTethering::kUnknown));
 }
 
+TEST_F(ConnectionManagerTest,
+       AllowUpdatesOverCellularIfPolicyFailsToBeLoaded) {
+  fake_system_state_.set_device_policy(nullptr);
+
+  EXPECT_TRUE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
+                                        ConnectionTethering::kUnknown));
+}
+
 TEST_F(ConnectionManagerTest, StringForConnectionTypeTest) {
   EXPECT_STREQ(shill::kTypeEthernet,
                StringForConnectionType(ConnectionType::kEthernet));
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 47f3e37d..ee9e9201 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -1621,10 +1621,12 @@ bool OmahaRequestAction::IsUpdateAllowedOverCurrentConnection(
     // is set regarding updates over cellular.
     if (!is_allowed)
       *error = ErrorCode::kOmahaUpdateIgnoredPerPolicy;
-  } else if (!IsUpdateAllowedOverCellularByPrefs(response)) {
-    // The user prefereces does not allow updates over cellular.
-    is_allowed = false;
-    *error = ErrorCode::kOmahaUpdateIgnoredOverCellular;
+  } else {
+    // Deivce policy is not set, so user preferences overwrite whether to
+    // allow updates over cellular.
+    is_allowed = IsUpdateAllowedOverCellularByPrefs(response);
+    if (!is_allowed)
+      *error = ErrorCode::kOmahaUpdateIgnoredOverCellular;
   }
 
   LOG(INFO) << "We are connected via "