Commit d38ccdc3 authored by stanisc's avatar stanisc Committed by Commit bot

Sync: Perf: Optimize handing FavIcon callbacks in BookmarkChangeProcessor

This change improves responsiveness of chrome UI thread by eliminating wasted work that BookmarkChangeProcessor
does in BookmarkNodeFaviconChanged in a scenario when user initially connects sync to an account with a large
number of bookmarks. This is achieved by ignoring the notification earlier when there is no FavIcon image, before doing any expensive work. Please note that the notification is called twice for each bookmark - first when a change in FavIcon is detected and then when the icon is uploaded to the bookmark model. The change cuts the work associated with the first call on the sync side.

Estimated saving on the UI thread is about 10-12% (as measured by reduction of samples in sampling profile).

BUG=239621

Review URL: https://codereview.chromium.org/661883002

Cr-Commit-Position: refs/heads/master@{#302685}
parent 9b7f6d98
...@@ -409,6 +409,23 @@ void BookmarkChangeProcessor::BookmarkNodeMoved(BookmarkModel* model, ...@@ -409,6 +409,23 @@ void BookmarkChangeProcessor::BookmarkNodeMoved(BookmarkModel* model,
void BookmarkChangeProcessor::BookmarkNodeFaviconChanged( void BookmarkChangeProcessor::BookmarkNodeFaviconChanged(
BookmarkModel* model, BookmarkModel* model,
const BookmarkNode* node) { const BookmarkNode* node) {
if (!CanSyncNode(node)) {
return;
}
// We shouldn't see changes to the top-level nodes.
if (model->is_permanent_node(node)) {
NOTREACHED() << "Saw Favicon update to permanent node!";
return;
}
// Ignore changes with empty images. This can happen if the favicon is
// still being loaded.
const gfx::Image& favicon = model->GetFavicon(node);
if (favicon.IsEmpty()) {
return;
}
BookmarkNodeChanged(model, node); BookmarkNodeChanged(model, node);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment