Reading List iOS: Use external clock in ReadingListEntry.

components/reading_list/ios/reading_list_entry.h

Index: components/reading_list/ios/reading_list_entry.h
diff --git a/components/reading_list/ios/reading_list_entry.h b/components/reading_list/ios/reading_list_entry.h
index fc73f39f0e5126aeb65f303a2db6cedc1d174cea..11af2a1a0ca215ce33a01b03aeb85b306b7f44ec 100644
--- a/components/reading_list/ios/reading_list_entry.h
+++ b/components/reading_list/ios/reading_list_entry.h
@@ -35,9 +35,16 @@ class ReadingListEntry;
 // 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();
@@ -94,23 +101,28 @@ class ReadingListEntry {
   // 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|.
@@ -129,19 +141,21 @@ class ReadingListEntry {
 
   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 };