[perf] Change google.com to google.co.uk in top_10_mobile_memory and dual_browser page sets.

[perf] Change google.com to google.co.uk in top_10_mobile_memory and dual_browser page sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_INCLUDE_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq

Committed: https://crrev.com/c5acd3851a2f34de15dd89119f0be48c22a9aa72
Cr-Commit-Position: refs/heads/master@{#405099}

[mythria, 2016-07-12 14:03:48]

Description was changed from

==========
[perf] Change google.com to google.co.uk in top_10_mobile and dual_browser page
sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser,
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
==========

to

==========
[perf] Change google.com to google.co.uk in top_10_mobile and dual_browser page
sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
==========

[mythria, 2016-07-12 14:03:52]

The CQ bit was checked by mythria@chromium.org to run a CQ dry run

[mythria, 2016-07-12 14:04:14]

Description was changed from

==========
[perf] Change google.com to google.co.uk in top_10_mobile and dual_browser page
sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
==========

to

==========
[perf] Change google.com to google.co.uk in top_10_mobile and dual_browser page
sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
==========

[commit-bot: I haz the power, 2016-07-12 14:04:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 14:04:22]

There were warnings when CQ was processing your CL:
 * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use
CQ_INCLUDE_TRYBOTS instead.

[mythria, 2016-07-12 14:05:03]

Description was changed from

==========
[perf] Change google.com to google.co.uk in top_10_mobile and dual_browser page
sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_EXTRA_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
==========

to

==========
[perf] Change google.com to google.co.uk in top_10_mobile and dual_browser page
sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_INCLUDE_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
==========

[mythria, 2016-07-12 14:18:58]

mythria@chromium.org changed reviewers:
+ nednguyen@google.com, petrcermak@chromium.org

[mythria, 2016-07-12 14:18:59]

Hi,

This cl is to fix the redirection problem with google.com. I updated the page
sets to use google.co.uk. I also updated the recordings. I am sorry for the
delay, I had some issues with my device to record the wpr.  PTAL.

Thanks,
Mythri

[nednguyen, 2016-07-12 14:21:30]

On 2016/07/12 14:18:59, mythria wrote:
> Hi,
> 
> This cl is to fix the redirection problem with http://google.com. I updated
the page
> sets to use google.co.uk. I also updated the recordings. I am sorry for the
> delay, I had some issues with my device to record the wpr.  PTAL.
> 
> Thanks,
> Mythri

Why does google.com got redirected to google.ca? Is it because the they recorded
the page with Canada IP?

[chromium-reviews, 2016-07-12 14:24:06]

Yes, the wpr contained google.ca. I think it was recorded with Canada IP.

On Tue, Jul 12, 2016 at 3:21 PM, <nednguyen@google.com> wrote:

> On 2016/07/12 14:18:59, mythria wrote:
> > Hi,
> >
> > This cl is to fix the redirection problem with http://google.com. I
> updated
> the page
> > sets to use google.co.uk. I also updated the recordings. I am sorry for
> the
> > delay, I had some issues with my device to record the wpr. PTAL.
> >
> > Thanks,
> > Mythri
>
> Why does google.com got redirected to google.ca? Is it because the they
> recorded
> the page with Canada IP?
>
> https://codereview.chromium.org/2138133003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[nednguyen, 2016-07-12 14:26:21]

nednguyen@google.com changed reviewers:
+ kouhei@chromium.org

[nednguyen, 2016-07-12 14:27:27]

On 2016/07/12 14:24:06, chromium-reviews wrote:
> Yes, the wpr contained google.ca. I think it was recorded with Canada IP.
> 
> On Tue, Jul 12, 2016 at 3:21 PM, <mailto:nednguyen@google.com> wrote:
> 
> > On 2016/07/12 14:18:59, mythria wrote:
> > > Hi,
> > >
> > > This cl is to fix the redirection problem with http://google.com. I
> > updated
> > the page
> > > sets to use google.co.uk. I also updated the recordings. I am sorry for
> > the
> > > delay, I had some issues with my device to record the wpr. PTAL.
> > >
> > > Thanks,
> > > Mythri
> >
> > Why does http://google.com got redirected to google.ca? Is it because the
they
> > recorded
> > the page with Canada IP?
> >
> > https://codereview.chromium.org/2138133003/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.


lgtm
CC'ed Kouhei because this may change perf metrics of page_cycler.top_10_mobile

[petrcermak, 2016-07-12 14:28:15]

petrcermak@chromium.org changed reviewers:
+ perezju@chromium.org, primiano@chromium.org

[petrcermak, 2016-07-12 14:28:16]

+primiano,perezju: I think it's fine to do this given that we've just had a
branch point, but I want to double check.

Apart from that↑, LGTM.

On 2016/07/12 14:21:30, nednguyen wrote:
> Why does http://google.com got redirected to google.ca? Is it because the they
recorded
> the page with Canada IP?

Yes, that is precisely the case.

Thanks,
Petr

https://codereview.chromium.org/2138133003/diff/1/tools/perf/page_sets/top_10...
File tools/perf/page_sets/top_10_mobile.py (right):

https://codereview.chromium.org/2138133003/diff/1/tools/perf/page_sets/top_10...
tools/perf/page_sets/top_10_mobile.py:35: # (mythria) Updated the page when
recording a new wpr. The old page
In my opinion, this should have been just a comment for the review (like this
one). I think you should remove it from the actual file (people will be confused
when they read it).

[kouhei (in TOK), 2016-07-12 14:29:05]

> CC'ed Kouhei because this may change perf metrics of page_cycler.top_10_mobile
ack

[petrcermak, 2016-07-12 14:32:26]

Actually, there's no need to re-record all pages. Should we just update the
google.com pages in those story sets (so that there's less impact on existing
benchmarks)? In theory, all the story sets could be relying on a single WPR
file.

Just to check (I know that we discussed the offline, but I don't remember the
precise answer): Couldn't we just relabel the WPRs with "google.ca" instead?
After all, since they contained a redirection from google.com to google.ca, it
should be possible to simply reply google.ca with the existing files.

Also note that top_10_mobile_memory is missing in the patch title (it will
probably become too long though).

Thanks,
Petr

[mythria, 2016-07-12 14:34:58]

Thanks. I checked with Juan on Friday, he is fine with this and asked me to go
ahead since he is OOO. I will mark it as TBR for Juan. I will wait for Primiano
before I land this.

https://codereview.chromium.org/2138133003/diff/1/tools/perf/page_sets/top_10...
File tools/perf/page_sets/top_10_mobile.py (right):

https://codereview.chromium.org/2138133003/diff/1/tools/perf/page_sets/top_10...
tools/perf/page_sets/top_10_mobile.py:35: # (mythria) Updated the page when
recording a new wpr. The old page
On 2016/07/12 14:28:16, petrcermak wrote:
> In my opinion, this should have been just a comment for the review (like this
> one). I think you should remove it from the actual file (people will be
confused
> when they read it).

Thanks Petr. Done.

[nednguyen, 2016-07-12 14:35:17]

On 2016/07/12 14:32:26, petrcermak wrote:
> Actually, there's no need to re-record all pages. Should we just update the
> http://google.com pages in those story sets (so that there's less impact on
existing
> benchmarks)? In theory, all the story sets could be relying on a single WPR
> file.
> 
> Just to check (I know that we discussed the offline, but I don't remember the
> precise answer): Couldn't we just relabel the WPRs with "google.ca" instead?
> After all, since they contained a redirection from http://google.com to
google.ca, it
> should be possible to simply reply google.ca with the existing files.
> 
> Also note that top_10_mobile_memory is missing in the patch title (it will
> probably become too long though).
> 
> Thanks,
> Petr

