All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=749331

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=78884277aa5c3aa8cf474abc71219be49a51fd898e564013a41a63fb1f568651


Bot(s) for this bug's original alert(s):

android-nexus7v2

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8972961866089287728

=== Auto-CCing suspected CL author mastiz@chromium.org ===

Hi mastiz@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : mastiz
  Commit : c63d78c7a25a50f2fdfa0ab5cbb5566a21a6774e
  Date   : Mon Jul 24 09:23:04 2017
  Subject: Always prefer 192x192 on mobile, also for touch icons

Bisect Details
  Configuration: android_nexus7_perf_bisect
  Benchmark    : memory.top_10_mobile
  Metric       : memory:chrome:all_processes:reported_by_chrome:java_heap:effective_size_avg/background/after_http_yandex_ru_touchsearch_text_science
  Change       : 0.89% | 31455004.4444 -> 31736263.1111

Revision             Result                  N
chromium@488906      31455004 +- 132740      9      good
chromium@488921      31433387 +- 133785      6      good
chromium@488928      31470023 +- 360236      9      good
chromium@488932      31476850 +- 183798      9      good
chromium@488934      31494144 +- 326500      9      good
chromium@488935      31736263 +- 439569      9      bad       <--

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests memory.top_10_mobile

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8972961866089287728


For feedback, file a bug with component Speed>Bisection

The regressing page, http://yandex.ru/touchsearch?text=science on a Nexus 7, lists two apple touch icons:
72x72: //yastatic.net/web4/_/tGZd6z3UqFQg6IhCYuMnyZGsfuQ.png
114x114: //yastatic.net/web4/_/4q1PtQIItK7Ln3YpUOvQc0QrpK4.png

That page alone cannot have regressed because both the new and the old behavior would choose the first, 114x144.

I need to investigate if the diff could be produces by some other page in a redirect chain. For example, yandex.ru does list touch icons that would be affected:
76x76: //yastatic.net/iconostasis/_/5mdPq4V7ghRgzBvMkCaTzd2fjYg.png 
120x120: //yastatic.net/iconostasis/_/s-hGoCQMUosTziuARBks08IUxmc.png
152x152: //yastatic.net/iconostasis/_/KnU823iWwj_vrPra7x9aQ-4yjRw.png
180x180: //yastatic.net/iconostasis/_/wT9gfGZZ80sP0VsoR6dgDyXJf2Y.png

The size increase (152x152->180x180) doesn't explain the observed memory regression (~300K) so it's more likely that what happens is that two yandex pages now list different icons and the new behavior chooses a different icon for them (i.e. one additional icon gets cached).

I'll investigate this further but I'm tempted to think it's a legit behavioral change to improve consistency.

From the waterfall, you could compare traces before/after your change, maybe you can find a clue there?

Before: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_19-2017-07-24_04-31-36-4587.html
After: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_19-2017-07-24_10-22-28-94714.html

