Refactor Reading List Model to use URL as key.

ios/chrome/browser/reading_list/reading_list_model.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 IOS_CHROME_BROWSER_READING_LIST_READING_LIST_MODEL_H_
 #define IOS_CHROME_BROWSER_READING_LIST_READING_LIST_MODEL_H_
 
 #include <memory>
 #include <string>
 #include <vector>
@@ skipping 40 matching lines @@
   // This method is reentrant, i.e. several batch updates may take place at the
   // same time.
   // Returns a scoped batch update object that should be retained while the
   // batch update is performed. Deallocating this object will inform model that
   // the batch update has completed.
   std::unique_ptr<ScopedReadingListBatchUpdate> BeginBatchUpdates();
 
   // Creates a batch token that will freeze the model while in scope.
   virtual std::unique_ptr<ScopedReadingListBatchUpdate> CreateBatchToken();
 
-  // Returns the size of read and unread entries.
+  // Returns the total number of entries in the model.
+  virtual size_t size() const = 0;
+
+  // Returns the total number of unread entries in the model.
   virtual size_t unread_size() const = 0;
+
+  // Returns the total number of read entries in the model.
   virtual size_t read_size() const = 0;

[gambard, 2016/11/23 12:11:06]

This can probably be removed.

[Olivier, 2016/11/28 14:54:14]

Done.

 
   // Returns true if there are entries in the model that were not seen by the
   // user yet. Reset to true when new unread entries are added. Reset to false
   // when ResetUnseenEntries is called.
   virtual bool HasUnseenEntries() const = 0;
   virtual void ResetUnseenEntries() = 0;
 
-  // TODO(659099): Remove methods.
-  // Returns a specific entry.
-  virtual const ReadingListEntry& GetUnreadEntryAtIndex(size_t index) const = 0;
-  virtual const ReadingListEntry& GetReadEntryAtIndex(size_t index) const = 0;

[gambard, 2016/11/23 12:11:06]

You are breaking downstream by renaming them no?

[Olivier, 2016/11/28 14:54:14]

Done.

+  // Returns a specific entry. Returns null if the entry does not exist.
+  virtual const ReadingListEntry* GetEntryFromURL(const GURL& gurl) const = 0;
 
-  // Returns a specific entry. Returns null if the entry does not exist.
-  // If |read| is not null and the entry is found, |*read| is the read status
-  // of the entry.
-  virtual const ReadingListEntry* GetEntryFromURL(const GURL& gurl,
-                                                  bool* read) const = 0;
+  // Allows to iterate through entries in the model. Must be called on a single
+  // runloop to ensure no entry is returned twice and all entries are returned.
+  virtual const ReadingListEntry* GetEntryAt(size_t index) const = 0;

[jif-google, 2016/11/22 18:21:45]

I think it would be useful to offer range based for-loops on the reading list
model, like:

for (const ReadingListEntry& entry : reading_list_model) {


}

[Olivier, 2016/11/28 14:54:14]

Done.

+  // Allows to iterate through read entries in the model. Must be called on a
+  // singlerunloop to ensure no entry is returned twice and all entries are
+  // returned
+  virtual const ReadingListEntry* GetReadEntryAt(size_t index) const = 0;

[jif-google, 2016/11/22 18:21:45]

this will be removed, right?

[Olivier, 2016/11/28 14:54:14]

Done.

+  // Allows to iterate through unread entries in the model. Must be called on a
+  // singlerunloop to ensure no entry is returned twice and all entries are
+  // returned
+  virtual const ReadingListEntry* GetUnreadEntryAt(size_t index) const = 0;

[jif-google, 2016/11/22 18:21:45]

this will be removed, right?

[Olivier, 2016/11/28 14:54:14]

Done.

 
   // Synchronously calls the |callback| with entry associated with this |url|.
   // Does nothing if there is no entry associated.
   // Returns whether the callback has been called.
   virtual bool CallbackEntryURL(

[gambard, 2016/11/23 12:11:06]

This can probably be removed, it was a workaround because we only returned
reference not pointers.

[Olivier, 2016/11/28 14:54:14]

Done.

       const GURL& url,
       base::Callback<void(const ReadingListEntry&)> callback) const = 0;
 
   // Adds |url| at the top of the unread entries, and removes entries with the
   // same |url| from everywhere else if they exist. The entry title will be a
   // trimmed copy of |title.
   // The addition may be asynchronous, and the data will be available only once
   // the observers are notified.
   virtual const ReadingListEntry& AddEntry(const GURL& url,
                                            const std::string& title) = 0;
 
   // Removes an entry. The removal may be asynchronous, and not happen
   // immediately.
   virtual void RemoveEntryByURL(const GURL& url) = 0;
 
-  // If the |url| is in the reading list and unread, mark it read. If it is in
-  // the reading list and read, move it to the top of unread if it is not here
-  // already. This may trigger deletion of old read entries.
-  virtual void MarkReadByURL(const GURL& url) = 0;
-  // If the |url| is in the reading list and read, mark it unread. If it is in
-  // the reading list and unread, move it to the top of read if it is not here
-  // already.
-  virtual void MarkUnreadByURL(const GURL& url) = 0;
+  // If the |url| is in the reading list entry(|url|).read != |read|, sets the
+  // read state of the URL to read. This will also update the update time of
+  // the entry.
+  virtual void SetReadStatus(const GURL& url, bool read) = 0;
 
   // Methods to mutate an entry. Will locate the relevant entry by URL. Does
   // nothing if the entry is not found.
   virtual void SetEntryTitle(const GURL& url, const std::string& title) = 0;
   virtual void SetEntryDistilledPath(const GURL& url,
                                      const base::FilePath& distilled_path) = 0;
   virtual void SetEntryDistilledState(
       const GURL& url,
       ReadingListEntry::DistillationState state) = 0;
 
@@ skipping 33 matching lines @@
   // Called when model is leaving batch update mode.
   virtual void LeavingBatchUpdates();
 
  private:
   unsigned int current_batch_updates_count_;
 
   DISALLOW_COPY_AND_ASSIGN(ReadingListModel);
 };
 
 #endif  // IOS_CHROME_BROWSER_READING_LIST_READING_LIST_MODEL_H_