We have to be sure that the actions on the google.ca page are the same. Then
manually update the .json files to reference the same WPR archive with
appropriate labels.

+Emily: your work on migrating this to dep manager may simplify this situation

[Primiano Tucci (use gerrit), 2016-07-12 15:31:50]

Having a shift in the benchmarks is fine, these things will always happen, we
can't always wait for a branch point. So LGTM on this point.
THe only odd thing with this cl is that by looking at the jsoin file it looks
like more pages were re-recorded, not just google.co.uk. Is that expected?

[Primiano Tucci (use gerrit), 2016-07-12 15:42:38]

On 2016/07/12 15:31:50, Primiano Tucci wrote:
> Is that expected?
Had a chat offline with mythria, sounds like the answer here is yes and this was
discussed with perezju@

[mythria, 2016-07-12 15:56:42]

The CQ bit was checked by mythria@chromium.org to run a CQ dry run

[mythria, 2016-07-12 15:57:20]

Thank you all. I will land this once bots are green.

[commit-bot: I haz the power, 2016-07-12 15:57:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 17:18:13]

The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-12 17:18:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mythria, 2016-07-13 07:57:44]

Description was changed from

==========
[perf] Change google.com to google.co.uk in top_10_mobile and dual_browser page
sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_INCLUDE_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
==========