Thanks for the pointers! By taking a look at those links at specifically the browser process (Which is the only one that could've been affected by the patch), there is indeed a ~300 KB increase in Java heap. I hope I'm looking at the right place.

Nevertheles, what bothers me the most is that I don't understand why the patch would cause any behavioral differences to http://yandex.ru/touchsearch?text=science considering which icons it lists.

Using an actual Nexus 7 and the regressing page (http://yandex.ru/touchsearch?text=science), there's only one apple touch icon downloaded and cached, which is:
https://yastatic.net/web4/_/4q1PtQIItK7Ln3YpUOvQc0QrpK4.png (114x114)

I don't see how the patch could cause any diff in that page specifically.

After some more investigation, I have one possible explanation to this.

The patch in question had an unanticipated side effect which is a bugfix for  crbug.com/735354  which was believed to be fixed at the time the feature was enabled by default (recently, in https://codereview.chromium.org/2958503002).

The reason is that FaviconHandler::GetMaximalIconSize() still returned 144x144 leading to truncation of the image.

I'll go ahead with a rollback and then roll forward after the underlying bug is fixed in a separate patch.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/195ee4a01e7dfb6a1a13a32e01526464e7715f12

commit 195ee4a01e7dfb6a1a13a32e01526464e7715f12
Author: mastiz <mastiz@chromium.org>
Date: Thu Jul 27 16:40:13 2017

Revert of Always prefer 192x192 on mobile, also for touch icons (patchset #2 id:20001 of https://codereview.chromium.org/2972643002/ )

Reason for revert:
Perf regression, suspect of accidentally fixing a bug that was believed to be previously fixed ( crbug.com/735354 ) due to the max image size used during download.

BUG= 749331 

Original issue's description:
> Always prefer 192x192 on mobile, also for touch icons
>
> Previously, the ideal size for a touch icon was 144x144, which was
> originally introduced honoring iPad icon size.
>
> Always going for 192x192 (for both touch and non-touch icons) is
> simpler, more consistent and more compatible with Apple's current
> recommendations for iOS [1], which for example recommend a 180x180 icon
> for iPhone 6 Plus (scale factor of 3x).
>
> [1] https://developer.apple.com/ios/human-interface-guidelines/graphics/app-icon/
>
> BUG= 736290 
>
> Review-Url: https://codereview.chromium.org/2972643002
> Cr-Commit-Position: refs/heads/master@{#488935}
> Committed: https://chromium.googlesource.com/chromium/src/+/c63d78c7a25a50f2fdfa0ab5cbb5566a21a6774e

TBR=pkotwicz@chromium.org,noyau@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 736290 

Review-Url: https://codereview.chromium.org/2986033002
Cr-Commit-Position: refs/heads/master@{#489971}

[modify] https://crrev.com/195ee4a01e7dfb6a1a13a32e01526464e7715f12/components/favicon/core/favicon_handler.cc
[modify] https://crrev.com/195ee4a01e7dfb6a1a13a32e01526464e7715f12/components/favicon/core/favicon_handler.h
[modify] https://crrev.com/195ee4a01e7dfb6a1a13a32e01526464e7715f12/components/favicon/core/favicon_handler_unittest.cc

Regression is gone after revert.

I will likely strike back soon, but I'm closing this for now.

 Issue 750234  has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/67bebf2024d59842f759169154b4b030fe7aafa4

commit 67bebf2024d59842f759169154b4b030fe7aafa4
Author: Mikel Astiz <mastiz@chromium.org>
Date: Fri Jan 05 10:46:31 2018

Reland "Always prefer 192x192 on mobile, also for touch icons"

Reland attempt after a perf regression motivated a revert,
https://codereview.chromium.org/2986033002.

The underlying bug ( crbug.com/735354 ) is believed to be fixed now so no
(illegit) perf regression is expected now.

Original change's description:
> Always prefer 192x192 on mobile, also for touch icons
>
> Previously, the ideal size for a touch icon was 144x144, which was
> originally introduced honoring iPad icon size.
>
> Always going for 192x192 (for both touch and non-touch icons) is
> simpler, more consistent and more compatible with Apple's current
> recommendations for iOS [1], which for example recommend a 180x180 icon
> for iPhone 6 Plus (scale factor of 3x).
>
> [1] https://developer.apple.com/ios/human-interface-guidelines/graphics/app-icon/
>
> BUG= 736290 
>
> Review-Url: https://codereview.chromium.org/2972643002
> Cr-Commit-Position: refs/heads/master@{#488935}

Bug:  736290 
Bug:  749331 
Change-Id: I18e9db377e890af0dd95aa2d3329f203611d1d4f
Reviewed-on: https://chromium-review.googlesource.com/836368
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527255}
[modify] https://crrev.com/67bebf2024d59842f759169154b4b030fe7aafa4/components/favicon/core/favicon_handler.cc
[modify] https://crrev.com/67bebf2024d59842f759169154b4b030fe7aafa4/components/favicon/core/favicon_handler_unittest.cc