Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(2)

Issue 9956045: Add Web Page Replay test to page cycler. (Closed)

Created:
7 years, 4 months ago by slamm_google
Modified:
7 years, 4 months ago
Reviewers:
cmp, tonyg, James Simonsen
CC:
chromium-reviews, Aaron Boodman, pam+watch_chromium.org, mihaip+watch_chromium.org, sullivan
Visibility:
Public.

Description

Add Web Page Replay test to page cycler. - Add supporting WPR extension. BUG= TEST= Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=132730

Patch Set 1 #

Patch Set 2 : Ready for review. #

Total comments: 6

Patch Set 3 : Updated based on simonjam's suggestions. #

Patch Set 4 : Make it easier to run replay tests manually. #

Patch Set 5 : Unfork head.js, report.html. Plus, other fixes. #

Patch Set 6 : Ready for review. #

Total comments: 36

Patch Set 7 : Implement review suggestions. #

Total comments: 8

Patch Set 8 : Implement review suggestions. #

Total comments: 2

Patch Set 9 : Kb -> KB #

Patch Set 10 : Fix compile error. #

Patch Set 11 : Fix Windows compile error. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1141 lines, -17 lines) Patch
M chrome/test/perf/page_cycler_test.cc View 1 2 3 4 5 6 7 8 9 10 7 chunks +101 lines, -8 lines 0 comments Download
M tools/page_cycler/common/head.js View 1 2 3 4 1 chunk +4 lines, -6 lines 0 comments Download
M tools/page_cycler/common/report.html View 1 2 3 4 5 6 7 chunks +13 lines, -3 lines 0 comments Download
A tools/page_cycler/webpagereplay/README View 1 2 3 4 1 chunk +15 lines, -0 lines 0 comments Download
A tools/page_cycler/webpagereplay/extension/background.html View 1 chunk +2 lines, -0 lines 0 comments Download
A tools/page_cycler/webpagereplay/extension/background.js View 1 2 3 4 5 6 7 8 1 chunk +328 lines, -0 lines 0 comments Download
A tools/page_cycler/webpagereplay/extension/content.js View 1 1 chunk +14 lines, -0 lines 0 comments Download
A tools/page_cycler/webpagereplay/extension/manifest.json View 1 2 3 4 5 6 7 1 chunk +26 lines, -0 lines 0 comments Download
A tools/page_cycler/webpagereplay/extension/page_cycler.js View 1 2 3 4 5 6 1 chunk +54 lines, -0 lines 0 comments Download
A tools/page_cycler/webpagereplay/extension/start.js View 1 2 3 4 5 1 chunk +89 lines, -0 lines 0 comments Download
A tools/page_cycler/webpagereplay/start.html View 1 2 3 1 chunk +26 lines, -0 lines 0 comments Download
A tools/page_cycler/webpagereplay/start.js View 1 2 3 4 1 chunk +123 lines, -0 lines 0 comments Download
A tools/page_cycler/webpagereplay/tests/2012Q2.js View 1 2 3 1 chunk +77 lines, -0 lines 0 comments Download
A tools/python/google/webpagereplay_utils.py View 1 2 3 4 5 6 7 1 chunk +269 lines, -0 lines 0 comments Download

Messages

