Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2680713002:
Should not invalidate font cache for purge+suspend. (Closed)
DescriptionShould not invalidate font cache for purge+suspend.
Invalidating font cache for purge+suspend always causes full layout
when the tab is foregrounded. The full layout increases tab-switching cost
significantly (observed when browsing en.wikipedia.org/wiki/wikipedia).
So we should not invoke fontCache()->invalidate() when purge+suspended.
BUG=635419
Review-Url: https://codereview.chromium.org/2680713002
Cr-Commit-Position: refs/heads/master@{#448944}
Committed: https://chromium.googlesource.com/chromium/src/+/235cb99dc91534cc729e07a7b1ca5878ee57fcf5
Patch Set 1 #
Total comments: 2
Patch Set 2 : Fixed. #Messages
Total messages: 20 (13 generated)
The CQ bit was checked by tasak@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Should not invalidate font cache for purge+suspend. Invalidating font cache for purge+suspend always causes full layout when the tab is foregrounded. The full layout increases tab-switching cost significantly (observed when browsing en.wikipedia.org/wiki/wikipedia). So we should not invoke fontCache()->invalidate() when purge+suspended. BUG= ========== to ========== Should not invalidate font cache for purge+suspend. Invalidating font cache for purge+suspend always causes full layout when the tab is foregrounded. The full layout increases tab-switching cost significantly (observed when browsing en.wikipedia.org/wiki/wikipedia). So we should not invoke fontCache()->invalidate() when purge+suspended. BUG=635419 ==========
tasak@google.com changed reviewers: + bashi@chromium.org, haraken@chromium.org
Would you review this CL?
LGTM https://codereview.chromium.org/2680713002/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/MemoryCoordinator.cpp (right): https://codereview.chromium.org/2680713002/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/MemoryCoordinator.cpp:69: // tab switching cost significantly. So for purge+throttle we should not So we should not invalidate the font cache in purge+throttle. We should invalidate the font cache only when receiving a critical memory pressure.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tasak@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Thank you for review. https://codereview.chromium.org/2680713002/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/MemoryCoordinator.cpp (right): https://codereview.chromium.org/2680713002/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/MemoryCoordinator.cpp:69: // tab switching cost significantly. So for purge+throttle we should not On 2017/02/07 08:33:21, haraken wrote: > > So we should not invalidate the font cache in purge+throttle. We should > invalidate the font cache only when receiving a critical memory pressure. Done.
LGTM
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tasak@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486548920383450,
"parent_rev": "1bdb6a7b7bba641be5dea26d9ae94b82ec3ee084", "commit_rev":
"235cb99dc91534cc729e07a7b1ca5878ee57fcf5"}
Message was sent while issue was closed.
Description was changed from ========== Should not invalidate font cache for purge+suspend. Invalidating font cache for purge+suspend always causes full layout when the tab is foregrounded. The full layout increases tab-switching cost significantly (observed when browsing en.wikipedia.org/wiki/wikipedia). So we should not invoke fontCache()->invalidate() when purge+suspended. BUG=635419 ========== to ========== Should not invalidate font cache for purge+suspend. Invalidating font cache for purge+suspend always causes full layout when the tab is foregrounded. The full layout increases tab-switching cost significantly (observed when browsing en.wikipedia.org/wiki/wikipedia). So we should not invoke fontCache()->invalidate() when purge+suspended. BUG=635419 Review-Url: https://codereview.chromium.org/2680713002 Cr-Commit-Position: refs/heads/master@{#448944} Committed: https://chromium.googlesource.com/chromium/src/+/235cb99dc91534cc729e07a7b1ca... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/235cb99dc91534cc729e07a7b1ca...
Message was sent while issue was closed.
lgtm |
