Desktop NTP: Improve thumbnails hit rate |
|||||||||
Issue descriptionThe metrics introduced in bug 703165 show that a large percentage of NTP thumbnails fail to load. Investigate why we don't have thumbnails for so many pages, and fix it!
,
May 4 2017
First potential problem I found: ThumbnailTabHelper (the class that manages taking thumbnails) only takes a thumbnail when a tab is hidden (i.e. closed, or switched away from). It does *not* take a thumbnail when the page finishes loading. So if you navigate away from a TopSite (e.g. by clicking a link), then we'll never attempt to take a thumbnail for it. I expect that this may be common for "portal" pages like reddit, news aggregators, etc.
,
May 5 2017
,
May 5 2017
,
May 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/32d2e42a7df134836e116dcadd1ad70cd489d1c3 commit 32d2e42a7df134836e116dcadd1ad70cd489d1c3 Author: treib <treib@chromium.org> Date: Mon May 08 10:00:35 2017 ThumbnailTabHelper: Capture thumbnail when page load finishes Hidden behind a flag. BUG= 718413 Review-Url: https://codereview.chromium.org/2859263002 Cr-Commit-Position: refs/heads/master@{#469943} [modify] https://crrev.com/32d2e42a7df134836e116dcadd1ad70cd489d1c3/chrome/browser/about_flags.cc [modify] https://crrev.com/32d2e42a7df134836e116dcadd1ad70cd489d1c3/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/32d2e42a7df134836e116dcadd1ad70cd489d1c3/chrome/browser/flag_descriptions.h [modify] https://crrev.com/32d2e42a7df134836e116dcadd1ad70cd489d1c3/chrome/browser/thumbnails/thumbnail_tab_helper.cc [modify] https://crrev.com/32d2e42a7df134836e116dcadd1ad70cd489d1c3/chrome/browser/thumbnails/thumbnail_tab_helper.h [modify] https://crrev.com/32d2e42a7df134836e116dcadd1ad70cd489d1c3/chrome/common/chrome_features.cc [modify] https://crrev.com/32d2e42a7df134836e116dcadd1ad70cd489d1c3/chrome/common/chrome_features.h [modify] https://crrev.com/32d2e42a7df134836e116dcadd1ad70cd489d1c3/tools/metrics/histograms/enums.xml
,
May 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/481b55a2fa48d01f9c8357dd8aad039ffaf90f69 commit 481b55a2fa48d01f9c8357dd8aad039ffaf90f69 Author: treib <treib@chromium.org> Date: Mon May 15 13:53:51 2017 TopSites: various small cleanups - Update some comments. - Replace some iterators with range-based fors. - Add some "const"s. - etc BUG= 718413 Review-Url: https://codereview.chromium.org/2881753002 Cr-Commit-Position: refs/heads/master@{#471748} [modify] https://crrev.com/481b55a2fa48d01f9c8357dd8aad039ffaf90f69/chrome/browser/search/thumbnail_source.cc [modify] https://crrev.com/481b55a2fa48d01f9c8357dd8aad039ffaf90f69/components/history/core/browser/top_sites.h [modify] https://crrev.com/481b55a2fa48d01f9c8357dd8aad039ffaf90f69/components/history/core/browser/top_sites_cache.h [modify] https://crrev.com/481b55a2fa48d01f9c8357dd8aad039ffaf90f69/components/history/core/browser/top_sites_impl.cc [modify] https://crrev.com/481b55a2fa48d01f9c8357dd8aad039ffaf90f69/components/history/core/browser/top_sites_impl.h [modify] https://crrev.com/481b55a2fa48d01f9c8357dd8aad039ffaf90f69/components/history/core/browser/top_sites_impl_unittest.cc
,
May 16 2017
Issue 140003 has been merged into this issue.
,
May 16 2017
what is this ???????????????
,
May 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a79f66a7649512ba0e1cebd1e1d3c3cccb1bdc1 commit 4a79f66a7649512ba0e1cebd1e1d3c3cccb1bdc1 Author: treib <treib@chromium.org> Date: Tue May 16 17:09:25 2017 Thumbnails: Add some perf UMA BUG= 718413 Review-Url: https://codereview.chromium.org/2882183002 Cr-Commit-Position: refs/heads/master@{#472138} [modify] https://crrev.com/4a79f66a7649512ba0e1cebd1e1d3c3cccb1bdc1/chrome/browser/thumbnails/thumbnail_tab_helper.cc [modify] https://crrev.com/4a79f66a7649512ba0e1cebd1e1d3c3cccb1bdc1/chrome/browser/thumbnails/thumbnail_tab_helper.h [modify] https://crrev.com/4a79f66a7649512ba0e1cebd1e1d3c3cccb1bdc1/tools/metrics/histograms/histograms.xml
,
Jun 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6fb10fe065b1cfe4c4aa691c31c03e7387a72d40 commit 6fb10fe065b1cfe4c4aa691c31c03e7387a72d40 Author: Marc Treib <treib@chromium.org> Date: Mon Jun 26 18:39:55 2017 Fieldtrial testing config for CaptureThumbnailOnLoadFinished Bug: 718413 Change-Id: I98eae051b6e19cedeb8b9f4cff2da6dcc7886d11 Reviewed-on: https://chromium-review.googlesource.com/548719 Reviewed-by: Ilya Sherman <isherman@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#482336} [modify] https://crrev.com/6fb10fe065b1cfe4c4aa691c31c03e7387a72d40/testing/variations/fieldtrial_testing_config.json
,
Jun 29 2017
,
Jun 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7a4f9830425ae18fde542707bb1fbcb617d07768 commit 7a4f9830425ae18fde542707bb1fbcb617d07768 Author: Marc Treib <treib@chromium.org> Date: Thu Jun 29 22:42:07 2017 Revert "Fieldtrial testing config for CaptureThumbnailOnLoadFinished" This reverts commit 6fb10fe065b1cfe4c4aa691c31c03e7387a72d40. Reason for revert: Perf regressions, see crbug.com/737396 Original change's description: > Fieldtrial testing config for CaptureThumbnailOnLoadFinished > > Bug: 718413 > Change-Id: I98eae051b6e19cedeb8b9f4cff2da6dcc7886d11 > Reviewed-on: https://chromium-review.googlesource.com/548719 > Reviewed-by: Ilya Sherman <isherman@chromium.org> > Commit-Queue: Marc Treib <treib@chromium.org> > Cr-Commit-Position: refs/heads/master@{#482336} TBR=isherman@chromium.org,treib@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 718413 , 737396 Change-Id: I186e304e227e2fe979341dd22b0f775b67fcc070 Reviewed-on: https://chromium-review.googlesource.com/556159 Reviewed-by: Marc Treib <treib@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#483524} [modify] https://crrev.com/7a4f9830425ae18fde542707bb1fbcb617d07768/testing/variations/fieldtrial_testing_config.json
,
Jul 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fce9531fe6af8d6b47a992888a21174f8e6ec1c5 commit fce9531fe6af8d6b47a992888a21174f8e6ec1c5 Author: Marc Treib <treib@chromium.org> Date: Wed Jul 05 08:09:55 2017 Thumbnails: Move score computation off the UI thread Metrics show that the computation (specifically, color_utils::CalculateBoringScore) can be fairly expensive, so move it off the UI thread, into a background task runner. Bug: 737396 , 718413 Change-Id: I82baca452bcd4bdda0fd225530e69aad189e95a5 Reviewed-on: https://chromium-review.googlesource.com/558924 Reviewed-by: Friedrich Horschig <fhorschig@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#484213} [modify] https://crrev.com/fce9531fe6af8d6b47a992888a21174f8e6ec1c5/chrome/browser/thumbnails/thumbnail_tab_helper.cc
,
Jul 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da2086682fef5cc44b539dc4bdc63af380e2988b commit da2086682fef5cc44b539dc4bdc63af380e2988b Author: Marc Treib <treib@chromium.org> Date: Tue Jul 11 09:16:07 2017 Fieldtrial testing config for CaptureThumbnailOnLoadFinished This is mostly a reland of https://chromium-review.googlesource.com/548719 which was reverted because of perf impact. In the meantime, https://chromium-review.googlesource.com/558924 has landed which hopefully improves performance. It also adds the CaptureThumbnailDependingOnTransitionType feature, since the two will likely launch together (and that might also alleviate the perf impact). Bug: 718413 , 737396 Change-Id: I0ef909f0015d07a3f908d0ba3dc52f6a897a53b1 Reviewed-on: https://chromium-review.googlesource.com/563202 Reviewed-by: Ilya Sherman <isherman@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#485568} [modify] https://crrev.com/da2086682fef5cc44b539dc4bdc63af380e2988b/testing/variations/fieldtrial_testing_config.json
,
Jul 13 2017
,
Jul 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c81cac01282f636fe594df9ae56b513cbfa4031 commit 1c81cac01282f636fe594df9ae56b513cbfa4031 Author: Marc Treib <treib@chromium.org> Date: Thu Jul 13 14:49:27 2017 Revert "Fieldtrial testing config for CaptureThumbnailOnLoadFinished" This reverts commit da2086682fef5cc44b539dc4bdc63af380e2988b. Reason for revert: Still perf regressions, though fewer of them. See crbug.com/741856 Original change's description: > Fieldtrial testing config for CaptureThumbnailOnLoadFinished > > This is mostly a reland of https://chromium-review.googlesource.com/548719 > which was reverted because of perf impact. In the meantime, > https://chromium-review.googlesource.com/558924 has landed which hopefully > improves performance. > > It also adds the CaptureThumbnailDependingOnTransitionType feature, since > the two will likely launch together (and that might also alleviate the > perf impact). > > Bug: 718413 , 737396 > Change-Id: I0ef909f0015d07a3f908d0ba3dc52f6a897a53b1 > Reviewed-on: https://chromium-review.googlesource.com/563202 > Reviewed-by: Ilya Sherman <isherman@chromium.org> > Commit-Queue: Marc Treib <treib@chromium.org> > Cr-Commit-Position: refs/heads/master@{#485568} TBR=isherman@chromium.org,treib@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 718413 , 741856 Change-Id: Ic2479ad7c0e3e32c7621a2e80385d64da5029aea Reviewed-on: https://chromium-review.googlesource.com/570218 Reviewed-by: Marc Treib <treib@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#486374} [modify] https://crrev.com/1c81cac01282f636fe594df9ae56b513cbfa4031/testing/variations/fieldtrial_testing_config.json
,
Jul 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e3651be71ad96579738d64ef59bfb61cc49f92a5 commit e3651be71ad96579738d64ef59bfb61cc49f92a5 Author: Marc Treib <treib@chromium.org> Date: Fri Jul 14 11:05:40 2017 Thumbnails: Capture just before navigating away Hidden behind new feature CaptureThumbnailOnNavigatingAway. This is very similar to what NavigationEntryScreenshotManager does. Hopefully this is a better time for capturing screenshots than when a page load finishes (see crbug.com/737396 and crbug.com/741856). Bug: 718413 Change-Id: I0fbcc7964b9f89c7ce589a197ac85b85a41385ba Reviewed-on: https://chromium-review.googlesource.com/570318 Reviewed-by: Friedrich Horschig <fhorschig@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#486729} [modify] https://crrev.com/e3651be71ad96579738d64ef59bfb61cc49f92a5/chrome/browser/about_flags.cc [modify] https://crrev.com/e3651be71ad96579738d64ef59bfb61cc49f92a5/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/e3651be71ad96579738d64ef59bfb61cc49f92a5/chrome/browser/flag_descriptions.h [modify] https://crrev.com/e3651be71ad96579738d64ef59bfb61cc49f92a5/chrome/browser/thumbnails/thumbnail_tab_helper.cc [modify] https://crrev.com/e3651be71ad96579738d64ef59bfb61cc49f92a5/chrome/browser/thumbnails/thumbnail_tab_helper.h [modify] https://crrev.com/e3651be71ad96579738d64ef59bfb61cc49f92a5/chrome/common/chrome_features.cc [modify] https://crrev.com/e3651be71ad96579738d64ef59bfb61cc49f92a5/chrome/common/chrome_features.h [modify] https://crrev.com/e3651be71ad96579738d64ef59bfb61cc49f92a5/tools/metrics/histograms/enums.xml
,
Jul 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/632bc455b5070bd3c09aeae61728fef3f83776da commit 632bc455b5070bd3c09aeae61728fef3f83776da Author: Marc Treib <treib@chromium.org> Date: Tue Jul 18 09:25:10 2017 Fieldtrial testing config for NTPCaptureThumbnail This is a replacement for CaptureThumbnailOnLoadFinished, which was reverted due to perf regressions (see https://chromium-review.googlesource.com/c/570218/). Bug: 718413 Change-Id: I399507f5791b5ca1fc7779331b82bb9782a27268 Reviewed-on: https://chromium-review.googlesource.com/571223 Reviewed-by: Ilya Sherman <isherman@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#487424} [modify] https://crrev.com/632bc455b5070bd3c09aeae61728fef3f83776da/testing/variations/fieldtrial_testing_config.json
,
Jul 27 2017
,
Aug 23 2017
Is the work for this M60 bug complete now? All its blocking bugs are closed, is there any further work remaining?
,
Aug 23 2017
Yup, this is done, though for M61, so not rolled out to Stable yet. But we have the launch bug for tracking that.
,
Sep 15 2017
,
Nov 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/761d0bcb85290a58043b287abe0f857aeace2a4d commit 761d0bcb85290a58043b287abe0f857aeace2a4d Author: Marc Treib <treib@chromium.org> Date: Mon Nov 20 19:15:08 2017 Add fieldtrial testing config for NTPCaptureThumbnail (again) Bug: 718413 Change-Id: I51a6a4b65c51495098778226d8976d2a7136cb78 Reviewed-on: https://chromium-review.googlesource.com/779183 Commit-Queue: Ilya Sherman <isherman@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Cr-Commit-Position: refs/heads/master@{#517885} [modify] https://crrev.com/761d0bcb85290a58043b287abe0f857aeace2a4d/testing/variations/fieldtrial_testing_config.json
,
Nov 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0801e739a9685f250f8559e08a6b0cf2eba5b716 commit 0801e739a9685f250f8559e08a6b0cf2eba5b716 Author: Marc Treib <treib@chromium.org> Date: Tue Nov 21 16:24:01 2017 Thumbnail capturing: Disable features by default The previous launch was aborted, let's start over fresh for M64. Bug: 718413 Change-Id: I4bdfc074c57051d20150e9352178acc0c1ee81ac Reviewed-on: https://chromium-review.googlesource.com/781759 Reviewed-by: Mikel Astiz <mastiz@chromium.org> Commit-Queue: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#518271} [modify] https://crrev.com/0801e739a9685f250f8559e08a6b0cf2eba5b716/chrome/common/chrome_features.cc |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by treib@chromium.org
, May 4 2017