Add Store+Sync to reading list.

ios/chrome/browser/reading_list/reading_list_model_impl.cc

Index: ios/chrome/browser/reading_list/reading_list_model_impl.cc
diff --git a/ios/chrome/browser/reading_list/reading_list_model_impl.cc b/ios/chrome/browser/reading_list/reading_list_model_impl.cc
index 1066206e65d581c687a5e366dfc55b1008cc2f4d..1a5c805c65cc55ffb5eb96638be5c9c562c4274f 100644
--- a/ios/chrome/browser/reading_list/reading_list_model_impl.cc
+++ b/ios/chrome/browser/reading_list/reading_list_model_impl.cc
@@ -4,86 +4,140 @@
 
 #include "ios/chrome/browser/reading_list/reading_list_model_impl.h"
 
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "components/prefs/pref_service.h"
 #include "ios/chrome/browser/reading_list/reading_list_model_storage.h"
+#include "ios/chrome/browser/reading_list/reading_list_pref_names.h"
+#include "ios/web/public/web_thread.h"
 #include "url/gurl.h"
 
-ReadingListModelImpl::ReadingListModelImpl() : ReadingListModelImpl(NULL) {}
+ReadingListModelImpl::ReadingListModelImpl()
+    : ReadingListModelImpl(NULL, NULL) {}

[jif, 2016/11/07 12:30:52]

s/NULL/nullptr/

(TIL that we can chain constructors in C++)

[Olivier, 2016/11/07 18:18:46]

Done.

 
 ReadingListModelImpl::ReadingListModelImpl(
-    std::unique_ptr<ReadingListModelStorage> storage)
-    : hasUnseen_(false) {
+    std::unique_ptr<ReadingListModelStorage> storage,
+    PrefService* pref_service)
+    : pref_service_(pref_service),
+      has_unseen_(false),
+      loaded_(false),
+      weak_ptr_factory_(this) {
+  DCHECK(CalledOnValidThread());
   if (storage) {
-    storageLayer_ = std::move(storage);
-    read_ = storageLayer_->LoadPersistentReadList();
-    unread_ = storageLayer_->LoadPersistentUnreadList();
-    hasUnseen_ = storageLayer_->LoadPersistentHasUnseen();
+    storage_layer_ = std::move(storage);
+    storage_layer_->SetReadingListModel(this);
+  } else {
+    loaded_ = true;
+    read_ = base::MakeUnique<ReadingListEntries>();
+    unread_ = base::MakeUnique<ReadingListEntries>();
   }
-  loaded_ = true;
+  has_unseen_ = LoadPersistentHasUnseen();
 }
 ReadingListModelImpl::~ReadingListModelImpl() {}
 
