Reading List create protobuf store

ios/chrome/browser/reading_list/reading_list_entry.cc

Index: ios/chrome/browser/reading_list/reading_list_entry.cc
diff --git a/ios/chrome/browser/reading_list/reading_list_entry.cc b/ios/chrome/browser/reading_list/reading_list_entry.cc
index bea3f83d348ca4aca4cee98b19fed190764e968f..76da4b88a99ef8b2ce3b30c775211f10c014373a 100644
--- a/ios/chrome/browser/reading_list/reading_list_entry.cc
+++ b/ios/chrome/browser/reading_list/reading_list_entry.cc
@@ -4,7 +4,18 @@
 
 #include "ios/chrome/browser/reading_list/reading_list_entry.h"
 
+#include "base/json/json_string_value_serializer.h"
 #include "base/memory/ptr_util.h"
+#include "ios/chrome/browser/reading_list/proto/reading_list.pb.h"
+#include "net/base/backoff_entry_serializer.h"
+
+namespace {
+// Default Tick clock for |net::BackoffEntry|.
+base::TickClock* ReadingListTickClock() {
+  static base::DefaultTickClock clock;

[sdefresne, 2016/10/07 09:44:30]

This is not thread-safe when building Chromium code (static initialization is
supposed to be thread-safe, but some of the platform we target have buggy
compiler, and thus we pass a flag to the compiler to disable the thread-safety
of static initialization to have the same behaviour on all platforms).

Do you really need to pass a non-null base::TickClock* to the net::BackoffEntry?
As far as I can tell, if you pass null, it will just use the system clock. If
this was already discussed in previous review comments, then maybe you need to
add a comment to this function explaining why this is required.

+  return &clock;
+}
+}
 
 // The backoff time is the following: 10min, 10min, 1h, 2h, 2h..., starting
 // after the first failure.
@@ -39,14 +50,34 @@ ReadingListEntry::ReadingListEntry(const GURL& url, const std::string& title)
 ReadingListEntry::ReadingListEntry(const GURL& url,
                                    const std::string& title,
                                    std::unique_ptr<net::BackoffEntry> backoff)
+    : ReadingListEntry(url, title, 0, 0, WAITING, GURL(), std::move(backoff)) {}
+
+ReadingListEntry::ReadingListEntry(
+    const GURL& url,
+    const std::string& title,
+    int64_t creation_time,
+    int64_t update_time,
+    ReadingListEntry::DistillationState distilled_state,
+    const GURL& distilled_url,
+    std::unique_ptr<net::BackoffEntry> backoff)
     : url_(url),
       title_(title),
