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

components/update_client/update_checker.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/update_client/update_checker.h"
 
 #include <stddef.h>
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/stringprintf.h"
 #include "base/thread_task_runner_handle.h"
 #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"
 
 namespace update_client {
 
 namespace {
 
 // Returns a sanitized version of the brand or an empty string otherwise.
 std::string SanitizeBrand(const std::string& brand) {
@@ skipping 18 matching lines @@
 // An app element looks like this:
 //    <app appid="hnimpnehoodheedghdeeijklkeaacbdc"
 //         version="0.1.2.3" installsource="ondemand">
 //      <updatecheck />
 //      <packages>
 //        <package fp="abcd" />
 //      </packages>
 //    </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;
   for (size_t i = 0; i != items.size(); ++i) {
     const CrxUpdateItem* item = items[i];
     std::string app("<app ");
     base::StringAppendF(&app, "appid=\"%s\" version=\"%s\"", item->id.c_str(),
                         item->component.version.GetString().c_str());
     if (!brand.empty())
       base::StringAppendF(&app, " brand=\"%s\"", brand.c_str());
     if (item->on_demand)
       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>"
                           "<package fp=\"%s\"/>"
                           "</packages>",
                           item->component.fingerprint.c_str());
     }
     base::StringAppendF(&app, "</app>");
     app_elements.append(app);
     VLOG(1) << "Appending to update request: " << app;
   }
 
   return BuildProtocolRequest(
       config.GetBrowserVersion().GetString(), config.GetChannel(),
       config.GetLang(), config.GetOSLongName(), config.GetDownloadPreference(),
       app_elements, additional_attributes);
 }
 
 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.
   bool CheckForUpdates(
       const std::vector<CrxUpdateItem*>& items_to_check,
       const std::string& additional_attributes,
       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());
 }
 
 bool UpdateCheckerImpl::CheckForUpdates(
     const std::vector<CrxUpdateItem*>& items_to_check,
     const std::string& additional_attributes,
     const UpdateCheckCallback& update_check_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (request_sender_.get()) {
     NOTREACHED();
     return false;  // Another update check is in progress.
   }
 
   update_check_callback_ = update_check_callback;
 
   auto urls(config_->UpdateUrl());
   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));
       return;
     }
 
     error = -1;
     VLOG(1) << "Parse failed " << update_response.errors();
   }
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(update_check_callback_, error,
                             UpdateResponse::Results(), retry_after_sec));
 }
 
 }  // 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