Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(138)

Issue 2092443002: [perf] Change google.com to google.ca in top_10_mobile page set. (Closed)

Created:
4 years, 6 months ago by mythria
Modified:
4 years, 5 months ago
Reviewers:
nednguyen, petrcermak
CC:
chromium-reviews, telemetry-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[perf] Change google.com to google.ca in top_10_mobile page set. In the top10_mobile page set google.com gets redirected to google.ca. This sometimes causes problems in telemetry with scrolling. As a short term fix, replace google.com with google.ca in the page set. BUG=chromium:617914 LOG=N CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq Committed: https://crrev.com/d0c95636111161400a576472427de8eeb2330d43 Cr-Commit-Position: refs/heads/master@{#401863}

Patch Set 1 #

Patch Set 2 : Fixed few more that are dependent on the same pageset. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -6 lines) Patch
M tools/perf/page_sets/data/memory_top_10_mobile.json View 1 2 chunks +2 lines, -2 lines 0 comments Download
M tools/perf/page_sets/data/top_10_mobile.json View 1 chunk +2 lines, -2 lines 0 comments Download
M tools/perf/page_sets/text_selection_sites.py View 1 1 chunk +1 line, -1 line 0 comments Download
M tools/perf/page_sets/top_10_mobile.py View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 31 (16 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2092443002/1
4 years, 6 months ago (2016-06-22 15:08:23 UTC) #3
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/91852)
4 years, 6 months ago (2016-06-22 15:55:00 UTC) #5
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2092443002/1
4 years, 6 months ago (2016-06-23 08:43:44 UTC) #7
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/248987)
4 years, 6 months ago (2016-06-23 09:24:10 UTC) #9
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2092443002/20001
4 years, 5 months ago (2016-06-24 09:56:43 UTC) #11
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 5 months ago (2016-06-24 10:58:33 UTC) #13
mythria
Hi Petr, I changed google.com to google.ca to avoid problems with scrolling due to redirection. ...
4 years, 5 months ago (2016-06-24 11:13:10 UTC) #15
petrcermak
LGTM. Out of curiosity, what does "LOG=N" in the patch description mean? I haven't seen ...
4 years, 5 months ago (2016-06-24 12:43:20 UTC) #17
petrcermak
Oh and I suggest you change the patch description to "[perf] Change google.com to google.ca ...
4 years, 5 months ago (2016-06-24 12:44:39 UTC) #18
mythria
Thanks Petr. Updated the description. I think LOG=N means that this need not be logged ...
4 years, 5 months ago (2016-06-24 13:14:07 UTC) #23
nednguyen
lgtm
4 years, 5 months ago (2016-06-24 14:35:32 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2092443002/20001
4 years, 5 months ago (2016-06-24 14:53:59 UTC) #26
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 5 months ago (2016-06-24 14:58:29 UTC) #28
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/d0c95636111161400a576472427de8eeb2330d43 Cr-Commit-Position: refs/heads/master@{#401863}
4 years, 5 months ago (2016-06-24 15:01:18 UTC) #30
perezju
4 years, 5 months ago (2016-06-29 16:43:44 UTC) #31
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2112583002/ by perezju@chromium.org.

The reason for reverting is: Caused artificial memory drop in
memory.top_10_mobile benchmark.

Sorry if this breaks or flakes other benchmarks. We can re-land (with
re-recorded pages) when the dust settles after health plan for this milestone is
completed..

Powered by Google App Engine
This is Rietveld 408576698