Add support to update_engine to poke Omaha after an update has been applied
successfully and is awaiting reboot to help ensure the number of actives
remains accurate.
BUG=chromium-os:12026
TEST=Manual test, unit tests
Change-Id: Ie3397264b0b34e8d423fb9748970f7d330122180
Review URL: http://codereview.chromium.org/6836025
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index a02387e..f75e443 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -83,7 +83,7 @@
string ping_active = GetPingAttribute("a", ping_active_days);
string ping_roll_call = GetPingAttribute("r", ping_roll_call_days);
if (!ping_active.empty() || !ping_roll_call.empty()) {
- return StringPrintf(" <o:ping%s%s></o:ping>\n",
+ return StringPrintf(" <o:ping active=\"1\"%s%s></o:ping>\n",
ping_active.c_str(),
ping_roll_call.c_str());
}
@@ -92,13 +92,15 @@
string FormatRequest(const OmahaEvent* event,
const OmahaRequestParams& params,
+ bool ping_only,
int ping_active_days,
int ping_roll_call_days,
PrefsInterface* prefs) {
string body;
if (event == NULL) {
- body = GetPingBody(ping_active_days, ping_roll_call_days) +
- " <o:updatecheck></o:updatecheck>\n";
+ body = GetPingBody(ping_active_days, ping_roll_call_days);
+ if (!ping_only)
+ body += " <o:updatecheck></o:updatecheck>\n";
// If this is the first update check after a reboot following a previous
// update, generate an event containing the previous version number. If the
// previous version preference file doesn't exist the event is still
@@ -173,11 +175,13 @@
OmahaRequestAction::OmahaRequestAction(PrefsInterface* prefs,
const OmahaRequestParams& params,
OmahaEvent* event,
- HttpFetcher* http_fetcher)
+ HttpFetcher* http_fetcher,
+ bool ping_only)
: prefs_(prefs),
params_(params),
event_(event),
http_fetcher_(http_fetcher),
+ ping_only_(ping_only),
ping_active_days_(0),
ping_roll_call_days_(0),
should_skip_(false) {}
@@ -225,6 +229,7 @@
InitPingDays();
string request_post(FormatRequest(event_.get(),
params_,
+ ping_only_,
ping_active_days_,
ping_roll_call_days_,
prefs_));
@@ -373,12 +378,6 @@
kActionCodeOmahaRequestHTTPResponseBase + code));
return;
}
- if (!HasOutputPipe()) {
- // Just set success to whether or not the http transfer succeeded,
- // which must be true at this point in the code.
- completer.set_code(kActionCodeSuccess);
- return;
- }
// parse our response and fill the fields in the output object
scoped_ptr_malloc<xmlDoc, ScopedPtrXmlDocFree> doc(
@@ -401,6 +400,13 @@
<< "Failed to update the last ping day preferences!";
}
+ if (!HasOutputPipe()) {
+ // Just set success to whether or not the http transfer succeeded,
+ // which must be true at this point in the code.
+ completer.set_code(kActionCodeSuccess);
+ return;
+ }
+
static const char* kNamespace("x");
static const char* kUpdatecheckNodeXpath("/x:gupdate/x:app/x:updatecheck");
static const char* kNsUrl("http://www.google.com/update2/response");
diff --git a/omaha_request_action.h b/omaha_request_action.h
index 8749546..fd7f548 100644
--- a/omaha_request_action.h
+++ b/omaha_request_action.h
@@ -129,7 +129,8 @@
OmahaRequestAction(PrefsInterface* prefs,
const OmahaRequestParams& params,
OmahaEvent* event,
- HttpFetcher* http_fetcher);
+ HttpFetcher* http_fetcher,
+ bool ping_only);
virtual ~OmahaRequestAction();
typedef ActionTraits<OmahaRequestAction>::InputObjectType InputObjectType;
typedef ActionTraits<OmahaRequestAction>::OutputObjectType OutputObjectType;
@@ -174,6 +175,9 @@
// pointer to the HttpFetcher that does the http work
scoped_ptr<HttpFetcher> http_fetcher_;
+ // If true, only include the <ping> element in the request.
+ bool ping_only_;
+
// Stores the response from the omaha server
std::vector<char> response_buffer_;
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 409e947..3bf02e2 100755
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -169,7 +169,8 @@
OmahaRequestAction action(prefs ? prefs : &local_prefs,
params,
NULL,
- fetcher);
+ fetcher,
+ false);
OmahaRequestActionTestProcessorDelegate delegate;
delegate.loop_ = loop;
delegate.expected_code_ = expected_code;
@@ -204,7 +205,7 @@
http_response.size(),
NULL);
NiceMock<PrefsMock> prefs;
- OmahaRequestAction action(&prefs, params, event, fetcher);
+ OmahaRequestAction action(&prefs, params, event, fetcher, false);
OmahaRequestActionTestProcessorDelegate delegate;
delegate.loop_ = loop;
ActionProcessor processor;
@@ -270,7 +271,8 @@
OmahaRequestAction action(&prefs, kDefaultTestParams, NULL,
new MockHttpFetcher(http_response.data(),
http_response.size(),
- NULL));
+ NULL),
+ false);
OmahaRequestActionTestProcessorDelegate delegate;
delegate.loop_ = loop;
ActionProcessor processor;
@@ -295,7 +297,8 @@
fetcher->set_never_use(true);
OmahaRequestAction action(&prefs, kDefaultTestParams,
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
- fetcher); // Passes fetcher ownership
+ fetcher, // Passes fetcher ownership
+ false);
action.set_should_skip(true);
OmahaRequestActionTestProcessorDelegate delegate;
delegate.loop_ = loop;
@@ -444,7 +447,8 @@
OmahaRequestAction action(&prefs, kDefaultTestParams, NULL,
new MockHttpFetcher(http_response.data(),
http_response.size(),
- NULL));
+ NULL),
+ false);
TerminateEarlyTestProcessorDelegate delegate;
delegate.loop_ = loop;
ActionProcessor processor;
@@ -560,8 +564,9 @@
&post_data));
// convert post_data to string
string post_str(&post_data[0], post_data.size());
- EXPECT_NE(post_str.find(" <o:ping a=\"-1\" r=\"-1\"></o:ping>\n"
- " <o:updatecheck></o:updatecheck>\n"),
+ EXPECT_NE(post_str.find(
+ " <o:ping active=\"1\" a=\"-1\" r=\"-1\"></o:ping>\n"
+ " <o:updatecheck></o:updatecheck>\n"),
string::npos);
EXPECT_NE(post_str.find("hardware_class=\"OEM MODEL 09235 7471\""),
string::npos);
@@ -584,8 +589,9 @@
&post_data));
// convert post_data to string
string post_str(&post_data[0], post_data.size());
- EXPECT_NE(post_str.find(" <o:ping a=\"-1\" r=\"-1\"></o:ping>\n"
- " <o:updatecheck></o:updatecheck>\n"),
+ EXPECT_NE(post_str.find(
+ " <o:ping active=\"1\" a=\"-1\" r=\"-1\"></o:ping>\n"
+ " <o:updatecheck></o:updatecheck>\n"),
string::npos);
EXPECT_NE(post_str.find("hardware_class=\"OEM MODEL 09235 7471\""),
string::npos);
@@ -643,7 +649,8 @@
NULL,
new MockHttpFetcher(http_response.data(),
http_response.size(),
- NULL));
+ NULL),
+ false);
EXPECT_FALSE(update_check_action.IsEvent());
OmahaRequestAction event_action(
@@ -652,7 +659,8 @@
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
new MockHttpFetcher(http_response.data(),
http_response.size(),
- NULL));
+ NULL),
+ false);
EXPECT_TRUE(event_action.IsEvent());
}
@@ -727,7 +735,8 @@
NULL,
&post_data));
string post_str(&post_data[0], post_data.size());
- EXPECT_NE(post_str.find("<o:ping a=\"6\" r=\"5\"></o:ping>"), string::npos);
+ EXPECT_NE(post_str.find("<o:ping active=\"1\" a=\"6\" r=\"5\"></o:ping>"),
+ string::npos);
}
TEST(OmahaRequestActionTest, ActivePingTest) {
@@ -749,7 +758,8 @@
NULL,
&post_data));
string post_str(&post_data[0], post_data.size());
- EXPECT_NE(post_str.find("<o:ping a=\"3\"></o:ping>"), string::npos);
+ EXPECT_NE(post_str.find("<o:ping active=\"1\" a=\"3\"></o:ping>"),
+ string::npos);
}
TEST(OmahaRequestActionTest, RollCallPingTest) {
@@ -771,7 +781,8 @@
NULL,
&post_data));
string post_str(&post_data[0], post_data.size());
- EXPECT_NE(post_str.find("<o:ping r=\"4\"></o:ping>\n"), string::npos);
+ EXPECT_NE(post_str.find("<o:ping active=\"1\" r=\"4\"></o:ping>\n"),
+ string::npos);
}
TEST(OmahaRequestActionTest, NoPingTest) {
diff --git a/update_attempter.cc b/update_attempter.cc
index e1ab0b1..61c9947 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -131,8 +131,11 @@
chrome_proxy_resolver_.Init();
UpdateBootFlags(); // Just in case we didn't do this yet.
if (status_ == UPDATE_STATUS_UPDATED_NEED_REBOOT) {
+ // Although we have applied an update, we still want to ping Omaha
+ // to ensure the number of active statistics is accurate.
LOG(INFO) << "Not updating b/c we already updated and we're waiting for "
- << "reboot";
+ << "reboot, we'll ping Omaha instead";
+ PingOmaha();
return;
}
if (status_ != UPDATE_STATUS_IDLE) {
@@ -176,7 +179,8 @@
new OmahaRequestAction(prefs_,
omaha_request_params_,
NULL,
- update_check_fetcher)); // passes ownership
+ update_check_fetcher, // passes ownership
+ false));
shared_ptr<OmahaResponseHandlerAction> response_handler_action(
new OmahaResponseHandlerAction(prefs_));
shared_ptr<FilesystemCopierAction> filesystem_copier_action(
@@ -188,7 +192,8 @@
omaha_request_params_,
new OmahaEvent(
OmahaEvent::kTypeUpdateDownloadStarted),
- new LibcurlHttpFetcher(GetProxyResolver())));
+ new LibcurlHttpFetcher(GetProxyResolver()),
+ false));
shared_ptr<DownloadAction> download_action(
new DownloadAction(prefs_, new MultiRangeHTTPFetcher(
new LibcurlHttpFetcher(GetProxyResolver()))));
@@ -202,7 +207,8 @@
OmahaEvent::kTypeUpdateDownloadFinished,
OmahaEvent::kResultError,
kActionCodeDownloadPayloadPubKeyVerificationError),
- new LibcurlHttpFetcher(GetProxyResolver())));
+ new LibcurlHttpFetcher(GetProxyResolver()),
+ false));
download_action->set_skip_reporting_signature_fail(
NewPermanentCallback(download_signature_warning.get(),
&OmahaRequestAction::set_should_skip,
@@ -212,7 +218,8 @@
omaha_request_params_,
new OmahaEvent(
OmahaEvent::kTypeUpdateDownloadFinished),
- new LibcurlHttpFetcher(GetProxyResolver())));
+ new LibcurlHttpFetcher(GetProxyResolver()),
+ false));
shared_ptr<FilesystemCopierAction> filesystem_verifier_action(
new FilesystemCopierAction(false, true));
shared_ptr<FilesystemCopierAction> kernel_filesystem_verifier_action(
@@ -223,7 +230,8 @@
new OmahaRequestAction(prefs_,
omaha_request_params_,
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
- new LibcurlHttpFetcher(GetProxyResolver())));
+ new LibcurlHttpFetcher(GetProxyResolver()),
+ false));
download_action->set_delegate(this);
response_handler_action_ = response_handler_action;
@@ -517,7 +525,8 @@
new OmahaRequestAction(prefs_,
omaha_request_params_,
error_event_.release(), // Pass ownership.
- new LibcurlHttpFetcher(GetProxyResolver())));
+ new LibcurlHttpFetcher(GetProxyResolver()),
+ false));
actions_.push_back(shared_ptr<AbstractAction>(error_event_action));
processor_->EnqueueAction(error_event_action.get());
SetStatusAndNotify(UPDATE_STATUS_REPORTING_ERROR_EVENT);
@@ -618,4 +627,19 @@
}
}
+void UpdateAttempter::PingOmaha() {
+ shared_ptr<OmahaRequestAction> ping_action(
+ new OmahaRequestAction(prefs_,
+ omaha_request_params_,
+ NULL,
+ new LibcurlHttpFetcher(GetProxyResolver()),
+ true));
+ actions_.push_back(shared_ptr<OmahaRequestAction>(ping_action));
+ CHECK(!processor_->IsRunning());
+ processor_->set_delegate(NULL);
+ processor_->EnqueueAction(ping_action.get());
+ g_idle_add(&StaticStartProcessing, this);
+ SetStatusAndNotify(UPDATE_STATUS_UPDATED_NEED_REBOOT);
+}
+
} // namespace chromeos_update_engine
diff --git a/update_attempter.h b/update_attempter.h
index 8a0bd26..d92ff97 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -127,6 +127,7 @@
FRIEND_TEST(UpdateAttempterTest, ScheduleErrorEventActionNoEventTest);
FRIEND_TEST(UpdateAttempterTest, ScheduleErrorEventActionTest);
FRIEND_TEST(UpdateAttempterTest, UpdateTest);
+ FRIEND_TEST(UpdateAttempterTest, PingOmahaTest);
// Sets the status to the given status and notifies a status update
// over dbus.
@@ -179,6 +180,13 @@
reinterpret_cast<ProxyResolver*>(&direct_proxy_resolver_);
}
+ // Sends a ping to Omaha.
+ // This is used after an update has been applied and we're waiting for the
+ // user to reboot. This ping helps keep the number of actives count
+ // accurate in case a user takes a long time to reboot the device after an
+ // update has been applied.
+ void PingOmaha();
+
// Last status notification timestamp used for throttling. Use monotonic
// TimeTicks to ensure that notifications are sent even if the system clock is
// set back in the middle of an update.
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 2c5c33c..a516695 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -64,6 +64,10 @@
void UpdateTestVerify();
static gboolean StaticUpdateTestStart(gpointer data);
static gboolean StaticUpdateTestVerify(gpointer data);
+ void PingOmahaTestStart();
+ void PingOmahaTestDone();
+ static gboolean StaticPingOmahaTestStart(gpointer data);
+ static gboolean StaticPingOmahaTestDone(gpointer data);
MockDbusGlib dbus_;
UpdateAttempterUnderTest attempter_;
@@ -97,7 +101,7 @@
scoped_ptr<MockHttpFetcher> fetcher(new MockHttpFetcher("", 0, NULL));
fetcher->FailTransfer(500); // Sets the HTTP response code.
OmahaRequestParams params;
- OmahaRequestAction action(&prefs_, params, NULL, fetcher.release());
+ OmahaRequestAction action(&prefs_, params, NULL, fetcher.release(), false);
ObjectCollectorAction<OmahaResponse> collector_action;
BondActions(&action, &collector_action);
OmahaResponse response;
@@ -130,7 +134,7 @@
GetErrorCodeForAction(NULL, kActionCodeSuccess));
OmahaRequestParams params;
- OmahaRequestAction omaha_request_action(NULL, params, NULL, NULL);
+ OmahaRequestAction omaha_request_action(NULL, params, NULL, NULL, false);
EXPECT_EQ(kActionCodeOmahaRequestError,
GetErrorCodeForAction(&omaha_request_action, kActionCodeError));
OmahaResponseHandlerAction omaha_response_handler_action(&prefs_);
@@ -243,6 +247,16 @@
return FALSE;
}
+gboolean UpdateAttempterTest::StaticPingOmahaTestStart(gpointer data) {
+ reinterpret_cast<UpdateAttempterTest*>(data)->PingOmahaTestStart();
+ return FALSE;
+}
+
+gboolean UpdateAttempterTest::StaticPingOmahaTestDone(gpointer data) {
+ reinterpret_cast<UpdateAttempterTest*>(data)->PingOmahaTestDone();
+ return FALSE;
+}
+
namespace {
const string kActionTypes[] = {
OmahaRequestAction::StaticType(),
@@ -299,4 +313,32 @@
loop_ = NULL;
}
+void UpdateAttempterTest::PingOmahaTestStart() {
+ EXPECT_CALL(*processor_,
+ EnqueueAction(Property(&AbstractAction::Type,
+ OmahaRequestAction::StaticType())))
+ .Times(1);
+ EXPECT_CALL(*processor_, StartProcessing()).Times(1);
+ attempter_.PingOmaha();
+ g_idle_add(&StaticPingOmahaTestDone, this);
+}
+
+void UpdateAttempterTest::PingOmahaTestDone() {
+ g_main_loop_quit(loop_);
+}
+
+TEST_F(UpdateAttempterTest, PingOmahaTest) {
+ UpdateCheckScheduler scheduler(&attempter_);
+ scheduler.enabled_ = true;
+ EXPECT_EQ(false, scheduler.scheduled_);
+ attempter_.set_update_check_scheduler(&scheduler);
+ loop_ = g_main_loop_new(g_main_context_default(), FALSE);
+ g_idle_add(&StaticPingOmahaTestStart, this);
+ g_main_loop_run(loop_);
+ g_main_loop_unref(loop_);
+ loop_ = NULL;
+ EXPECT_EQ(UPDATE_STATUS_UPDATED_NEED_REBOOT, attempter_.status());
+ EXPECT_EQ(true, scheduler.scheduled_);
+}
+
} // namespace chromeos_update_engine
diff --git a/update_check_scheduler.cc b/update_check_scheduler.cc
index 16f1929..f9b45bd 100644
--- a/update_check_scheduler.cc
+++ b/update_check_scheduler.cc
@@ -137,7 +137,11 @@
}
void UpdateCheckScheduler::SetUpdateStatus(UpdateStatus status) {
- if (status == UPDATE_STATUS_IDLE) {
+ // We want to schedule the update checks for when we're idle as well as
+ // after we've successfully applied an update and waiting for the user
+ // to reboot to ensure our active count is accurate.
+ if (status == UPDATE_STATUS_IDLE ||
+ status == UPDATE_STATUS_UPDATED_NEED_REBOOT) {
ScheduleNextCheck();
}
}
diff --git a/update_check_scheduler.h b/update_check_scheduler.h
index 1811291..8239c0d 100644
--- a/update_check_scheduler.h
+++ b/update_check_scheduler.h
@@ -76,6 +76,7 @@
FRIEND_TEST(UpdateCheckSchedulerTest, SetUpdateStatusNonIdleTest);
FRIEND_TEST(UpdateCheckSchedulerTest, StaticCheckOOBECompleteTest);
FRIEND_TEST(UpdateCheckSchedulerTest, StaticCheckOOBENotCompleteTest);
+ FRIEND_TEST(UpdateAttempterTest, PingOmahaTest);
// Wraps GLib's g_timeout_add_seconds so that it can be mocked in tests.
virtual guint GTimeoutAddSeconds(guint interval, GSourceFunc function);