-      distilled_state_(WAITING),
-      failed_download_counter_(0) {
+      distilled_url_(distilled_url),
+      distilled_state_(distilled_state),
+      failed_download_counter_(0),
+      creation_time_us_(creation_time),
+      update_time_us_(update_time) {
   if (backoff) {
     backoff_ = std::move(backoff);
   } else {
-    backoff_ = base::MakeUnique<net::BackoffEntry>(&kBackoffPolicy);
+    backoff_ = base::MakeUnique<net::BackoffEntry>(&kBackoffPolicy,
+                                                   ReadingListTickClock());
+  }
+  if (creation_time_us_ == 0) {
+    DCHECK(update_time_us_ == 0);
+    creation_time_us_ =
+        (base::Time::Now() - base::Time::UnixEpoch()).InMicroseconds();
+    update_time_us_ = creation_time_us_;
   }
   DCHECK(!url.is_empty());
   DCHECK(url.is_valid());
@@ -58,7 +89,9 @@ ReadingListEntry::ReadingListEntry(ReadingListEntry&& entry)
       distilled_url_(std::move(entry.distilled_url_)),
       distilled_state_(std::move(entry.distilled_state_)),
       backoff_(std::move(entry.backoff_)),
-      failed_download_counter_(std::move(entry.failed_download_counter_)) {}
+      failed_download_counter_(std::move(entry.failed_download_counter_)),
+      creation_time_us_(std::move(entry.creation_time_us_)),
+      update_time_us_(std::move(entry.update_time_us_)) {}
 
 ReadingListEntry::~ReadingListEntry() {}
 
@@ -126,3 +159,132 @@ void ReadingListEntry::SetDistilledState(DistillationState distilled_state) {
   distilled_state_ = distilled_state;
   distilled_url_ = GURL();
 }
+
+int64_t ReadingListEntry::UpdateTime() const {
+  return update_time_us_;
+}
+
+void ReadingListEntry::MarkEntryUpdated() {
+  update_time_us_ =
+      (base::Time::Now() - base::Time::UnixEpoch()).InMicroseconds();
+}
+
+// static
+std::unique_ptr<ReadingListEntry> ReadingListEntry::FromReadingListLocal(
+    const reading_list::ReadingListLocal& pb_entry) {
+  if (!pb_entry.has_entry() || !pb_entry.entry().has_url()) {
+    return nullptr;
+  }
+  GURL url(pb_entry.entry().url());
+  if (url.is_empty() || !url.is_valid()) {
+    return nullptr;
+  }
+  std::string title;
+  if (pb_entry.entry().has_title()) {
+    title = pb_entry.entry().title();
+  }
+
+  int64_t creation_time_us = 0;
+  if (pb_entry.entry().has_creation_time_us()) {
+    creation_time_us = pb_entry.entry().creation_time_us();
+  }
+
+  int64_t update_time_us = 0;
+  if (pb_entry.entry().has_update_time_us()) {
+    update_time_us = pb_entry.entry().update_time_us();
+  }
+
+  ReadingListEntry::DistillationState distillation_state =
+      ReadingListEntry::WAITING;
+  if (pb_entry.has_distillation_state()) {
+    switch (pb_entry.distillation_state()) {
+      case reading_list::ReadingListLocal::WAITING:
+        distillation_state = ReadingListEntry::WAITING;
+      case reading_list::ReadingListLocal::PROCESSING:
+        distillation_state = ReadingListEntry::PROCESSING;
+      case reading_list::ReadingListLocal::PROCESSED:
+        distillation_state = ReadingListEntry::PROCESSED;
+      case reading_list::ReadingListLocal::WILL_RETRY:
+        distillation_state = ReadingListEntry::WILL_RETRY;
+      case reading_list::ReadingListLocal::ERROR:
+        distillation_state = ReadingListEntry::ERROR;
+    }
+  }
+
+  GURL distilled_url;
+  if (pb_entry.has_distilled_url()) {
+    distilled_url = GURL(pb_entry.distilled_url());
+  }
+
+  std::unique_ptr<net::BackoffEntry> backoff;
+  if (pb_entry.has_backoff()) {
+    JSONStringValueDeserializer deserializer(pb_entry.backoff());
+    std::unique_ptr<base::Value> value(
+        deserializer.Deserialize(nullptr, nullptr));
+    if (value) {
+      backoff = net::BackoffEntrySerializer::DeserializeFromValue(
+          *value, &kBackoffPolicy, ReadingListTickClock(), base::Time::Now());
+    }
+  }
+
+  return base::WrapUnique<ReadingListEntry>(new ReadingListEntry(
+      url, title, creation_time_us, update_time_us, distillation_state,
+      distilled_url, std::move(backoff)));
+}
+
+std::unique_ptr<reading_list::ReadingListLocal>
+ReadingListEntry::AsReadingListLocal(bool read) const {
+  std::unique_ptr<reading_list::ReadingListLocal> pb_entry =
+      base::MakeUnique<reading_list::ReadingListLocal>();
+
+  // URL is used as the key for the database and sync as there is only one entry
+  // per URL.
+  pb_entry->mutable_entry()->set_entry_id(URL().spec());
+  pb_entry->mutable_entry()->set_title(Title());
+  pb_entry->mutable_entry()->set_url(URL().spec());
+  DCHECK(creation_time_us_ > 100000000);

[sdefresne, 2016/10/07 09:44:30]

Why this DCHECK? What does it mean. What does 100000000 correspond to?

+  pb_entry->mutable_entry()->set_creation_time_us(creation_time_us_);
+  pb_entry->mutable_entry()->set_update_time_us(UpdateTime());
+
+  if (read) {
+    pb_entry->mutable_entry()->set_status(sync_pb::ReadingListSpecifics::READ);
+  } else {
+    pb_entry->mutable_entry()->set_status(
+        sync_pb::ReadingListSpecifics::UNREAD);
+  }
+
+  reading_list::ReadingListLocal::DistillationState distilation_state;
+  switch (DistilledState()) {
+    case ReadingListEntry::WAITING:
+      distilation_state = reading_list::ReadingListLocal::WAITING;
+    case ReadingListEntry::PROCESSING:
+      distilation_state = reading_list::ReadingListLocal::PROCESSING;
+    case ReadingListEntry::PROCESSED:
+      distilation_state = reading_list::ReadingListLocal::PROCESSED;
+    case ReadingListEntry::WILL_RETRY:
+      distilation_state = reading_list::ReadingListLocal::WILL_RETRY;
+    case ReadingListEntry::ERROR:
+      distilation_state = reading_list::ReadingListLocal::ERROR;
+  }
+  pb_entry->set_distillation_state(distilation_state);
+  if (DistilledURL().is_valid()) {
+    pb_entry->set_distilled_url(DistilledURL().spec());
+  }
+
+  if (backoff_) {
+    std::unique_ptr<base::Value> backoff =
+        net::BackoffEntrySerializer::SerializeToValue(*backoff_,
+                                                      base::Time::Now());
+
+    std::string output;
+    JSONStringValueSerializer serializer(&output);
+    serializer.Serialize(*backoff);
+    pb_entry->set_backoff(output);
+  }
+  return pb_entry;
+}
+
+bool ReadingListEntry::CompareEntryUpdateTime(const ReadingListEntry& lhs,
+                                              const ReadingListEntry& rhs) {
+  return lhs.UpdateTime() > rhs.UpdateTime();
+}