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

components/update_client/persisted_data.h

+// Copyright 2016 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.
+
+#ifndef COMPONENTS_UPDATE_CLIENT_PERSISTED_DATA_H_
+#define COMPONENTS_UPDATE_CLIENT_PERSISTED_DATA_H_
+
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/threading/thread_checker.h"
+#include "base/values.h"
+
+class PrefRegistrySimple;
+class PrefService;
+
+namespace update_client {
+
+// A PersistedData is a wrapper layer around a PrefService, designed to maintain
+// update data that outlives the browser process and isn't exposed outside of
+// update_client.
+//
+// The public methods of this class should be called only on the thread that
+// initializes it - which also has to match the thread the PrefService has been
+// initialized on.
+class PersistedData : public base::RefCountedThreadSafe<PersistedData> {
+ public:
+  // Constructs a provider that uses the specified |prefService|.
+  // The associated preferences are assumed to already be registered.
+  // The prefService must outlive the entire update_client.

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

Nit: |prefService| in pipes?

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

Done.

+  PersistedData(PrefService* prefService);

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

Also, underscore_style for variables.

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

Done.

+
+  // Returns the DateLastRollCall of the specified |id|. -2 indicates that there
+  // is no record of the app.

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

Can you explain what the value means otherwise?

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

Done.

+  int DateLastRollCall(const std::string& id) const;

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

Nit: GetDateLastRollCall(), so it doesn't sound like "date" is a verb here?

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

Done. (Too much Go lately, I guess.)

+
+  // Records the DateLastRollCall for the specified |ids|. |datenum| must be a
+  // non-negative integer: calls with a negative |datenum| are simply ignored.

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

Is there a reason you ignore negative values rather than DCHECKing? I.e., can
this really happen in the absence of bugs?

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

The value ultimately comes from the network - so we could either require the
parser to branch to not call this if it sees a negative number and then DCHECK
here, or just have the parsing code treat it as an opaque int and have the
branch here. Seemed better to me to handle it here, since this module is really
responsible for interpreting the meaning of these values.

+  // 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);
+
+ private:
+  friend class base::RefCountedThreadSafe<PersistedData>;
+
+  ~PersistedData();
+
+  base::ThreadChecker thread_checker_;
+  PrefService* pref_service_;
+
+  DISALLOW_COPY_AND_ASSIGN(PersistedData);
+};
+
+// This is called only via update_client's RegisterUpdateClientPreferences.
+void RegisterPersistedDataPreferences(PrefRegistrySimple* registry);

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

Make this a static method on PersistedData?

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

Done.

+
+}  // namespace update_client
+
+#endif  // COMPONENTS_UPDATE_CLIENT_PERSISTED_DATA_H_