Add module for counting date-last-roll-call and persisting those counts

components/update_client/update_checker.cc

Index: components/update_client/update_checker.cc
diff --git a/components/update_client/update_checker.cc b/components/update_client/update_checker.cc
index 234ef225d5c44f7552b0f834f77e51bd0ac97e95..4055a22ea708a6423ba2e4e0b42ec0371e4fffc3 100644
--- a/components/update_client/update_checker.cc
+++ b/components/update_client/update_checker.cc
@@ -20,6 +20,7 @@
 #include "base/threading/thread_checker.h"
 #include "components/update_client/configurator.h"
 #include "components/update_client/crx_update_item.h"
+#include "components/update_client/persisted_data.h"
 #include "components/update_client/request_sender.h"
 #include "components/update_client/utils.h"
 #include "url/gurl.h"
@@ -58,6 +59,7 @@ bool IsEncryptionRequired(const std::vector<CrxUpdateItem*>& items) {
 //    </app>
 std::string BuildUpdateCheckRequest(const Configurator& config,
                                     const std::vector<CrxUpdateItem*>& items,
+                                    const PersistedData& metadata,
                                     const std::string& additional_attributes) {
   const std::string brand(SanitizeBrand(config.GetBrand()));
   std::string app_elements;
@@ -72,6 +74,8 @@ std::string BuildUpdateCheckRequest(const Configurator& config,
       base::StringAppendF(&app, " installsource=\"ondemand\"");
     base::StringAppendF(&app, ">");
     base::StringAppendF(&app, "<updatecheck />");
+    base::StringAppendF(
+        &app, "<ping rd=\"%d\" />", metadata.DateLastRollCall(item->id));
     if (!item->component.fingerprint.empty()) {
       base::StringAppendF(&app,
                           "<packages>"
@@ -92,7 +96,9 @@ std::string BuildUpdateCheckRequest(const Configurator& config,
 
 class UpdateCheckerImpl : public UpdateChecker {
  public:
-  explicit UpdateCheckerImpl(const scoped_refptr<Configurator>& config);
+  UpdateCheckerImpl(
+      const scoped_refptr<Configurator>& config,
+      const scoped_refptr<PersistedData>& metadata);
   ~UpdateCheckerImpl() override;
 
   // Overrides for UpdateChecker.
@@ -102,20 +108,24 @@ class UpdateCheckerImpl : public UpdateChecker {
       const UpdateCheckCallback& update_check_callback) override;
 
  private:
-  void OnRequestSenderComplete(int error,
+  void OnRequestSenderComplete(scoped_ptr<std::vector<std::string>> ids_checked,
+                               int error,
                                const std::string& response,
                                int retry_after_sec);
   base::ThreadChecker thread_checker_;
 
   const scoped_refptr<Configurator> config_;
+  const scoped_refptr<PersistedData>& metadata_;

[Bernhard Bauer, 2016/04/08 09:06:10]

Uh, this is a bit... unconventional. Why not just take a regular reference to
the PersistedData?

[waffles, 2016/04/08 19:11:01]

Done. (I don't think I originally meant to have that & in there.)

   UpdateCheckCallback update_check_callback_;
   scoped_ptr<RequestSender> request_sender_;
 
   DISALLOW_COPY_AND_ASSIGN(UpdateCheckerImpl);
 };
 
-UpdateCheckerImpl::UpdateCheckerImpl(const scoped_refptr<Configurator>& config)
-    : config_(config) {}
+UpdateCheckerImpl::UpdateCheckerImpl(
+    const scoped_refptr<Configurator>& config,
+    const scoped_refptr<PersistedData>& metadata)
+    : config_(config), metadata_(metadata) {}
 
 UpdateCheckerImpl::~UpdateCheckerImpl() {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -138,23 +148,35 @@ bool UpdateCheckerImpl::CheckForUpdates(
   if (IsEncryptionRequired(items_to_check))
     RemoveUnsecureUrls(&urls);
 
+  scoped_ptr<std::vector<std::string>> ids_checked(
+      new std::vector<std::string>());
+  for (auto crx : items_to_check) {
+    ids_checked->push_back(crx->id);

[Bernhard Bauer, 2016/04/08 09:06:10]

Nit: Braces are optional for one-line bodies (and not used elsewhere, e.g. line
149).

[waffles, 2016/04/08 19:11:01]

Done.

+  }
   request_sender_.reset(new RequestSender(config_));
   request_sender_->Send(
       config_->UseCupSigning(),
-      BuildUpdateCheckRequest(*config_, items_to_check, additional_attributes),
+      BuildUpdateCheckRequest(*config_, items_to_check, *metadata_,
+                              additional_attributes),
       urls, base::Bind(&UpdateCheckerImpl::OnRequestSenderComplete,
-                       base::Unretained(this)));
+                       base::Unretained(this), base::Passed(&ids_checked)));

[Bernhard Bauer, 2016/04/08 09:06:10]

Since base::Passed is now implemented in terms of std::move, you could try just
using the std::vector<> directly here, and base::Passed should make sure that
its passed to the function instead of copied.

[waffles, 2016/04/08 19:11:01]

Hm, not sure I'm understand what you're suggesting. The std::vector will be an
lvalue, so we can't use base::Passed, right?

[Bernhard Bauer, 2016/04/08 21:54:07]

No, base::Passed has a form where it takes a pointer, and then calls std::move
on the value pointed to, which allows it to make it an rvalue. Basically, that
is the same trick that binding a scoped_ptr to a callback (which isn't copyable
at all), so it should also work for not copying a std::vector. But this is
definitely okay too – no need for you to become the guinea pig for me here :-D

   return true;
 }
 
-void UpdateCheckerImpl::OnRequestSenderComplete(int error,
-                                                const std::string& response,
-                                                int retry_after_sec) {
+void UpdateCheckerImpl::OnRequestSenderComplete(
+    scoped_ptr<std::vector<std::string>> ids_checked,

[Bernhard Bauer, 2016/04/08 09:06:10]

std::unique_ptr

[waffles, 2016/04/08 19:11:01]

Done.

+    int error,
+    const std::string& response,
+    int retry_after_sec) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (!error) {
     UpdateResponse update_response;
     if (update_response.Parse(response)) {
+      int daynum = update_response.results().daystart_elapsed_days;
+      if (daynum != UpdateResponse::kNoDaystart) {

[Bernhard Bauer, 2016/04/08 09:06:10]

No braces.

[waffles, 2016/04/08 19:11:01]

Done.

+        metadata_->SetDateLastRollCall(*ids_checked, daynum);
+      }
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::Bind(update_check_callback_, error,
                                 update_response.results(), retry_after_sec));
@@ -173,8 +195,9 @@ void UpdateCheckerImpl::OnRequestSenderComplete(int error,
 }  // namespace
 
 scoped_ptr<UpdateChecker> UpdateChecker::Create(
-    const scoped_refptr<Configurator>& config) {
-  return scoped_ptr<UpdateChecker>(new UpdateCheckerImpl(config));
+    const scoped_refptr<Configurator>& config,
+    const scoped_refptr<PersistedData>& persistent) {
+  return scoped_ptr<UpdateChecker>(new UpdateCheckerImpl(config, persistent));
 }
 
 }  // namespace update_client