Extend BookmarkModel by last visit date |
||||||
Issue descriptionFor the new tab page of Chrome on mobile, a list of recently visited bookmarks is needed. Currently, the bookmark model does not store last visit dates for bookmarks. After discussion with zea@ and sky@, I propose to extend the model. In the beginning, the info should be stored in meta data (moving it into a dedicated field later when experiments confirm that this is a usuful feature).
,
Jul 28 2016
The code should not live in the BookmarkModel, at first.
,
Jul 29 2016
I have the CL ready, not LGTMed, yet.
,
Jul 29 2016
,
Aug 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9ace14b3b2c1fba4261d6edb19965f7373355c4b commit 9ace14b3b2c1fba4261d6edb19965f7373355c4b Author: pke <pke@google.com> Date: Fri Aug 05 09:50:53 2016 Add a tab helper to record the last visit date for each bookmark The NTP on Android will display a list of recently visited bookmarks. This CL introduces a tab helper that stores current date to the meta data of a bookmark when the bookmark gets visited. The code is split into a part that depends on content (living in chrome/browser/ntp_snippets) and a platform-independent part (living in components/ntp_snippets). The platform independent code also provides functions to retrieve the list of most recent bookmarks. The code for iOS, analogous to the first part, will come in another CL. BUG= 631474 Review-Url: https://codereview.chromium.org/2200113002 Cr-Commit-Position: refs/heads/master@{#410022} [modify] https://crrev.com/9ace14b3b2c1fba4261d6edb19965f7373355c4b/chrome/browser/bookmarks/bookmark_model_factory.cc [modify] https://crrev.com/9ace14b3b2c1fba4261d6edb19965f7373355c4b/chrome/browser/bookmarks/bookmark_model_factory.h [add] https://crrev.com/9ace14b3b2c1fba4261d6edb19965f7373355c4b/chrome/browser/ntp_snippets/bookmark_last_visit_updater.cc [add] https://crrev.com/9ace14b3b2c1fba4261d6edb19965f7373355c4b/chrome/browser/ntp_snippets/bookmark_last_visit_updater.h [modify] https://crrev.com/9ace14b3b2c1fba4261d6edb19965f7373355c4b/chrome/browser/ui/tab_helpers.cc [modify] https://crrev.com/9ace14b3b2c1fba4261d6edb19965f7373355c4b/chrome/chrome_browser.gypi [modify] https://crrev.com/9ace14b3b2c1fba4261d6edb19965f7373355c4b/components/ntp_snippets.gypi [modify] https://crrev.com/9ace14b3b2c1fba4261d6edb19965f7373355c4b/components/ntp_snippets/BUILD.gn [add] https://crrev.com/9ace14b3b2c1fba4261d6edb19965f7373355c4b/components/ntp_snippets/bookmarks/DEPS [add] https://crrev.com/9ace14b3b2c1fba4261d6edb19965f7373355c4b/components/ntp_snippets/bookmarks/bookmark_last_visit_utils.cc [add] https://crrev.com/9ace14b3b2c1fba4261d6edb19965f7373355c4b/components/ntp_snippets/bookmarks/bookmark_last_visit_utils.h
,
Aug 5 2016
,
Aug 9 2016
,
Aug 11 2016
As a fallback when last_visited is missing, creation date is considered. This is broken as bookmarks created by sync get current time as creation date. Thus, on a freshly synced device, one has all bookmarks as recently visited.
,
Aug 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdf25cacddb2b0e7a3da28199fb57a7d9407eb28 commit bdf25cacddb2b0e7a3da28199fb57a7d9407eb28 Author: jkrcal <jkrcal@chromium.org> Date: Thu Aug 11 15:08:05 2016 Marking last_visited date for bookmarks created from an open tab Previously, last_visited date was noted only when a (previously bookmarked) page is opened. This CL notes the date also when a new bookmark is created (while the URL of the new bookmark is opened in some browser tab). Previously, we used creation date of a bookmark as a fallback date when last_visited info is missing. This does not work as bookmarks created by sync get the time they are synced to the device as their creation date. Thus, on a freshly synced device, one gets all bookmarks as recently visited. This CL also removes the fallback. Lastly, the CL also adds marking last_visited dates upon server-side redirects. E.g. if you have a bit.ly/foo link to foo.com and bookmark foo.com, going there through bit.ly/foo also marks this bookmark as visited. Previously only the bit.ly/foo URL is checked. BUG= 631474 Review-Url: https://codereview.chromium.org/2234393002 Cr-Commit-Position: refs/heads/master@{#411331} [modify] https://crrev.com/bdf25cacddb2b0e7a3da28199fb57a7d9407eb28/chrome/browser/ntp_snippets/bookmark_last_visit_updater.cc [modify] https://crrev.com/bdf25cacddb2b0e7a3da28199fb57a7d9407eb28/chrome/browser/ntp_snippets/bookmark_last_visit_updater.h [modify] https://crrev.com/bdf25cacddb2b0e7a3da28199fb57a7d9407eb28/components/ntp_snippets/bookmarks/bookmark_last_visit_utils.cc [modify] https://crrev.com/bdf25cacddb2b0e7a3da28199fb57a7d9407eb28/components/ntp_snippets/bookmarks/bookmark_last_visit_utils.h
,
Aug 11 2016
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by jkrcal@chromium.org
, Jul 26 2016