Implement ping_freshness for update_client.
BUG=603980
Review URL: https://codereview.chromium.org/1889263002
Cr-Commit-Position: refs/heads/master@{#387754}
diff --git a/components/update_client/persisted_data.cc b/components/update_client/persisted_data.cc
index 0894dba..673ba1e6 100644
--- a/components/update_client/persisted_data.cc
+++ b/components/update_client/persisted_data.cc
@@ -7,7 +7,9 @@
#include <string>
#include <vector>
+#include "base/guid.h"
#include "base/macros.h"
+#include "base/strings/stringprintf.h"
#include "base/threading/thread_checker.h"
#include "base/values.h"
#include "components/prefs/pref_registry_simple.h"
@@ -35,13 +37,28 @@
pref_service_->GetDictionary(kPersistedDataPreference);
// We assume ids do not contain '.' characters.
DCHECK_EQ(std::string::npos, id.find('.'));
- if (!dict->GetInteger("apps." + id + ".dlrc", &dlrc))
+ if (!dict->GetInteger(base::StringPrintf("apps.%s.dlrc", id.c_str()), &dlrc))
return kDateLastRollCallUnknown;
return dlrc;
}
+std::string PersistedData::GetPingFreshness(const std::string& id) const {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ if (!pref_service_)
+ return std::string();
+ std::string freshness;
+ const base::DictionaryValue* dict =
+ pref_service_->GetDictionary(kPersistedDataPreference);
+ // We assume ids do not contain '.' characters.
+ DCHECK_EQ(std::string::npos, id.find('.'));
+ if (!dict->GetString(base::StringPrintf("apps.%s.pf", id.c_str()),
+ &freshness))
+ return std::string();
+ return base::StringPrintf("{%s}", freshness.c_str());
+}
+
void PersistedData::SetDateLastRollCall(const std::vector<std::string>& ids,
- int datenum) const {
+ int datenum) {
DCHECK(thread_checker_.CalledOnValidThread());
if (!pref_service_ || datenum < 0)
return;
@@ -49,7 +66,9 @@
for (auto id : ids) {
// We assume ids do not contain '.' characters.
DCHECK_EQ(std::string::npos, id.find('.'));
- update->SetInteger("apps." + id + ".dlrc", datenum);
+ update->SetInteger(base::StringPrintf("apps.%s.dlrc", id.c_str()), datenum);
+ update->SetString(base::StringPrintf("apps.%s.pf", id.c_str()),
+ base::GenerateGUID());
}
}
diff --git a/components/update_client/persisted_data.h b/components/update_client/persisted_data.h
index d4673b4..6b24e57 100644
--- a/components/update_client/persisted_data.h
+++ b/components/update_client/persisted_data.h
@@ -38,12 +38,16 @@
// -2 indicates that there is no recorded date number for the |id|.
int GetDateLastRollCall(const std::string& id) const;
+ // Returns the PingFreshness (a random token that is written into the profile
+ // data whenever the DateLastRollCall it is modified) for the specified |id|.
+ // "" indicates that there is no recorded freshness value for the |id|.
+ std::string GetPingFreshness(const std::string& id) const;
+
// Records the DateLastRollCall for the specified |ids|. |datenum| must be a
// non-negative integer: calls with a negative |datenum| are simply ignored.
// Calls to SetDateLastRollCall that occur prior to the persisted data store
- // has been fully initialized are ignored.
- void SetDateLastRollCall(const std::vector<std::string>& ids,
- int datenum) const;
+ // has been fully initialized are ignored. Also sets the PingFreshness.
+ void SetDateLastRollCall(const std::vector<std::string>& ids, int datenum);
// This is called only via update_client's RegisterUpdateClientPreferences.
static void RegisterPrefs(PrefRegistrySimple* registry);
diff --git a/components/update_client/persisted_data_unittest.cc b/components/update_client/persisted_data_unittest.cc
index d89060f..dfe242c2 100644
--- a/components/update_client/persisted_data_unittest.cc
+++ b/components/update_client/persisted_data_unittest.cc
@@ -22,9 +22,15 @@
metadata->SetDateLastRollCall(items, 3383);
EXPECT_EQ(3383, metadata->GetDateLastRollCall("someappid"));
EXPECT_EQ(-2, metadata->GetDateLastRollCall("someotherappid"));
+ const std::string pf1 = metadata->GetPingFreshness("someappid");
+ EXPECT_FALSE(pf1.empty());
metadata->SetDateLastRollCall(items, 3386);
EXPECT_EQ(3386, metadata->GetDateLastRollCall("someappid"));
EXPECT_EQ(-2, metadata->GetDateLastRollCall("someotherappid"));
+ const std::string pf2 = metadata->GetPingFreshness("someappid");
+ EXPECT_FALSE(pf2.empty());
+ // The following has a 1 / 2^128 chance of being flaky.
+ EXPECT_NE(pf1, pf2);
}
TEST(PersistedDataTest, SharedPref) {
diff --git a/components/update_client/update_checker.cc b/components/update_client/update_checker.cc
index ec88892..7f40a0c 100644
--- a/components/update_client/update_checker.cc
+++ b/components/update_client/update_checker.cc
@@ -64,7 +64,7 @@
// </app>
std::string BuildUpdateCheckRequest(const Configurator& config,
const std::vector<CrxUpdateItem*>& items,
- const PersistedData& metadata,
+ PersistedData* metadata,
const std::string& additional_attributes) {
const std::string brand(SanitizeBrand(config.GetBrand()));
std::string app_elements;
@@ -82,8 +82,9 @@
base::StringAppendF(&app, " ap=\"%s\"", ap.c_str());
base::StringAppendF(&app, ">");
base::StringAppendF(&app, "<updatecheck />");
- base::StringAppendF(&app, "<ping rd=\"%d\" />",
- metadata.GetDateLastRollCall(item->id));
+ base::StringAppendF(&app, "<ping rd=\"%d\" ping_freshness=\"%s\" />",
+ metadata->GetDateLastRollCall(item->id),
+ metadata->GetPingFreshness(item->id).c_str());
if (!item->component.fingerprint.empty()) {
base::StringAppendF(&app,
"<packages>"
@@ -105,7 +106,7 @@
class UpdateCheckerImpl : public UpdateChecker {
public:
UpdateCheckerImpl(const scoped_refptr<Configurator>& config,
- const PersistedData& metadata);
+ PersistedData* metadata);
~UpdateCheckerImpl() override;
// Overrides for UpdateChecker.
@@ -122,7 +123,7 @@
base::ThreadChecker thread_checker_;
const scoped_refptr<Configurator> config_;
- const PersistedData& metadata_;
+ PersistedData* metadata_;
UpdateCheckCallback update_check_callback_;
scoped_ptr<RequestSender> request_sender_;
@@ -130,7 +131,7 @@
};
UpdateCheckerImpl::UpdateCheckerImpl(const scoped_refptr<Configurator>& config,
- const PersistedData& metadata)
+ PersistedData* metadata)
: config_(config), metadata_(metadata) {}
UpdateCheckerImpl::~UpdateCheckerImpl() {
@@ -180,7 +181,7 @@
if (update_response.Parse(response)) {
int daynum = update_response.results().daystart_elapsed_days;
if (daynum != UpdateResponse::kNoDaystart)
- metadata_.SetDateLastRollCall(*ids_checked, daynum);
+ metadata_->SetDateLastRollCall(*ids_checked, daynum);
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::Bind(update_check_callback_, error,
update_response.results(), retry_after_sec));
@@ -200,7 +201,7 @@
scoped_ptr<UpdateChecker> UpdateChecker::Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& persistent) {
+ PersistedData* persistent) {
return scoped_ptr<UpdateChecker>(new UpdateCheckerImpl(config, persistent));
}
diff --git a/components/update_client/update_checker.h b/components/update_client/update_checker.h
index 94b88d6..2af7e7a0 100644
--- a/components/update_client/update_checker.h
+++ b/components/update_client/update_checker.h
@@ -32,7 +32,7 @@
using Factory =
scoped_ptr<UpdateChecker> (*)(const scoped_refptr<Configurator>& config,
- const PersistedData& persistent);
+ PersistedData* persistent);
virtual ~UpdateChecker() {}
@@ -46,7 +46,7 @@
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& persistent);
+ PersistedData* persistent);
protected:
UpdateChecker() {}
diff --git a/components/update_client/update_checker_unittest.cc b/components/update_client/update_checker_unittest.cc
index b866643..7fd0972 100644
--- a/components/update_client/update_checker_unittest.cc
+++ b/components/update_client/update_checker_unittest.cc
@@ -165,7 +165,7 @@
EXPECT_TRUE(post_interceptor_->ExpectRequest(
new PartialMatch("updatecheck"), test_file("updatecheck_reply_1.xml")));
- update_checker_ = UpdateChecker::Create(config_, *metadata_);
+ update_checker_ = UpdateChecker::Create(config_, metadata_.get());
CrxUpdateItem item(BuildCrxUpdateItem());
item.component.ap = "some_ap";
@@ -194,8 +194,10 @@
string::npos,
post_interceptor_->GetRequests()[0].find(
"<app appid=\"jebgalgnebhfojomionfpkfelancnnkf\" version=\"0.9\" "
- "brand=\"TEST\" ap=\"some_ap\"><updatecheck /><ping rd=\"-2\" />"
- "<packages><package fp=\"fp1\"/></packages></app>"));
+ "brand=\"TEST\" ap=\"some_ap\"><updatecheck /><ping rd=\"-2\" "));
+ EXPECT_NE(string::npos,
+ post_interceptor_->GetRequests()[0].find(
+ "<packages><package fp=\"fp1\"/></packages></app>"));
EXPECT_NE(string::npos,
post_interceptor_->GetRequests()[0].find("<hw physmemory="));
@@ -213,7 +215,7 @@
EXPECT_TRUE(post_interceptor_->ExpectRequest(
new PartialMatch("updatecheck"), test_file("updatecheck_reply_1.xml")));
- update_checker_ = UpdateChecker::Create(config_, *metadata_);
+ update_checker_ = UpdateChecker::Create(config_, metadata_.get());
CrxUpdateItem item(BuildCrxUpdateItem());
item.component.ap = std::string(257, 'a'); // Too long.
@@ -230,8 +232,10 @@
string::npos,
post_interceptor_->GetRequests()[0].find(
"app appid=\"jebgalgnebhfojomionfpkfelancnnkf\" version=\"0.9\" "
- "brand=\"TEST\"><updatecheck /><ping rd=\"-2\" />"
- "<packages><package fp=\"fp1\"/></packages></app>"));
+ "brand=\"TEST\"><updatecheck /><ping rd=\"-2\" "));
+ EXPECT_NE(string::npos,
+ post_interceptor_->GetRequests()[0].find(
+ "<packages><package fp=\"fp1\"/></packages></app>"));
}
TEST_F(UpdateCheckerTest, UpdateCheckSuccessNoBrand) {
@@ -239,7 +243,7 @@
new PartialMatch("updatecheck"), test_file("updatecheck_reply_1.xml")));
config_->SetBrand("TOOLONG"); // Sets an invalid brand code.
- update_checker_ = UpdateChecker::Create(config_, *metadata_);
+ update_checker_ = UpdateChecker::Create(config_, metadata_.get());
CrxUpdateItem item(BuildCrxUpdateItem());
std::vector<CrxUpdateItem*> items_to_check;
@@ -255,8 +259,10 @@
string::npos,
post_interceptor_->GetRequests()[0].find(
"<app appid=\"jebgalgnebhfojomionfpkfelancnnkf\" version=\"0.9\">"
- "<updatecheck /><ping rd=\"-2\" /><packages><package fp=\"fp1\"/>"
- "</packages></app>"));
+ "<updatecheck /><ping rd=\"-2\" "));
+ EXPECT_NE(string::npos,
+ post_interceptor_->GetRequests()[0].find(
+ "<packages><package fp=\"fp1\"/></packages></app>"));
}
// Simulates a 403 server response error.
@@ -264,7 +270,7 @@
EXPECT_TRUE(
post_interceptor_->ExpectRequest(new PartialMatch("updatecheck"), 403));
- update_checker_ = UpdateChecker::Create(config_, *metadata_);
+ update_checker_ = UpdateChecker::Create(config_, metadata_.get());
CrxUpdateItem item(BuildCrxUpdateItem());
std::vector<CrxUpdateItem*> items_to_check;
@@ -290,7 +296,7 @@
config_->SetDownloadPreference(string("cacheable"));
- update_checker_ = UpdateChecker::Create(config_, *metadata_);
+ update_checker_ = UpdateChecker::Create(config_, metadata_.get());
CrxUpdateItem item(BuildCrxUpdateItem());
std::vector<CrxUpdateItem*> items_to_check;
@@ -316,7 +322,7 @@
new PartialMatch("updatecheck"), test_file("updatecheck_reply_1.xml")));
config_->SetUseCupSigning(true);
- update_checker_ = UpdateChecker::Create(config_, *metadata_);
+ update_checker_ = UpdateChecker::Create(config_, metadata_.get());
CrxUpdateItem item(BuildCrxUpdateItem());
std::vector<CrxUpdateItem*> items_to_check;
@@ -338,8 +344,10 @@
string::npos,
post_interceptor_->GetRequests()[0].find(
"<app appid=\"jebgalgnebhfojomionfpkfelancnnkf\" version=\"0.9\" "
- "brand=\"TEST\"><updatecheck /><ping rd=\"-2\" />"
- "<packages><package fp=\"fp1\"/></packages></app>"));
+ "brand=\"TEST\"><updatecheck /><ping rd=\"-2\" "));
+ EXPECT_NE(string::npos,
+ post_interceptor_->GetRequests()[0].find(
+ "<packages><package fp=\"fp1\"/></packages></app>"));
// Expect an error since the response is not trusted.
EXPECT_EQ(-10000, error_);
@@ -351,7 +359,7 @@
TEST_F(UpdateCheckerTest, UpdateCheckRequiresEncryptionError) {
config_->SetUpdateCheckUrl(GURL("http:\\foo\bar"));
- update_checker_ = UpdateChecker::Create(config_, *metadata_);
+ update_checker_ = UpdateChecker::Create(config_, metadata_.get());
CrxUpdateItem item(BuildCrxUpdateItem());
item.component.requires_network_encryption = true;
@@ -375,7 +383,7 @@
EXPECT_TRUE(post_interceptor_->ExpectRequest(
new PartialMatch("updatecheck"), test_file("updatecheck_reply_4.xml")));
- update_checker_ = UpdateChecker::Create(config_, *metadata_);
+ update_checker_ = UpdateChecker::Create(config_, metadata_.get());
CrxUpdateItem item(BuildCrxUpdateItem());
std::vector<CrxUpdateItem*> items_to_check;
@@ -387,7 +395,7 @@
base::Bind(&UpdateCheckerTest::UpdateCheckComplete,
base::Unretained(this)));
RunThreads();
- update_checker_ = UpdateChecker::Create(config_, *metadata_);
+ update_checker_ = UpdateChecker::Create(config_, metadata_.get());
update_checker_->CheckForUpdates(
items_to_check, "extra=\"params\"",
base::Bind(&UpdateCheckerTest::UpdateCheckComplete,
@@ -398,10 +406,10 @@
<< post_interceptor_->GetRequestsAsString();
ASSERT_EQ(2, post_interceptor_->GetCount())
<< post_interceptor_->GetRequestsAsString();
- EXPECT_NE(string::npos,
- post_interceptor_->GetRequests()[0].find("<ping rd=\"-2\" />"));
- EXPECT_NE(string::npos,
- post_interceptor_->GetRequests()[1].find("<ping rd=\"3383\" />"));
+ EXPECT_NE(string::npos, post_interceptor_->GetRequests()[0].find(
+ "<ping rd=\"-2\" ping_freshness="));
+ EXPECT_NE(string::npos, post_interceptor_->GetRequests()[1].find(
+ "<ping rd=\"3383\" ping_freshness="));
}
} // namespace update_client
diff --git a/components/update_client/update_client_unittest.cc b/components/update_client/update_client_unittest.cc
index 8e8c943..1bdfa2f1 100644
--- a/components/update_client/update_client_unittest.cc
+++ b/components/update_client/update_client_unittest.cc
@@ -148,7 +148,7 @@
static base::FilePath TestFilePath(const char* file);
scoped_refptr<update_client::Configurator> config() { return config_; }
- const update_client::PersistedData& metadata() { return *metadata_; }
+ update_client::PersistedData* metadata() { return metadata_.get(); }
base::Closure quit_closure() { return quit_closure_; }
@@ -227,7 +227,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
@@ -334,7 +334,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
@@ -520,7 +520,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
@@ -768,7 +768,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
@@ -1019,7 +1019,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
@@ -1311,7 +1311,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
@@ -1495,7 +1495,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
@@ -1785,7 +1785,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
@@ -1885,7 +1885,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
@@ -2072,7 +2072,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
@@ -2166,7 +2166,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
@@ -2212,7 +2212,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return nullptr;
}
@@ -2310,7 +2310,7 @@
public:
static scoped_ptr<UpdateChecker> Create(
const scoped_refptr<Configurator>& config,
- const PersistedData& metadata) {
+ PersistedData* metadata) {
return scoped_ptr<UpdateChecker>(new FakeUpdateChecker());
}
diff --git a/components/update_client/update_engine.cc b/components/update_client/update_engine.cc
index 87cbda2b..b9959a23 100644
--- a/components/update_client/update_engine.cc
+++ b/components/update_client/update_engine.cc
@@ -100,7 +100,7 @@
CrxUpdateItem update_item;
scoped_ptr<ActionUpdateCheck> update_check_action(new ActionUpdateCheck(
- (*update_context->update_checker_factory)(config_, *metadata_),
+ (*update_context->update_checker_factory)(config_, metadata_.get()),
config_->GetBrowserVersion(), config_->ExtraRequestParams()));
update_context->current_action.reset(update_check_action.release());
