AU: coalesce interactive / user-initiated flags
The latter was introduced recently but turns out it just carries
a duplicate meaning to the former, so we're eliminating it. This also
makes the case of a simulated scheduled update check more accurate, as
all of the effects of a scheduled update are made.
BUG=None
TEST=Unit tests; interactive/scheduled update check behaves as expected.
Change-Id: I8971aefcfc15cb76733059860832507e88795883
Reviewed-on: https://gerrit.chromium.org/gerrit/41082
Commit-Queue: Gilad Arnold <[email protected]>
Reviewed-by: Gilad Arnold <[email protected]>
Tested-by: Gilad Arnold <[email protected]>
diff --git a/dbus_service.cc b/dbus_service.cc
index 2d0f31b..7d3d39d 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -67,7 +67,7 @@
GError **error) {
string update_app_version;
string update_omaha_url;
- bool is_user_initiated = true;
+ bool interactive = true;
// Only non-official (e.g., dev and test) builds can override the current
// version and update server URL over D-Bus. However, pointing to the
@@ -86,17 +86,17 @@
// pretend that it's not user-initiated even though it is,
// so as to test scattering logic, etc. which get kicked off
// only in scheduled update checks.
- is_user_initiated = false;
+ interactive = false;
} else if (strcmp(omaha_url, kAUTestURLRequest) == 0) {
update_omaha_url = kAUTestURL;
}
}
LOG(INFO) << "Attempt update: app_version=\"" << update_app_version << "\" "
<< "omaha_url=\"" << update_omaha_url << "\" "
- << "is_user_initiated=" << (is_user_initiated? "yes" : "no");
+ << "interactive=" << (interactive? "yes" : "no");
self->update_attempter_->CheckForUpdate(update_app_version,
update_omaha_url,
- is_user_initiated);
+ interactive);
return TRUE;
}
diff --git a/update_attempter.cc b/update_attempter.cc
index b2cbe4c..1feda05 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -148,8 +148,7 @@
const string& omaha_url,
bool obey_proxies,
bool interactive,
- bool is_test_mode,
- bool is_user_initiated) {
+ bool is_test_mode) {
chrome_proxy_resolver_.Init();
fake_update_success_ = false;
if (status_ == UPDATE_STATUS_UPDATED_NEED_REBOOT) {
@@ -169,8 +168,7 @@
omaha_url,
obey_proxies,
interactive,
- is_test_mode,
- is_user_initiated)) {
+ is_test_mode)) {
return;
}
@@ -189,8 +187,7 @@
const string& omaha_url,
bool obey_proxies,
bool interactive,
- bool is_test_mode,
- bool is_user_initiated) {
+ bool is_test_mode) {
http_response_code_ = 0;
// Set the test mode flag for the current update attempt.
@@ -231,7 +228,7 @@
system_state_->set_device_policy(NULL);
}
- CalculateScatteringParams(is_user_initiated);
+ CalculateScatteringParams(interactive);
// Determine whether an alternative test address should be used.
string omaha_url_to_use = omaha_url;
@@ -283,7 +280,7 @@
return true;
}
-void UpdateAttempter::CalculateScatteringParams(bool is_user_initiated) {
+void UpdateAttempter::CalculateScatteringParams(bool interactive) {
// Take a copy of the old scatter value before we update it, as
// we need to update the waiting period if this value changes.
TimeDelta old_scatter_factor = scatter_factor_;
@@ -299,8 +296,8 @@
bool is_scatter_enabled = false;
if (scatter_factor_.InSeconds() == 0) {
LOG(INFO) << "Scattering disabled since scatter factor is set to 0";
- } else if (is_user_initiated) {
- LOG(INFO) << "Scattering disabled as this update check is user-initiated";
+ } else if (interactive) {
+ LOG(INFO) << "Scattering disabled as this is an interactive update check";
} else if (!system_state_->IsOOBEComplete()) {
LOG(INFO) << "Scattering disabled since OOBE is not complete yet";
} else {
@@ -510,7 +507,7 @@
void UpdateAttempter::CheckForUpdate(const string& app_version,
const string& omaha_url,
- bool is_user_initiated) {
+ bool interactive) {
LOG(INFO) << "New update check requested";
if (status_ != UPDATE_STATUS_IDLE) {
@@ -529,9 +526,9 @@
is_test_update_attempted_ = true;
}
- // Passing true for is_user_initiated to indicate that this
- // is not a scheduled update check.
- Update(app_version, omaha_url, true, true, is_test_mode, is_user_initiated);
+ // Pass through the interactive flag, in case we want to simulate a scheduled
+ // test.
+ Update(app_version, omaha_url, true, interactive, is_test_mode);
}
bool UpdateAttempter::RebootIfNeeded() {
diff --git a/update_attempter.h b/update_attempter.h
index c42715f..79c4c65 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -76,8 +76,7 @@
const std::string& omaha_url,
bool obey_proxies,
bool interactive,
- bool is_test_mode,
- bool is_user_initiated);
+ bool is_test_mode);
// ActionProcessorDelegate methods:
void ProcessingDone(const ActionProcessor* processor, ActionExitCode code);
@@ -141,7 +140,7 @@
// This is called by the DBus implementation.
void CheckForUpdate(const std::string& app_version,
const std::string& omaha_url,
- bool is_user_initiated);
+ bool is_interactive);
// Initiates a reboot if the current state is
// UPDATED_NEED_REBOOT. Returns true on sucess, false otherwise.
@@ -246,14 +245,13 @@
const std::string& omaha_url,
bool obey_proxies,
bool interactive,
- bool is_test,
- bool is_user_initiated);
+ bool is_test);
// Calculates all the scattering related parameters (such as waiting period,
// which type of scattering is enabled, etc.) and also updates/deletes
// the corresponding prefs file used in scattering. Should be called
// only after the device policy has been loaded and set in the system_state_.
- void CalculateScatteringParams(bool is_user_initiated);
+ void CalculateScatteringParams(bool is_interactive);
// Sets a random value for the omaha_request_params_.waiting_period
// based on the current scatter_factor_ value.
diff --git a/update_attempter_mock.h b/update_attempter_mock.h
index 782277a..693cc57 100644
--- a/update_attempter_mock.h
+++ b/update_attempter_mock.h
@@ -21,12 +21,11 @@
MockDbusGlib* dbus)
: UpdateAttempter(mock_system_state, dbus) {}
- MOCK_METHOD6(Update, void(const std::string& app_version,
+ MOCK_METHOD5(Update, void(const std::string& app_version,
const std::string& omaha_url,
bool obey_proxies,
bool interactive,
- bool is_test,
- bool is_user_initiated));
+ bool is_test));
};
} // namespace chromeos_update_engine
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 18bcb67..39ad109 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -373,7 +373,7 @@
}
EXPECT_CALL(*processor_, StartProcessing()).Times(1);
- attempter_.Update("", "", false, false, false, false);
+ attempter_.Update("", "", false, false, false);
g_idle_add(&StaticUpdateTestVerify, this);
}
@@ -474,7 +474,7 @@
SetArgumentPointee<0>(std::string("canary-channel")),
Return(true)));
- attempter_.Update("", "", false, false, false, false);
+ attempter_.Update("", "", false, false, false);
EXPECT_EQ("canary-channel", attempter_.omaha_request_params_.app_track);
g_idle_add(&StaticQuitMainLoop, this);
@@ -502,7 +502,7 @@
SetArgumentPointee<0>(true),
Return(true)));
- attempter_.Update("", "", false, false, false, false);
+ attempter_.Update("", "", false, false, false);
EXPECT_TRUE(attempter_.omaha_request_params_.update_disabled);
g_idle_add(&StaticQuitMainLoop, this);
@@ -532,7 +532,7 @@
SetArgumentPointee<0>(target_version_prefix),
Return(true)));
- attempter_.Update("", "", false, false, false, false);
+ attempter_.Update("", "", false, false, false);
EXPECT_EQ(target_version_prefix.c_str(),
attempter_.omaha_request_params_.target_version_prefix);
@@ -565,7 +565,7 @@
SetArgumentPointee<0>(scatter_factor_in_seconds),
Return(true)));
- attempter_.Update("", "", false, false, false, false);
+ attempter_.Update("", "", false, false, false);
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
g_idle_add(&StaticQuitMainLoop, this);
@@ -612,7 +612,7 @@
SetArgumentPointee<0>(scatter_factor_in_seconds),
Return(true)));
- attempter_.Update("", "", false, false, false, false);
+ attempter_.Update("", "", false, false, false);
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
// Make sure the file still exists.
@@ -627,7 +627,7 @@
// However, if the count is already 0, it's not decremented. Test that.
initial_value = 0;
EXPECT_TRUE(prefs.SetInt64(kPrefsUpdateCheckCount, initial_value));
- attempter_.Update("", "", false, false, false, false);
+ attempter_.Update("", "", false, false, false);
EXPECT_TRUE(prefs.Exists(kPrefsUpdateCheckCount));
EXPECT_TRUE(prefs.GetInt64(kPrefsUpdateCheckCount, &new_value));
EXPECT_EQ(initial_value, new_value);
@@ -679,9 +679,8 @@
SetArgumentPointee<0>(scatter_factor_in_seconds),
Return(true)));
- // pass true for is_user_initiated so we can test that the
- // scattering is disabled.
- attempter_.Update("", "", false, false, false, true);
+ // Trigger an interactive check so we can test that scattering is disabled.
+ attempter_.Update("", "", false, true, false);
EXPECT_EQ(scatter_factor_in_seconds, attempter_.scatter_factor_.InSeconds());
// Make sure scattering is disabled for manual (i.e. user initiated) update
diff --git a/update_check_scheduler.cc b/update_check_scheduler.cc
index 227816d..e77b5bb 100644
--- a/update_check_scheduler.cc
+++ b/update_check_scheduler.cc
@@ -102,7 +102,7 @@
// Before updating, we flush any previously generated UMA reports.
CertificateChecker::FlushReport();
- me->update_attempter_->Update("", "", false, false, is_test_mode, false);
+ me->update_attempter_->Update("", "", false, false, is_test_mode);
} else {
// Skips all automatic update checks if the OOBE process is not complete and
// schedules a new check as if it is the first one.
diff --git a/update_check_scheduler_unittest.cc b/update_check_scheduler_unittest.cc
index b751efa..c9e0447 100644
--- a/update_check_scheduler_unittest.cc
+++ b/update_check_scheduler_unittest.cc
@@ -287,7 +287,7 @@
EXPECT_TRUE(scheduler_.mock_system_state_ != NULL);
EXPECT_CALL(*scheduler_.mock_system_state_,
IsOOBEComplete()).Times(1).WillOnce(Return(true));
- EXPECT_CALL(attempter_, Update("", "", false, false, false, false))
+ EXPECT_CALL(attempter_, Update("", "", false, false, false))
.Times(1)
.WillOnce(Assign(&scheduler_.scheduled_, true));
scheduler_.enabled_ = true;
@@ -299,7 +299,7 @@
scheduler_.scheduled_ = true;
EXPECT_CALL(*scheduler_.mock_system_state_,
IsOOBEComplete()).Times(1).WillOnce(Return(false));
- EXPECT_CALL(attempter_, Update("", "", _, _, _,_)).Times(0);
+ EXPECT_CALL(attempter_, Update("", "", _, _, _)).Times(0);
int interval_min, interval_max;
FuzzRange(UpdateCheckScheduler::kTimeoutInitialInterval,
UpdateCheckScheduler::kTimeoutRegularFuzz,