+void ReadingListModelImpl::ModelLoaded(
+    std::unique_ptr<ReadingListEntries> unread,
+    std::unique_ptr<ReadingListEntries> read) {
+  DCHECK(CalledOnValidThread());
+  read_ = std::move(read);
+  unread_ = std::move(unread);
+  loaded_ = true;
+  SortEntries();
+  for (auto& observer : observers_)
+    observer.ReadingListModelLoaded(this);
+}
+
 void ReadingListModelImpl::Shutdown() {
+  DCHECK(CalledOnValidThread());
   for (auto& observer : observers_)
     observer.ReadingListModelBeingDeleted(this);
   loaded_ = false;
 }
 
 bool ReadingListModelImpl::loaded() const {
+  DCHECK(CalledOnValidThread());
   return loaded_;
 }
 
 size_t ReadingListModelImpl::unread_size() const {
-  DCHECK(loaded());
-  return unread_.size();
+  DCHECK(CalledOnValidThread());
+  if (!loaded())
+    return 0;
+  return unread_->size();
 }
 
 size_t ReadingListModelImpl::read_size() const {
-  DCHECK(loaded());
-  return read_.size();
+  DCHECK(CalledOnValidThread());
+  if (!loaded())
+    return 0;
+  return read_->size();
 }
 
 bool ReadingListModelImpl::HasUnseenEntries() const {
-  DCHECK(loaded());
-  return unread_size() && hasUnseen_;
+  DCHECK(CalledOnValidThread());
+  if (!loaded())
+    return false;
+  return unread_size() && has_unseen_;
 }
 
 void ReadingListModelImpl::ResetUnseenEntries() {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
-  hasUnseen_ = false;
-  if (storageLayer_ && !IsPerformingBatchUpdates())
-    storageLayer_->SavePersistentHasUnseen(false);
+  has_unseen_ = false;
+  if (!IsPerformingBatchUpdates())
+    SavePersistentHasUnseen(false);
 }
 
 const ReadingListEntry& ReadingListModelImpl::GetUnreadEntryAtIndex(
     size_t index) const {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
-  return unread_[index];
+  return unread_->at(index);
 }
 
 const ReadingListEntry& ReadingListModelImpl::GetReadEntryAtIndex(
     size_t index) const {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
-  return read_[index];
+  return read_->at(index);
 }
 
 const ReadingListEntry* ReadingListModelImpl::GetEntryFromURL(
     const GURL& gurl) const {
+  DCHECK(CalledOnValidThread());
+  DCHECK(loaded());
+  bool read;
+  return GetMutableEntryFromURL(gurl, &read);

[jif, 2016/11/07 12:30:52]

s/&read/nullptr/ ?

[Olivier, 2016/11/07 18:18:46]

Done.

+}
+
+ReadingListEntry* ReadingListModelImpl::GetMutableEntryFromURL(
+    const GURL& gurl,
+    bool* read) const {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
+  bool is_read;
   ReadingListEntry entry(gurl, std::string());
-  auto it = std::find(read_.begin(), read_.end(), entry);
-  if (it == read_.end()) {
-    it = std::find(unread_.begin(), unread_.end(), entry);
-    if (it == unread_.end())
+  auto it = std::find(read_->begin(), read_->end(), entry);
+  is_read = true;
+  if (it == read_->end()) {
+    it = std::find(unread_->begin(), unread_->end(), entry);
+    is_read = false;
+    if (it == unread_->end())
       return nullptr;
   }
+  if (read) {

[jif, 2016/11/07 12:30:52]

I really dislike the fact that we are forbidden to use references for input
parameters.

+    *read = is_read;
+  }
   return &(*it);
 }
 
 bool ReadingListModelImpl::CallbackEntryURL(
     const GURL& url,
     base::Callback<void(const ReadingListEntry&)> callback) const {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
-  const ReadingListEntry* entry = GetEntryFromURL(url);
+  const ReadingListEntry* entry = GetMutableEntryFromURL(url, nullptr);
   if (entry) {
     callback.Run(*entry);
     return true;
@@ -91,33 +145,126 @@ bool ReadingListModelImpl::CallbackEntryURL(
   return false;
 }
 
-void ReadingListModelImpl::RemoveEntryByUrl(const GURL& url) {
+bool ReadingListModelImpl::CallbackEntryReadStatusURL(
+    const GURL& url,
+    base::Callback<void(const ReadingListEntry&, bool read)> callback) const {
+  DCHECK(CalledOnValidThread());
+  DCHECK(loaded());
+  bool read;
+  const ReadingListEntry* entry = GetMutableEntryFromURL(url, &read);
+  if (entry) {
+    callback.Run(*entry, read);
+    return true;
+  }
+  return false;
+}
+
+void ReadingListModelImpl::SyncAddEntry(std::unique_ptr<ReadingListEntry> entry,
+                                        bool read) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(loaded());
+  bool is_existing_entry_read;
+  ReadingListEntry* existing_entry =
+      GetMutableEntryFromURL(entry->URL(), &is_existing_entry_read);
+  if (!existing_entry) {
+    if (storage_layer_) {
+      storage_layer_->SaveEntry(*entry, read, true);
+    }
+    if (read) {
+      for (auto& observer : observers_)
+        observer.ReadingListWillAddReadEntry(this, *entry);
+      read_->insert(read_->begin(), std::move(*entry));
+    } else {
+      for (auto& observer : observers_)
+        observer.ReadingListWillAddUnreadEntry(this, *entry);
+      has_unseen_ = true;
+      SavePersistentHasUnseen(true);
+      unread_->insert(unread_->begin(), std::move(*entry));
+    }
+    for (auto& observer : observers_)
+      observer.ReadingListDidApplyChanges(this);
+    return;
+  }
+
+  if (existing_entry->UpdateTime() > entry->UpdateTime()) {
+    // Existing entry is newer thatn sync one. Do not update it.

[jif, 2016/11/07 12:30:52]

s/thatn/than the/

[Olivier, 2016/11/07 18:18:45]

Done.

+    return;
+  }
+
+  // Merge local data in new entry.
+  entry->MergeLocalStateFrom(*existing_entry);
+
+  if (is_existing_entry_read) {
+    auto result = std::find(read_->begin(), read_->end(), *existing_entry);
+    for (auto& observer : observers_)
+      observer.ReadingListWillMoveEntry(
+          this, std::distance(read_->begin(), result), true);

[jif, 2016/11/07 12:30:51]

Optional nit: writing:
observer.ReadingListWillMoveEntry(this, index, true);
would be more readable.

[Olivier, 2016/11/07 18:18:46]

Done.

+    if (result != read_->end()) {

[jif, 2016/11/07 12:30:52]

I don't think "result != read_->end()" is supposed to happen, so how about a
DCHECK(result != read_->end()) ?

[Olivier, 2016/11/07 18:18:46]

Done.

+      read_->erase(result);
+    }
+  } else {
+    auto result = std::find(unread_->begin(), unread_->end(), *existing_entry);
+    for (auto& observer : observers_)
+      observer.ReadingListWillMoveEntry(
+          this, std::distance(unread_->begin(), result), false);
+    if (result != unread_->end()) {

[jif, 2016/11/07 12:30:52]

Same here: replace with DCHECK?

[Olivier, 2016/11/07 18:18:46]

Done.

+      unread_->erase(result);
+    }
+  }
+
+  if (storage_layer_) {
+    storage_layer_->SaveEntry(*entry, read, true);
+  }
+
+  if (read) {
+    read_->push_back(std::move(*entry));
+  } else {
+    unread_->push_back(std::move(*entry));
+  }
+  for (auto& observer : observers_)
+    observer.ReadingListDidApplyChanges(this);
+}
+
+void ReadingListModelImpl::SyncRemoveEntry(const GURL& gurl) {
+  RemoveEntryByURLImpl(gurl, true);
+}
+
+void ReadingListModelImpl::RemoveEntryByURL(const GURL& url) {
+  RemoveEntryByURLImpl(url, false);
+}
+
+void ReadingListModelImpl::RemoveEntryByURLImpl(const GURL& url,
+                                                bool from_sync) {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
   const ReadingListEntry entry(url, std::string());
 
-  auto result = std::find(unread_.begin(), unread_.end(), entry);
-  if (result != unread_.end()) {
-    for (auto& observer : observers_) {
+  auto result = std::find(unread_->begin(), unread_->end(), entry);
+  if (result != unread_->end()) {
+    for (auto& observer : observers_)
       observer.ReadingListWillRemoveUnreadEntry(
-          this, std::distance(unread_.begin(), result));
+          this, std::distance(unread_->begin(), result));
+
+    if (storage_layer_) {
+      storage_layer_->RemoveEntry(*result, from_sync);
     }
-    unread_.erase(result);
-    if (storageLayer_ && !IsPerformingBatchUpdates())
-      storageLayer_->SavePersistentUnreadList(unread_);
+    unread_->erase(result);
+
     for (auto& observer : observers_)
       observer.ReadingListDidApplyChanges(this);
     return;
   }
 
-  result = std::find(read_.begin(), read_.end(), entry);
-  if (result != read_.end()) {
-    for (auto& observer : observers_) {
+  result = std::find(read_->begin(), read_->end(), entry);
+  if (result != read_->end()) {
+    for (auto& observer : observers_)
       observer.ReadingListWillRemoveReadEntry(
-          this, std::distance(read_.begin(), result));
+          this, std::distance(read_->begin(), result));
+    if (storage_layer_) {
+      storage_layer_->RemoveEntry(*result, from_sync);
     }
-    read_.erase(result);
-    if (storageLayer_ && !IsPerformingBatchUpdates())
-      storageLayer_->SavePersistentReadList(read_);
+    read_->erase(result);
+
     for (auto& observer : observers_)
       observer.ReadingListDidApplyChanges(this);
     return;
@@ -127,97 +274,104 @@ void ReadingListModelImpl::RemoveEntryByUrl(const GURL& url) {
 const ReadingListEntry& ReadingListModelImpl::AddEntry(
     const GURL& url,
     const std::string& title) {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
-  RemoveEntryByUrl(url);
+  RemoveEntryByURL(url);
   ReadingListEntry entry(url, title);
   for (auto& observer : observers_)
     observer.ReadingListWillAddUnreadEntry(this, entry);
-  unread_.insert(unread_.begin(), std::move(entry));
-  hasUnseen_ = true;
-  if (storageLayer_ && !IsPerformingBatchUpdates()) {
-    storageLayer_->SavePersistentUnreadList(unread_);
-    storageLayer_->SavePersistentHasUnseen(true);
+  has_unseen_ = true;
+  SavePersistentHasUnseen(true);
+  if (storage_layer_) {
+    storage_layer_->SaveEntry(entry, false, false);
   }
+  unread_->insert(unread_->begin(), std::move(entry));

[jif, 2016/11/07 12:30:52]

Would push_back(std::move(entry)) work?
If yes, change line 292 as well.

[Olivier, 2016/11/07 18:18:46]

It would, but may be bad in term of performances as it will require a sorting
after.
We know that the correct place is in front, and I hope std::sort will find out
hte vector is already sorted.

+
   for (auto& observer : observers_)
     observer.ReadingListDidApplyChanges(this);
-  return *unread_.begin();
+  return *unread_->begin();
 }
 
 void ReadingListModelImpl::MarkUnreadByURL(const GURL& url) {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
   ReadingListEntry entry(url, std::string());
-  auto result = std::find(read_.begin(), read_.end(), entry);
-  if (result == read_.end())
+  auto result = std::find(read_->begin(), read_->end(), entry);
+  if (result == read_->end())
     return;
 
   for (ReadingListModelObserver& observer : observers_) {
-    observer.ReadingListWillMoveEntry(this,
-                                      std::distance(read_.begin(), result));
+    observer.ReadingListWillMoveEntry(
+        this, std::distance(read_->begin(), result), true);
   }
 
-  unread_.insert(unread_.begin(), std::move(*result));
-  read_.erase(result);
-
-  if (storageLayer_ && !IsPerformingBatchUpdates()) {
-    storageLayer_->SavePersistentUnreadList(read_);
-    storageLayer_->SavePersistentReadList(unread_);
+  result->MarkEntryUpdated();
+  if (storage_layer_) {
+    storage_layer_->SaveEntry(*result, false, false);
   }
+
+  unread_->insert(unread_->begin(), std::move(*result));
+  read_->erase(result);
+
   for (ReadingListModelObserver& observer : observers_) {
     observer.ReadingListDidApplyChanges(this);
   }
 }
 
 void ReadingListModelImpl::MarkReadByURL(const GURL& url) {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
   ReadingListEntry entry(url, std::string());
-  auto result = std::find(unread_.begin(), unread_.end(), entry);
-  if (result == unread_.end())
+  auto result = std::find(unread_->begin(), unread_->end(), entry);
+  if (result == unread_->end())
     return;
 
-  for (auto& observer : observers_) {
-    observer.ReadingListWillMoveEntry(this,
-                                      std::distance(unread_.begin(), result));
+  for (auto& observer : observers_)
+    observer.ReadingListWillMoveEntry(
+        this, std::distance(unread_->begin(), result), false);
+
+  result->MarkEntryUpdated();
+  if (storage_layer_) {
+    storage_layer_->SaveEntry(*result, true, false);
   }
 
-  read_.insert(read_.begin(), std::move(*result));
-  unread_.erase(result);
+  read_->insert(read_->begin(), std::move(*result));
+  unread_->erase(result);
 
-  if (storageLayer_ && !IsPerformingBatchUpdates()) {
-    storageLayer_->SavePersistentUnreadList(unread_);
-    storageLayer_->SavePersistentReadList(read_);
-  }
   for (auto& observer : observers_)
     observer.ReadingListDidApplyChanges(this);
 }
 
 void ReadingListModelImpl::SetEntryTitle(const GURL& url,
                                          const std::string& title) {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
   const ReadingListEntry entry(url, std::string());
 
-  auto result = std::find(unread_.begin(), unread_.end(), entry);
-  if (result != unread_.end()) {
-    for (auto& observer : observers_) {
+  auto result = std::find(unread_->begin(), unread_->end(), entry);
+  if (result != unread_->end()) {
+    for (auto& observer : observers_)
       observer.ReadingListWillUpdateUnreadEntry(
-          this, std::distance(unread_.begin(), result));
-    }
+          this, std::distance(unread_->begin(), result));
     result->SetTitle(title);
-    if (storageLayer_ && !IsPerformingBatchUpdates())
-      storageLayer_->SavePersistentUnreadList(unread_);
+
+    if (storage_layer_) {
+      storage_layer_->SaveEntry(*result, false, false);
+    }
     for (auto& observer : observers_)
       observer.ReadingListDidApplyChanges(this);
     return;
   }
 
-  result = std::find(read_.begin(), read_.end(), entry);
-  if (result != read_.end()) {
-    for (auto& observer : observers_) {
+  result = std::find(read_->begin(), read_->end(), entry);
+  if (result != read_->end()) {
+    for (auto& observer : observers_)
       observer.ReadingListWillUpdateReadEntry(
-          this, std::distance(read_.begin(), result));
-    }
+          this, std::distance(read_->begin(), result));
     result->SetTitle(title);
-    if (storageLayer_ && !IsPerformingBatchUpdates())
-      storageLayer_->SavePersistentReadList(read_);
+    if (storage_layer_) {
+      storage_layer_->SaveEntry(*result, true, false);
+    }
     for (auto& observer : observers_)
       observer.ReadingListDidApplyChanges(this);
     return;
@@ -226,32 +380,33 @@ void ReadingListModelImpl::SetEntryTitle(const GURL& url,
 
 void ReadingListModelImpl::SetEntryDistilledURL(const GURL& url,
                                                 const GURL& distilled_url) {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
   const ReadingListEntry entry(url, std::string());
 
-  auto result = std::find(unread_.begin(), unread_.end(), entry);
-  if (result != unread_.end()) {
-    for (auto& observer : observers_) {
+  auto result = std::find(unread_->begin(), unread_->end(), entry);
+  if (result != unread_->end()) {
+    for (auto& observer : observers_)
       observer.ReadingListWillUpdateUnreadEntry(
-          this, std::distance(unread_.begin(), result));
-    }
+          this, std::distance(unread_->begin(), result));
     result->SetDistilledURL(distilled_url);
-    if (storageLayer_ && !IsPerformingBatchUpdates())
-      storageLayer_->SavePersistentUnreadList(unread_);
+    if (storage_layer_) {
+      storage_layer_->SaveEntry(*result, false, false);
+    }
     for (auto& observer : observers_)
       observer.ReadingListDidApplyChanges(this);
     return;
   }
 
-  result = std::find(read_.begin(), read_.end(), entry);
-  if (result != read_.end()) {
-    for (auto& observer : observers_) {
+  result = std::find(read_->begin(), read_->end(), entry);
+  if (result != read_->end()) {
+    for (auto& observer : observers_)
       observer.ReadingListWillUpdateReadEntry(
-          this, std::distance(read_.begin(), result));
-    }
+          this, std::distance(read_->begin(), result));
     result->SetDistilledURL(distilled_url);
-    if (storageLayer_ && !IsPerformingBatchUpdates())
-      storageLayer_->SavePersistentReadList(read_);
+    if (storage_layer_) {
+      storage_layer_->SaveEntry(*result, true, false);
+    }
     for (auto& observer : observers_)
       observer.ReadingListDidApplyChanges(this);
     return;
@@ -261,44 +416,85 @@ void ReadingListModelImpl::SetEntryDistilledURL(const GURL& url,
 void ReadingListModelImpl::SetEntryDistilledState(
     const GURL& url,
     ReadingListEntry::DistillationState state) {
+  DCHECK(CalledOnValidThread());
   DCHECK(loaded());
   const ReadingListEntry entry(url, std::string());
 
-  auto result = std::find(unread_.begin(), unread_.end(), entry);
-  if (result != unread_.end()) {
-    for (auto& observer : observers_) {
+  auto result = std::find(unread_->begin(), unread_->end(), entry);
+  if (result != unread_->end()) {
+    for (auto& observer : observers_)
       observer.ReadingListWillUpdateUnreadEntry(
-          this, std::distance(unread_.begin(), result));
-    }
+          this, std::distance(unread_->begin(), result));
     result->SetDistilledState(state);
-    if (storageLayer_ && !IsPerformingBatchUpdates())
-      storageLayer_->SavePersistentUnreadList(unread_);
+    if (storage_layer_) {
+      storage_layer_->SaveEntry(*result, false, false);
+    }
     for (auto& observer : observers_)
       observer.ReadingListDidApplyChanges(this);
     return;
   }
 
-  result = std::find(read_.begin(), read_.end(), entry);
-  if (result != read_.end()) {
-    for (auto& observer : observers_) {
+  result = std::find(read_->begin(), read_->end(), entry);
+  if (result != read_->end()) {
+    for (auto& observer : observers_)
       observer.ReadingListWillUpdateReadEntry(
-          this, std::distance(read_.begin(), result));
-    }
+          this, std::distance(read_->begin(), result));
     result->SetDistilledState(state);
-    if (storageLayer_ && !IsPerformingBatchUpdates())
-      storageLayer_->SavePersistentReadList(read_);
+    if (storage_layer_) {
+      storage_layer_->SaveEntry(*result, true, false);
+    }
     for (auto& observer : observers_)
       observer.ReadingListDidApplyChanges(this);
     return;
   }
 };
 
-void ReadingListModelImpl::EndBatchUpdates() {
-  ReadingListModel::EndBatchUpdates();
-  if (IsPerformingBatchUpdates() || !storageLayer_) {
+void ReadingListModelImpl::LeavingBatchUpdates() {
+  DCHECK(CalledOnValidThread());
+  ReadingListModel::LeavingBatchUpdates();

[jif, 2016/11/07 12:30:52]

Shouldn't |ReadingListModel::LeavingBatchUpdates()| be called at the end of the
method?

[Olivier, 2016/11/07 18:18:46]

Done.

+  if (storage_layer_) {
+    SavePersistentHasUnseen(has_unseen_);
+    storage_layer_->CommitTransaction();
+    SortEntries();
+  }
+}
+
+void ReadingListModelImpl::EnteringBatchUpdates() {
+  DCHECK(CalledOnValidThread());
+  if (storage_layer_) {
+    storage_layer_->BeginTransaction();
+  }
+  ReadingListModel::EnteringBatchUpdates();

[jif, 2016/11/07 12:30:52]

And this, at the beginning of the method?

[Olivier, 2016/11/07 18:18:46]

Done.

+}
+
+void ReadingListModelImpl::SavePersistentHasUnseen(bool has_unseen) {
+  DCHECK(CalledOnValidThread());
+  if (!pref_service_) {
     return;
   }
-  storageLayer_->SavePersistentUnreadList(unread_);
-  storageLayer_->SavePersistentReadList(read_);
-  storageLayer_->SavePersistentHasUnseen(hasUnseen_);
+  pref_service_->SetBoolean(reading_list::prefs::kReadingListHasUnseenEntries,
+                            has_unseen);
+}
+
+bool ReadingListModelImpl::LoadPersistentHasUnseen() {

[jif, 2016/11/07 12:30:52]

Tiny nit: we aren't loading anything here.
How about GetLoadPersistentHasUnseen?
And by symmetry, s/SavePersistentHasUnseen/SetPersistentHasUnseen/ ?

[Olivier, 2016/11/07 18:18:46]

Done.

+  DCHECK(CalledOnValidThread());
+  if (!pref_service_) {
+    return false;
+  }
+  return pref_service_->GetBoolean(
+      reading_list::prefs::kReadingListHasUnseenEntries);
+}
+
+syncer::ModelTypeService* ReadingListModelImpl::GetModelTypeService() {
+  DCHECK(loaded());
+  return storage_layer_->GetModelTypeService();

[jif, 2016/11/07 12:30:51]

Would this:

if (!storage_layer_)
  return nullptr;

be useful?

[Olivier, 2016/11/07 18:18:46]

Yes, thanks !

+}
+
+void ReadingListModelImpl::SortEntries() {
+  DCHECK(CalledOnValidThread());
+  DCHECK(loaded());
+  std::sort(read_->begin(), read_->end(),
+            ReadingListEntry::CompareEntryUpdateTime);
+  std::sort(unread_->begin(), unread_->end(),
+            ReadingListEntry::CompareEntryUpdateTime);
 }