Index: chrome/browser/win/jumplist.h |
diff --git a/chrome/browser/win/jumplist.h b/chrome/browser/win/jumplist.h |
index 42473cf41ac391eef923b3256415a778ea212294..21c800d252e8837e2c3778a44e7994382da54d5a 100644 |
--- a/chrome/browser/win/jumplist.h |
+++ b/chrome/browser/win/jumplist.h |
@@ -67,38 +67,31 @@ class JumpList : public sessions::TabRestoreServiceObserver, |
public history::TopSitesObserver, |
public RefcountedKeyedService { |
grt (UTC plus 2)
2017/06/09 10:42:58
can this be a regular KeyedService? i don't see an
chengx
2017/06/10 05:56:18
I think refcounting is used when one user (i.e., o
grt (UTC plus 2)
2017/06/12 07:15:32
Making it a keyed service accomplishes that (one i
chengx
2017/06/12 22:05:28
I've made it not refcounted.
|
public: |
- struct JumpListData { |
- JumpListData(); |
- ~JumpListData(); |
- |
- // Lock for most_visited_pages_, recently_closed_pages_, icon_urls_ |
- // as they may be used by up to 2 threads. |
- base::Lock list_lock_; |
- |
- // A list of URLs we need to retrieve their favicons, |
- // protected by the list_lock_. |
- typedef std::pair<std::string, scoped_refptr<ShellLinkItem> > URLPair; |
- std::list<URLPair> icon_urls_; |
- |
- // Items in the "Most Visited" category of the application JumpList, |
- // protected by the list_lock_. |
- ShellLinkItemList most_visited_pages_; |
- |
- // Items in the "Recently Closed" category of the application JumpList, |
- // protected by the list_lock_. |
- ShellLinkItemList recently_closed_pages_; |
- |
- // A boolean flag indicating if "Most Visited" category of the JumpList |
- // has new updates therefore its icons need to be updated. |
- // By default, this flag is set to false. If there's any change in |
- // TabRestoreService, this flag will be set to true. |
- bool most_visited_pages_have_updates_ = false; |
- |
- // A boolean flag indicating if "Recently Closed" category of the JumpList |
- // has new updates therefore its icons need to be updated. |
- // By default, this flag is set to false. If there's any change in TopSites |
- // service, this flag will be set to true. |
- bool recently_closed_pages_have_updates_ = false; |
+ using URLIconCache = base::flat_map<std::string, base::FilePath>; |
+ |
+ // Holds results of the RunUpdateJumpList run. |
+ struct JumpListUpdateResults { |
grt (UTC plus 2)
2017/06/09 10:42:58
nit: just UpdateResults since this is within JumpL
chengx
2017/06/10 05:56:18
Done. Renamed to UpdateResults and made private.
|
+ JumpListUpdateResults(); |
+ ~JumpListUpdateResults(); |
+ |
+ // Icon file paths of the most visited links, indexed by tab url. |
+ // Holding a copy of most_visited_icons_ initially, it's updated by the |
+ // JumpList update run. If the update run succeeds, it overwrites |
+ // most_visited_icons_. |
+ URLIconCache most_visited_icons_in_update_; |
+ |
+ // icon file paths of the recently closed links, indexed by tab url. |
+ // Holding a copy of recently_closed_icons_ initially, it's updated by the |
+ // JumpList update run. If the update run succeeds, it overwrites |
+ // recently_closed_icons_. |
+ URLIconCache recently_closed_icons_in_update; |
+ |
+ // A flag indicating if a JumpList update run is successful. |
+ bool update_success_ = true; |
grt (UTC plus 2)
2017/06/09 10:42:58
initialize to false and set it to true in the one
chengx
2017/06/10 05:56:18
Done.
|
+ |
+ // A flag indicating if there is a timeout in notifying the JumpList update |
+ // to shell. Note that this variable is independent of update_success_. |
+ bool update_timeout_ = false; |
}; |
// Observer callback for TabRestoreService::Observer to notify when a tab is |
grt (UTC plus 2)
2017/06/09 10:42:57
nit: remove the comments on these two overrides. t
grt (UTC plus 2)
2017/06/09 10:42:58
nit: move these overrides down into the private: s
chengx
2017/06/10 05:56:18
Done with moving the TopSitesObserver overrides to
chengx
2017/06/10 05:56:19
Done. Comments updated.
|
@@ -110,10 +103,10 @@ class JumpList : public sessions::TabRestoreServiceObserver, |
void TabRestoreServiceDestroyed( |
sessions::TabRestoreService* service) override; |
- // Cancel a pending jumplist update. |
+ // Cancel a pending JumpList update. |
void CancelPendingUpdate(); |
grt (UTC plus 2)
2017/06/09 10:42:58
this should be private:
chengx
2017/06/10 05:56:18
Done.
|
- // Terminate the jumplist: cancel any pending updates and stop observing |
+ // Terminate the JumpList: cancel any pending updates and stop observing |
// the Profile and its services. This must be called before the |profile_| |
// is destroyed. |
void Terminate(); |
grt (UTC plus 2)
2017/06/09 10:42:57
private as well
chengx
2017/06/10 05:56:18
Done.
|
@@ -129,19 +122,14 @@ class JumpList : public sessions::TabRestoreServiceObserver, |
explicit JumpList(Profile* profile); // Use JumpListFactory instead |
~JumpList() override; |
- enum class JumpListCategory { kMostVisited, kRecentlyClosed }; |
+ // Adds a new ShellLinkItem for |tab| to the JumpList data provided that doing |
+ // so will not exceed |max_items|. |
+ bool AddTab(const sessions::TabRestoreService::Tab& tab, size_t max_items); |
- // Adds a new ShellLinkItem for |tab| to |data| provided that doing so will |
- // not exceed |max_items|. |
- bool AddTab(const sessions::TabRestoreService::Tab& tab, |
- size_t max_items, |
- JumpListData* data); |
- |
- // Adds a new ShellLinkItem for each tab in |window| to |data| provided that |
- // doing so will not exceed |max_items|. |
+ // Adds a new ShellLinkItem for each tab in |window| to the JumpList data |
+ // provided that doing so will not exceed |max_items|. |
void AddWindow(const sessions::TabRestoreService::Window& window, |
- size_t max_items, |
- JumpListData* data); |
+ size_t max_items); |
// Starts loading a favicon for each URL in |icon_urls_|. |
// This function sends a query to HistoryService. |
@@ -149,10 +137,9 @@ class JumpList : public sessions::TabRestoreServiceObserver, |
// decompresses collected favicons and updates a JumpList. |
void StartLoadingFavicon(); |
- // A callback function for HistoryService that notify when a requested favicon |
- // is available. |
- // To avoid file operations, this function just attaches the given data to |
- // a ShellLinkItem object. |
+ // Callback for HistoryService that notifies when a requested favicon is |
+ // available. To avoid file operations, this function just attaches the given |
+ // data to a ShellLinkItem object. |
void OnFaviconDataAvailable( |
const favicon_base::FaviconImageResult& image_result); |
@@ -174,56 +161,58 @@ class JumpList : public sessions::TabRestoreServiceObserver, |
void TopSitesChanged(history::TopSites* top_sites, |
ChangeReason change_reason) override; |
- // Called on a timer to update the most visited URLs after requests storms |
- // have subsided. |
+ // Called on a timer after requests storms have subsided. Calls APIs |
+ // DeferredTop{SitesChanged, ServiceChanged} on demand to do the actual work. |
+ void DeferredChanged(); |
grt (UTC plus 2)
2017/06/09 10:42:58
nit: i think this should either be named after whe
chengx
2017/06/10 05:56:18
I'll go with OnDelayTimer.
|
+ |
+ // Update the most visited URLs, called in DeferredChanged(). |
grt (UTC plus 2)
2017/06/09 10:42:58
i don't think it's necessary to document who calls
chengx
2017/06/10 05:56:18
I've updated the comments.
|
void DeferredTopSitesChanged(); |
grt (UTC plus 2)
2017/06/09 10:42:58
since the "deferred" part is now taken care of by
chengx
2017/06/10 05:56:18
Renamed to ProcessTopSitesNotification
|
- // Called on a timer to update the "Recently Closed" category of JumpList |
- // after requests storms have subsided. |
+ // Update the recently closed URLs, called in DeferredChanged(). |
void DeferredTabRestoreServiceChanged(); |
grt (UTC plus 2)
2017/06/09 10:42:58
ProcessTabRestoreServiceNotification?
chengx
2017/06/10 05:56:18
SGTM. I've renamed the variable.
|
- // Deletes icon files of |category| in |icon_dir| which are not in the cache |
- // anymore. |
- void DeleteIconFiles(const base::FilePath& icon_dir, |
- JumpListCategory category); |
+ // Deletes icon files in |icon_dir| which are not in |icon_cache| anymore. |
+ static void DeleteIconFiles(const base::FilePath& icon_dir, |
+ URLIconCache* icon_cache); |
- // Creates at most |max_items| icon files of |category| in |icon_dir| for the |
- // asynchrounously loaded icons stored in |item_list|. |
+ // In |icon_dir|, creates at most |max_items| icon files which are not in |
+ // |icon_cache| for the asynchrounously loaded icons stored in |item_list|. |
+ // |icon_cache| is also updated for newly created icons. |
// Returns the number of new icon files created. |
- int CreateIconFiles(const base::FilePath& icon_dir, |
- const ShellLinkItemList& item_list, |
- size_t max_items, |
- JumpListCategory category); |
- |
- // Updates icon files in |icon_dir|, which includes deleting old icons and |
- // creating at most |slot_limit| new icons for |page_list|. |
+ static int CreateIconFiles(const base::FilePath& icon_dir, |
+ const ShellLinkItemList& item_list, |
+ size_t max_items, |
+ URLIconCache* icon_cache); |
+ |
+ // Updates icon files for |page_list| in |icon_dir|, which consists of |
+ // 1) creating at most |slot_limit| new icons which are not in |icon_cache|; |
+ // 2) deleting old icons which are not in |icon_cache|. |
// Returns the number of new icon files created. |
- int UpdateIconFiles(const base::FilePath& icon_dir, |
- const ShellLinkItemList& page_list, |
- size_t slot_limit, |
- JumpListCategory category); |
- |
- // Updates the jumplist, once all the data has been fetched. This method calls |
- // UpdateJumpList() to do most of the work. |
- void RunUpdateJumpList( |
- IncognitoModePrefs::Availability incognito_availability, |
+ static int UpdateIconFiles(const base::FilePath& icon_dir, |
+ const ShellLinkItemList& page_list, |
+ size_t slot_limit, |
+ URLIconCache* icon_cache); |
+ |
+ // Updates the application JumpList, which consists of 1) create new icon |
+ // files; 2) delete obsolete icon files; 3) notify the OS. |
+ // Note that any timeout error along the way results in the old JumpList being |
+ // left as-is, while any non-timeout error results in the old JumpList being |
+ // left as-is, but without icon files. |
+ static void RunUpdateJumpList( |
const base::string16& app_id, |
const base::FilePath& profile_dir, |
- base::RefCountedData<JumpListData>* ref_counted_data); |
+ const ShellLinkItemList& most_visited_pages, |
+ const ShellLinkItemList& recently_closed_pages, |
+ bool most_visited_pages_have_updates, |
+ bool recently_closed_pages_have_updates, |
+ IncognitoModePrefs::Availability incognito_availability, |
+ JumpListUpdateResults* update_results); |
- // Updates the application JumpList, which consists of 1) delete old icon |
- // files; 2) create new icon files; 3) notify the OS. This method is called |
- // from RunUpdateJumpList(). |
- // Note that any timeout error along the way results in the old jumplist being |
- // left as-is, while any non-timeout error results in the old jumplist being |
- // left as-is, but without icon files. |
- bool UpdateJumpList(const base::string16& app_id, |
- const base::FilePath& profile_dir, |
- const ShellLinkItemList& most_visited_pages, |
- const ShellLinkItemList& recently_closed_pages, |
- bool most_visited_pages_have_updates, |
- bool recently_closed_pages_have_updates, |
- IncognitoModePrefs::Availability incognito_availability); |
+ // Callback for RunUpdateJumpList that notifies when it finishes running. |
+ // Updates certain JumpList member variables and/or triggers a new JumpList |
+ // update based on |update_results|. |
+ void OnRunUpdateCompletion( |
+ std::unique_ptr<JumpListUpdateResults> update_results); |
// Tracks FaviconService tasks. |
base::CancelableTaskTracker cancelable_task_tracker_; |
@@ -234,32 +223,50 @@ class JumpList : public sessions::TabRestoreServiceObserver, |
// Lives on the UI thread. |
grt (UTC plus 2)
2017/06/09 10:42:58
all members live on the UI thread now, so this com
chengx
2017/06/10 05:56:18
Done.
|
std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_; |
- // App id to associate with the jump list. |
+ // App id to associate with the JumpList. |
base::string16 app_id_; |
- // Timer for requesting delayed updates of the "Most Visited" category of |
- // jumplist. |
- base::OneShotTimer timer_most_visited_; |
+ // Timer for requesting delayed JumpList updates. |
+ base::OneShotTimer timer_; |
- // Timer for requesting delayed updates of the "Recently Closed" category of |
- // jumplist. |
- base::OneShotTimer timer_recently_closed_; |
+ // A list of URLs we need to retrieve their favicons, |
+ typedef std::pair<std::string, scoped_refptr<ShellLinkItem>> URLPair; |
grt (UTC plus 2)
2017/06/09 10:42:57
wdyt of:
using UrlAndLinkItem = ...
chengx
2017/06/10 05:56:18
Done.
|
+ std::list<URLPair> icon_urls_; |
- // Number of updates to skip to alleviate the machine when a previous update |
- // was too slow. Updates will be resumed when this reaches 0 again. |
- int updates_to_skip_ = 0; |
+ // Items in the "Most Visited" category of the JumpList. |
+ ShellLinkItemList most_visited_pages_; |
+ |
+ // Items in the "Recently Closed" category of the JumpList. |
+ ShellLinkItemList recently_closed_pages_; |
+ |
+ // The icon file paths of the most visited links, indexed by tab url. |
+ URLIconCache most_visited_icons_; |
+ |
+ // The icon file paths of the recently closed links, indexed by tab url. |
+ URLIconCache recently_closed_icons_; |
+ |
+ // A flag indicating if "Most Visited" category has pending notifications. |
+ bool most_visited_has_pending_notification_ = false; |
+ |
+ // A flag indicating if "Recently Closed" category has pending notifications. |
+ bool recently_closed_has_pending_notification_ = false; |
+ |
+ // A flag indicating if "Most Visited" category has new updates. |
+ bool most_visited_pages_have_updates_ = false; |
+ |
+ // A flag indicating if "Recently Closed" category has new updates. |
+ bool recently_closed_pages_have_updates_ = false; |
+ |
+ // A boolean flag indicating if there's a JumpList update task already posted |
+ // or currently running. |
+ bool update_in_progress_ = false; |
// A boolean flag indicating if a session has at least one tab closed. |
bool has_tab_closed_ = false; |
- // Holds data that can be accessed from multiple threads. |
- scoped_refptr<base::RefCountedData<JumpListData>> jumplist_data_; |
- |
- // The icon file paths of the most visited links and the recently closed links |
- // in the current jumplist, indexed by tab url, respectively. |
- // They may only be accessed on update_jumplist_task_runner_. |
- base::flat_map<std::string, base::FilePath> most_visited_icons_; |
- base::flat_map<std::string, base::FilePath> recently_closed_icons_; |
+ // Number of updates to skip to alleviate the machine when a previous update |
+ // was too slow. Updates will be resumed when this reaches 0 again. |
+ int updates_to_skip_ = 0; |
// Id of last favicon task. It's used to cancel current task if a new one |
// comes in before it finishes. |
@@ -274,7 +281,7 @@ class JumpList : public sessions::TabRestoreServiceObserver, |
SEQUENCE_CHECKER(sequence_checker_); |
- // For callbacks may be run after destruction. |
+ // For callbacks may run after destruction. |
base::WeakPtrFactory<JumpList> weak_ptr_factory_; |
DISALLOW_COPY_AND_ASSIGN(JumpList); |