Do not report NEED_PERMISSION_TO_UPDATE for non-critical OOBE updates
As part of https://crbug.com/889948, Chrome OS OOBE will start properly
handling NEED_PERMISSION_TO_UPDATE update status (instead of treating it
as any other error) - it will display a message to the user informing
them that the update can cause overage changes on mobile network, and
ask their permission to proceed.
Currently, NEED_PERMISSION_TO_UPDATE status is reported over cellular
for non-critical updates during OOBE, even though these are later
ignored by the update engine. This can produce sub-optimal UX where the
user is prompted to proceed with update, even though the update gets
skipped when attempted.
This CL makes sure that kNonCriticalUpdateInOOBE error status has
precedence over kOmahaUpdateIgnoredOverCellular in omaha_request_action
(the latter one causes NEED_PERMISSION_TO_UPDATE status)
BUG=chromium:889948
TEST=run OOBE with update engine thinking it's on cellular network,
verify that the user sees prompt about possible overage
charges for critical updates only
(dut setup included patching update engine to treat ethernet connection
as cellular, and running devserver locally with and without
--critical_update flag)
Change-Id: If87b9477fd7f826892b1aeedc1017d11b1a0af32
Reviewed-on: https://chromium-review.googlesource.com/1277708
Commit-Ready: Toni Baržić <tbarzic@chromium.org>
Tested-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index c512031..759eeeb 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -1154,6 +1154,37 @@
EXPECT_FALSE(response.update_exists);
}
+// Verify that non-critical updates are skipped by reporting the
+// kNonCriticalUpdateInOOBE error code when attempted over cellular network -
+// i.e. when the update would need user permission. Note that reporting
+// kOmahaUpdateIgnoredOverCellular error in this case might cause undesired UX
+// in OOBE (warning the user about an update that will be skipped).
+TEST_F(OmahaRequestActionTest, SkipNonCriticalUpdatesInOOBEOverCellular) {
+ OmahaResponse response;
+ fake_system_state_.fake_hardware()->UnsetIsOOBEComplete();
+
+ MockConnectionManager mock_cm;
+ fake_system_state_.set_connection_manager(&mock_cm);
+
+ EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
+ .WillRepeatedly(DoAll(SetArgPointee<0>(ConnectionType::kCellular),
+ SetArgPointee<1>(ConnectionTethering::kUnknown),
+ Return(true)));
+ EXPECT_CALL(mock_cm, IsAllowedConnectionTypesForUpdateSet())
+ .WillRepeatedly(Return(false));
+
+ ASSERT_FALSE(TestUpdateCheck(fake_update_response_.GetUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kNonCriticalUpdateInOOBE,
+ metrics::CheckResult::kParsingError,
+ metrics::CheckReaction::kUnset,
+ metrics::DownloadErrorCode::kUnset,
+ &response,
+ nullptr));
+ EXPECT_FALSE(response.update_exists);
+}
+
TEST_F(OmahaRequestActionTest, WallClockBasedWaitAloneCausesScattering) {
OmahaResponse response;
request_params_.set_wall_clock_based_wait_enabled(true);