Adds check/set for first_active_omaha_ping_sent (from vpd).
first_active_omaha_ping_sent is a boolean in RW_VPD that is checked to
decide whether to send the first active ping (a=-1) to omaha. update_engine
reads its value and sets it to "true" after first active ping is sent or
if the first ping has been sent already and it does not exits in the vpd
(to capture the same feature for older devices).
BUG=chromium:717788
TEST=cros_workon_make --test; Deployed on a device and made sure the
flag is read and writen properly.
CQ-DEPEND=CL:469126
Change-Id: I867d16de1d5d2da4d7a7e1f26aa8e951d90b68f2
Reviewed-on: https://chromium-review.googlesource.com/469107
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Andrew de los Reyes <adlr@chromium.org>
diff --git a/common/fake_hardware.h b/common/fake_hardware.h
index 5d0fca3..01d23d0 100644
--- a/common/fake_hardware.h
+++ b/common/fake_hardware.h
@@ -81,6 +81,14 @@
return false;
}
+ bool GetFirstActiveOmahaPingSent() const override {
+ return first_active_omaha_ping_sent_;
+ }
+
+ void SetFirstActiveOmahaPingSent() override {
+ first_active_omaha_ping_sent_ = true;
+ }
+
// Setters
void SetIsOfficialBuild(bool is_official_build) {
is_official_build_ = is_official_build;
@@ -137,6 +145,7 @@
std::string ec_version_{"Fake EC v1.0a"};
int powerwash_count_{kPowerwashCountNotSet};
bool powerwash_scheduled_{false};
+ bool first_active_omaha_ping_sent_{false};
DISALLOW_COPY_AND_ASSIGN(FakeHardware);
};
diff --git a/common/hardware_interface.h b/common/hardware_interface.h
index 316ad3d..541a68a 100644
--- a/common/hardware_interface.h
+++ b/common/hardware_interface.h
@@ -89,6 +89,15 @@
// powerwash cycles. In case of an error, such as no directory available,
// returns false.
virtual bool GetPowerwashSafeDirectory(base::FilePath* path) const = 0;
+
+ // Returns whether the first active ping was sent to Omaha at some point, and
+ // that the value is persisted across recovery (and powerwash) once set with
+ // |SetFirstActiveOmahaPingSent()|.
+ virtual bool GetFirstActiveOmahaPingSent() const = 0;
+
+ // Persist the fact that first active ping was sent to omaha. It bails out if
+ // it fails.
+ virtual void SetFirstActiveOmahaPingSent() = 0;
};
} // namespace chromeos_update_engine
diff --git a/common/mock_hardware.h b/common/mock_hardware.h
index 1c4253a..42fa7ba 100644
--- a/common/mock_hardware.h
+++ b/common/mock_hardware.h
@@ -63,6 +63,12 @@
ON_CALL(*this, GetPowerwashSafeDirectory(testing::_))
.WillByDefault(testing::Invoke(&fake_,
&FakeHardware::GetPowerwashSafeDirectory));
+ ON_CALL(*this, GetFirstActiveOmahaPingSent())
+ .WillByDefault(testing::Invoke(&fake_,
+ &FakeHardware::GetFirstActiveOmahaPingSent()));
+ ON_CALL(*this, SetFirstActiveOmahaPingSent())
+ .WillByDefault(testing::Invoke(&fake_,
+ &FakeHardware::SetFirstActiveOmahaPingSent()));
}
~MockHardware() override = default;
@@ -78,6 +84,7 @@
MOCK_CONST_METHOD0(GetPowerwashCount, int());
MOCK_CONST_METHOD1(GetNonVolatileDirectory, bool(base::FilePath*));
MOCK_CONST_METHOD1(GetPowerwashSafeDirectory, bool(base::FilePath*));
+ MOCK_CONST_METHOD0(GetFirstActiveOmahaPingSent, bool());
// Returns a reference to the underlying FakeHardware.
FakeHardware& fake() {
diff --git a/hardware_android.cc b/hardware_android.cc
index 91c3fbe..d4a6838 100644
--- a/hardware_android.cc
+++ b/hardware_android.cc
@@ -200,4 +200,14 @@
return false;
}
+bool HardwareAndroid::GetFirstActiveOmahaPingSent() const {
+ LOG(WARNING) << "STUB: Assuming first active omaha was never set.";
+ return false;
+}
+
+void HardwareChromeOS::SetFirstActiveOmahaPingSent() {
+ LOG(WARNING) << "STUB: Assuming first active omaha is never set.";
+ return;
+}
+
} // namespace chromeos_update_engine
diff --git a/hardware_android.h b/hardware_android.h
index 78af871..37393ce 100644
--- a/hardware_android.h
+++ b/hardware_android.h
@@ -47,6 +47,8 @@
bool CancelPowerwash() override;
bool GetNonVolatileDirectory(base::FilePath* path) const override;
bool GetPowerwashSafeDirectory(base::FilePath* path) const override;
+ bool GetFirstActiveOmahaPingSent() const override;
+ void SetFirstActiveOmahaPingSent() override;
private:
DISALLOW_COPY_AND_ASSIGN(HardwareAndroid);
diff --git a/hardware_chromeos.cc b/hardware_chromeos.cc
index 4b0b82f..47beea8 100644
--- a/hardware_chromeos.cc
+++ b/hardware_chromeos.cc
@@ -69,6 +69,8 @@
// UpdateManager config options:
const char* kConfigOptsIsOOBEEnabled = "is_oobe_enabled";
+const char* kActivePingKey = "first_active_omaha_ping_sent";
+
} // namespace
namespace chromeos_update_engine {
@@ -248,4 +250,49 @@
is_oobe_enabled_ = true; // Default value.
}
+bool HardwareChromeOS::GetFirstActiveOmahaPingSent() const {
+ int exit_code = 0;
+ string active_ping_str;
+ vector<string> cmd = { "vpd_get_value", kActivePingKey };
+ if (!Subprocess::SynchronousExec(cmd, &exit_code, &active_ping_str) ||
+ exit_code) {
+ LOG(ERROR) << "Failed to get vpd key for " << kActivePingKey
+ << " with exit code: " << exit_code;
+ return false;
+ }
+
+ base::TrimWhitespaceASCII(active_ping_str,
+ base::TRIM_ALL,
+ &active_ping_str);
+ int active_ping;
+ if (active_ping_str.empty() ||
+ !base::StringToInt(active_ping_str, &active_ping)) {
+ LOG(INFO) << "Failed to parse active_ping value: " << active_ping_str;
+ return false;
+ }
+ return static_cast<bool>(active_ping);
+}
+
+void HardwareChromeOS::SetFirstActiveOmahaPingSent() {
+ int exit_code = 0;
+ string output;
+ vector<string> vpd_set_cmd = {
+ "vpd", "-i", "RW_VPD", "-s", string(kActivePingKey) + "=1" };
+ if (!Subprocess::SynchronousExec(vpd_set_cmd, &exit_code, &output) ||
+ exit_code) {
+ LOG(ERROR) << "Failed to set vpd key for " << kActivePingKey
+ << " with exit code: " << exit_code
+ << " with error: " << output;
+ return;
+ }
+
+ vector<string> vpd_dump_cmd = { "dump_vpd_log", "--force" };
+ if (!Subprocess::SynchronousExec(vpd_dump_cmd, &exit_code, &output) ||
+ exit_code) {
+ LOG(ERROR) << "Failed to cache " << kActivePingKey<< " using dump_vpd_log"
+ << " with exit code: " << exit_code
+ << " with error: " << output;
+ }
+}
+
} // namespace chromeos_update_engine
diff --git a/hardware_chromeos.h b/hardware_chromeos.h
index 03ad750..3a0bba2 100644
--- a/hardware_chromeos.h
+++ b/hardware_chromeos.h
@@ -51,6 +51,8 @@
bool CancelPowerwash() override;
bool GetNonVolatileDirectory(base::FilePath* path) const override;
bool GetPowerwashSafeDirectory(base::FilePath* path) const override;
+ bool GetFirstActiveOmahaPingSent() const override;
+ void SetFirstActiveOmahaPingSent() override;
private:
friend class HardwareChromeOSTest;
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index ed3c716..47f3e37 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -574,6 +574,11 @@
<< "powerwash_count is " << powerwash_count;
return false;
}
+ if (system_state_->hardware()->GetFirstActiveOmahaPingSent()) {
+ LOG(INFO) << "Not sending ping with a=-1 r=-1 to omaha because "
+ << "the first_active_omaha_ping_sent is true";
+ return false;
+ }
return true;
}
return ping_active_days_ > 0 || ping_roll_call_days_ > 0;
@@ -988,6 +993,16 @@
LOG_IF(ERROR, !UpdateLastPingDays(&parser_data, system_state_->prefs()))
<< "Failed to update the last ping day preferences!";
+ // Sets first_active_omaha_ping_sent to true (vpd in CrOS). We only do this if
+ // we have got a response from omaha and if its value has never been set to
+ // true before. Failure of this function should be ignored. There should be no
+ // need to check if a=-1 has been sent because older devices have already sent
+ // their a=-1 in the past and we have to set first_active_omaha_ping_sent for
+ // future checks.
+ if (!system_state_->hardware()->GetFirstActiveOmahaPingSent()) {
+ system_state_->hardware()->SetFirstActiveOmahaPingSent();
+ }
+
if (!HasOutputPipe()) {
// Just set success to whether or not the http transfer succeeded,
// which must be true at this point in the code.
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index cb3ef71..8c92d00 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -2145,6 +2145,35 @@
EXPECT_EQ(string::npos, post_str.find("<ping"));
}
+// Checks that the initial ping with a=-1 r=-1 is not send when the device
+// first_active_omaha_ping_sent is set.
+TEST_F(OmahaRequestActionTest, PingWhenFirstActiveOmahaPingIsSent) {
+ fake_prefs_.SetString(kPrefsPreviousVersion, "");
+
+ // Flag that the device was not powerwashed in the past.
+ fake_system_state_.fake_hardware()->SetPowerwashCount(0);
+
+ // Flag that the device has sent first active ping in the past.
+ fake_system_state_.fake_hardware()->SetFirstActiveOmahaPingSent();
+
+ brillo::Blob post_data;
+ ASSERT_TRUE(
+ TestUpdateCheck(nullptr, // request_params
+ fake_update_response_.GetNoUpdateResponse(),
+ -1,
+ false, // ping_only
+ ErrorCode::kSuccess,
+ metrics::CheckResult::kNoUpdateAvailable,
+ metrics::CheckReaction::kUnset,
+ metrics::DownloadErrorCode::kUnset,
+ nullptr,
+ &post_data));
+ // We shouldn't send a ping in this case since
+ // first_active_omaha_ping_sent=true
+ string post_str(post_data.begin(), post_data.end());
+ EXPECT_EQ(string::npos, post_str.find("<ping"));
+}
+
// Checks that the event 54 is sent on a reboot to a new update.
TEST_F(OmahaRequestActionTest, RebootAfterUpdateEvent) {
// Flag that the device was updated in a previous boot.