Reading List iOS: Use external clock in ReadingListEntry.

components/reading_list/ios/reading_list_entry.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_READING_LIST_IOS_READING_LIST_ENTRY_H_
 #define COMPONENTS_READING_LIST_IOS_READING_LIST_ENTRY_H_
 
 #include <string>
 
 #include "base/files/file_path.h"
@@ skipping 17 matching lines @@
 class ReadingListSpecifics;
 }
 
 class ReadingListEntry;
 
 // An entry in the reading list. The URL is a unique identifier for an entry, as
 // such it should not be empty and is the only thing considered when comparing
 // entries.
 class ReadingListEntry {
  public:
-  ReadingListEntry(const GURL& url, const std::string& title);
+  // Creates a ReadingList entry. |url| and |title| are the main fields of the
+  // entry.
+  // |now| is used to fill the |creation_time_us_| and all the update timestamp
+  // fields.
   ReadingListEntry(const GURL& url,
                    const std::string& title,
+                   const base::Time& now);
+  ReadingListEntry(const GURL& url,
+                   const std::string& title,
+                   const base::Time& now,
                    std::unique_ptr<net::BackoffEntry> backoff);
   ReadingListEntry(ReadingListEntry&& entry);
   ~ReadingListEntry();
 
   // Entries are created in WAITING state. At some point they will be PROCESSING
   // into one of the three state: PROCESSED, the only state a distilled URL
   // would be set, WILL_RETRY, similar to wait, but with exponential delays or
   // ERROR where the system will not retry at all.
   enum DistillationState { WAITING, PROCESSING, PROCESSED, WILL_RETRY, ERROR };
 
@@ skipping 36 matching lines @@
   int64_t UpdateTitleTime() const;
 
   // The creation update time of the entry. The value is in microseconds since
   // Jan 1st 1970.
   int64_t CreationTime() const;
 
   // The time when the entry was read for the first time. The value is in
   // microseconds since Jan 1st 1970.
   int64_t FirstReadTime() const;
 
-  // Set the update time to now.
-  void MarkEntryUpdated();
+  // Set the update time to |now|.
+  void MarkEntryUpdated(const base::Time& now);
 
   // Returns a protobuf encoding the content of this ReadingListEntry for local
-  // storage.
-  std::unique_ptr<reading_list::ReadingListLocal> AsReadingListLocal() const;
+  // storage. Use |now| to serialize the backoff_entry.
+  std::unique_ptr<reading_list::ReadingListLocal> AsReadingListLocal(
+      const base::Time& now) const;
 
   // Returns a protobuf encoding the content of this ReadingListEntry for sync.
   std::unique_ptr<sync_pb::ReadingListSpecifics> AsReadingListSpecifics() const;
 
   // Created a ReadingListEntry from the protobuf format.
+  // Use |now| to deserialize the backoff_entry.
   static std::unique_ptr<ReadingListEntry> FromReadingListLocal(
-      const reading_list::ReadingListLocal& pb_entry);
+      const reading_list::ReadingListLocal& pb_entry,
+      const base::Time& now);
 
   // Created a ReadingListEntry from the protobuf format.
+  // If creation time is not set, it will be set to |now|.
   static std::unique_ptr<ReadingListEntry> FromReadingListSpecifics(
-      const sync_pb::ReadingListSpecifics& pb_entry);
+      const sync_pb::ReadingListSpecifics& pb_entry,
+      const base::Time& now);
 
   // Merge |this| and |other| into this.
   // Local fields are kept from |this|.
   // Each field is merged individually keeping the highest value as defined by
   // the |ReadingListStore.CompareEntriesForSync| function.
   //
   // After calling |MergeLocalStateFrom|, the result must verify
   // ReadingListStore.CompareEntriesForSync(old_this.AsReadingListSpecifics(),
   //                                        new_this.AsReadingListSpecifics())
   // and
   // ReadingListStore.CompareEntriesForSync(other.AsReadingListSpecifics(),
   //                                        new_this.AsReadingListSpecifics()).
   void MergeWithEntry(const ReadingListEntry& other);
 
   ReadingListEntry& operator=(ReadingListEntry&& other);
 
   bool operator==(const ReadingListEntry& other) const;
 
-  // Sets the title.
-  void SetTitle(const std::string& title);
+  // Sets |title_| to |title|. Sets |update_title_time_us_| to |now|.

[gambard, 2017/03/21 13:08:06]

Here and in the others comments of the Entry, you don't set
|update_title_time_us_| to |now|. You set |update_title_time_us_| to the
difference between |now| and the 01/01/1970.
Based on this comment and the one for |update_title_time_us_|, I would have
expected |now| to be this difference (i.e. using TimeToUS in the caller).
Can you update the comments?

[Olivier, 2017/03/21 14:01:09]

Technically, update_title_time_us_ is a timestamp, not a difference. So now =
update_title_time_us_, just in a different unit (documented in the variable and
in the Time class). I will add a class comment.

[gambard, 2017/03/21 15:36:53]

Acknowledged.

+  void SetTitle(const std::string& title, const base::Time& now);
   // Sets the distilled info (offline path, online URL, size and date of the
   // stored files) about distilled page, switch the state to PROCESSED and reset
   // the time until the next try.
   void SetDistilledInfo(const base::FilePath& path,
                         const GURL& distilled_url,
                         int64_t distilation_size,
-                        int64_t distilation_time);
+                        const base::Time& distilation_time);
   // Sets the state to one of PROCESSING, WILL_RETRY or ERROR.
   void SetDistilledState(DistillationState distilled_state);
   // Sets the read state of the entry. Will set the UpdateTime of the entry.
-  void SetRead(bool read);
+  // If |first_read_time_us_| is 0 and read is READ, sets |first_read_time_us_|
+  // to |now|.
+  void SetRead(bool read, const base::Time& now);
 
  private:
   enum State { UNSEEN, UNREAD, READ };
   ReadingListEntry(const GURL& url,
                    const std::string& title,
                    State state,
                    int64_t creation_time,
                    int64_t first_read_time,
                    int64_t update_time,
                    int64_t update_title_time,
@@ skipping 21 matching lines @@
   int64_t first_read_time_us_;
   int64_t update_time_us_;
   int64_t update_title_time_us_;
   int64_t distillation_time_us_;
   int64_t distillation_size_;
 
   DISALLOW_COPY_AND_ASSIGN(ReadingListEntry);
 };
 
 #endif  // COMPONENTS_READING_LIST_IOS_READING_LIST_ENTRY_H_