to

==========
[perf] Change google.com to google.co.uk in top_10_mobile_memory and
dual_browser page sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_INCLUDE_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
==========

[mythria, 2016-07-13 08:23:11]

The CQ bit was checked by mythria@chromium.org

[mythria, 2016-07-13 08:23:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
nednguyen@google.com, petrcermak@chromium.org
Link to the patchset: https://codereview.chromium.org/2138133003/#ps20001
(title: "Removed an unnecessary comment.")

[commit-bot: I haz the power, 2016-07-13 08:23:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 09:02:57]

Description was changed from

==========
[perf] Change google.com to google.co.uk in top_10_mobile_memory and
dual_browser page sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_INCLUDE_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
==========

to

==========
[perf] Change google.com to google.co.uk in top_10_mobile_memory and
dual_browser page sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_INCLUDE_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
==========

[commit-bot: I haz the power, 2016-07-13 09:02:58]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-13 09:03:01]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-13 09:04:51]

Description was changed from

==========
[perf] Change google.com to google.co.uk in top_10_mobile_memory and
dual_browser page sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_INCLUDE_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq
==========

to

==========
[perf] Change google.com to google.co.uk in top_10_mobile_memory and
dual_browser page sets.

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.co.uk in the page set. We cannot change it
to google.ca because memory_top_10_mobile uses the same page set but with
a different wpr that has a recording of google.co.uk. Updated dual_browser
page set to be consistent with top_10_mobile page set.

Also recorded new wprs for memory_top_10_mobile, dual_browser and
top_10_mobile_memory.

BUG=chromium:617914
LOG=N
CQ_INCLUDE_TRYBOTS=tryserver.chromium.perf:android_s5_perf_cq;tryserver.chromium.perf:mac_retina_perf_cq

Committed: https://crrev.com/c5acd3851a2f34de15dd89119f0be48c22a9aa72
Cr-Commit-Position: refs/heads/master@{#405099}
==========

[commit-bot: I haz the power, 2016-07-13 09:04:52]

Patchset 2 (id:??) landed as
https://crrev.com/c5acd3851a2f34de15dd89119f0be48c22a9aa72
Cr-Commit-Position: refs/heads/master@{#405099}

[perezju, 2016-07-22 09:30:31]

On 2016/07/13 09:04:52, commit-bot: I haz the power wrote:
> Patchset 2 (id:??) landed as
> https://crrev.com/c5acd3851a2f34de15dd89119f0be48c22a9aa72
> Cr-Commit-Position: refs/heads/master@{#405099}

Thanks a lot Mythri! And thanks all for the reviews.

Just to confirm and reiterate: Yes, in this case it was better/easier to just
re-record the whole thing since some of the benchmarks (particularly the android
ones: opening two browsers, going foreground/background) do not always play well
with story-filter.

Moreover, we also took the change to fix some of the old recordings (e.g. the
page for tobao was half-broken on some stories, and did not exist anymore on the
live website).