Total messages: 22 (0 generated)
slamm_google
This is CL #2 of 3: 1. "webpagereplay/2012Q2/start.html" test and supporting files. (Google internal tree) ...
7 years, 4 months ago (2012-04-04 22:34:37 UTC) #1
James Simonsen
LGTM https://chromiumcodereview.appspot.com/9956045/diff/1001/chrome/test/perf/page_cycler_test.cc File chrome/test/perf/page_cycler_test.cc (right): https://chromiumcodereview.appspot.com/9956045/diff/1001/chrome/test/perf/page_cycler_test.cc#newcode450 chrome/test/perf/page_cycler_test.cc:450: // launches replay.py to support these tests. nit: ...
7 years, 4 months ago (2012-04-05 00:07:22 UTC) #2
slamm_google
Thanks for the comments. I even found a bug where I had used the wrong ...
7 years, 4 months ago (2012-04-05 00:32:25 UTC) #3
cmp_google
lgtm
7 years, 4 months ago (2012-04-06 01:10:21 UTC) #4
slamm_google
Please take another look. I moved everything except the archive data to the public tree. ...
7 years, 4 months ago (2012-04-11 00:39:06 UTC) #5
slamm_google
I am still wrapping up the fixes to page_cycler_test.cc. I am getting a compile error ...
7 years, 4 months ago (2012-04-11 01:33:24 UTC) #6
James Simonsen
I'm guessing you can't assert from a function that returns non-void. Can you use a ...
7 years, 4 months ago (2012-04-11 03:19:30 UTC) #7
slamm_google
[Resending with "Reply to all".] James, that was it. I did not realize a MACRO ...
7 years, 4 months ago (2012-04-11 16:45:53 UTC) #8
slamm_google
Everyone please take another look. Almost all the changes are in this CL now. The ...
7 years, 4 months ago (2012-04-12 21:16:09 UTC) #9
James Simonsen
LGTM https://chromiumcodereview.appspot.com/9956045/diff/18001/tools/python/google/webpagereplay_utils.py File tools/python/google/webpagereplay_utils.py (right): https://chromiumcodereview.appspot.com/9956045/diff/18001/tools/python/google/webpagereplay_utils.py#newcode70 tools/python/google/webpagereplay_utils.py:70: def OpenLogFile_(self): I think the _ goes on ...
7 years, 4 months ago (2012-04-12 23:14:19 UTC) #10
tonyg
Looking great. A few comments to consider. https://chromiumcodereview.appspot.com/9956045/diff/18001/chrome/test/perf/page_cycler_test.cc File chrome/test/perf/page_cycler_test.cc (right): https://chromiumcodereview.appspot.com/9956045/diff/18001/chrome/test/perf/page_cycler_test.cc#newcode461 chrome/test/perf/page_cycler_test.cc:461: switches::kHostResolverRules, "MAP ...
7 years, 4 months ago (2012-04-13 14:32:21 UTC) #11
slamm_google
Tony and James, thanks for the reviews. I have addressed most of the issues in ...
7 years, 4 months ago (2012-04-13 23:43:36 UTC) #12
tonyg
Thanks for the updates. Looking forward to seeing the changes. A couple of responses inline, ...
7 years, 4 months ago (2012-04-16 15:34:32 UTC) #13
slamm_google
I have uploaded my latest (for the other two CLs too.) PTAL https://chromiumcodereview.appspot.com/9956045/diff/18001/chrome/test/perf/page_cycler_test.cc File chrome/test/perf/page_cycler_test.cc ...
7 years, 4 months ago (2012-04-16 23:43:07 UTC) #14
James Simonsen
https://chromiumcodereview.appspot.com/9956045/diff/18001/tools/page_cycler/webpagereplay/extension/background.js File tools/page_cycler/webpagereplay/extension/background.js (right): https://chromiumcodereview.appspot.com/9956045/diff/18001/tools/page_cycler/webpagereplay/extension/background.js#newcode12 tools/page_cycler/webpagereplay/extension/background.js:12: var kWaitUrl = "http://wprwprwpr/web-page-replay-generate-200"; On 2012/04/16 15:34:32, tonyg wrote: ...
7 years, 4 months ago (2012-04-17 03:30:18 UTC) #15
tonyg
Last comments from me. Everything else looks good. https://chromiumcodereview.appspot.com/9956045/diff/18001/tools/page_cycler/webpagereplay/extension/background.js File tools/page_cycler/webpagereplay/extension/background.js (right): https://chromiumcodereview.appspot.com/9956045/diff/18001/tools/page_cycler/webpagereplay/extension/background.js#newcode12 tools/page_cycler/webpagereplay/extension/background.js:12: var ...
7 years, 4 months ago (2012-04-17 11:04:45 UTC) #16
slamm_google
Thank you for all the reviews. I remembered another feature of the perftracker extension. It ...
7 years, 4 months ago (2012-04-17 20:47:16 UTC) #17
James Simonsen
On 2012/04/17 20:47:16, slamm wrote: > The optparse documentation suggests that "required options" do not ...
7 years, 4 months ago (2012-04-17 21:08:28 UTC) #18
James Simonsen
Oops, meant to include this in the last message... https://chromiumcodereview.appspot.com/9956045/diff/38001/tools/page_cycler/webpagereplay/extension/background.js File tools/page_cycler/webpagereplay/extension/background.js (right): https://chromiumcodereview.appspot.com/9956045/diff/38001/tools/page_cycler/webpagereplay/extension/background.js#newcode201 tools/page_cycler/webpagereplay/extension/background.js:201: ...
7 years, 4 months ago (2012-04-17 21:08:52 UTC) #19
slamm_google
Thanks for moving this onto the trybots. -slamm https://chromiumcodereview.appspot.com/9956045/diff/38001/tools/page_cycler/webpagereplay/extension/background.js File tools/page_cycler/webpagereplay/extension/background.js (right): https://chromiumcodereview.appspot.com/9956045/diff/38001/tools/page_cycler/webpagereplay/extension/background.js#newcode201 tools/page_cycler/webpagereplay/extension/background.js:201: result_.readBytesKb ...
7 years, 4 months ago (2012-04-17 21:24:02 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/slamm@google.com/9956045/43016
7 years, 4 months ago (2012-04-18 00:36:38 UTC) #21
commit-bot: I haz the power
7 years, 4 months ago (2012-04-18 02:40:47 UTC) #22
Change committed as 132730

Powered by Google App Engine
This is Rietveld 408576698