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

Issue 3017573002: Make sure snap_page combined iframe serialized dom

Created:
3 years, 3 months ago by nednguyen
Modified:
3 years, 2 months ago
Reviewers:
pdr., wkorman
CC:
catapult-reviews_chromium.org, telemetry-reviews_chromium.org, chrishtr
Target Ref:
refs/heads/master
Project:
catapult
Visibility:
Public.

Description

Make sure snap_page combined iframe serialized dom To try this out, run: ./telemetry/bin/snap_page --browser=system --url=https://www.cnn.com BUG=chromium:763119 COMMIT=false # will split this to smaller CLs when it works end to end

Patch Set 1 #

Patch Set 2 : . #

Total comments: 1

Patch Set 3 : . #

Total comments: 10

Patch Set 4 : Rebase & address review comments #

Total comments: 1

Patch Set 5 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+86 lines, -18 lines) Patch
M telemetry/telemetry/internal/snap_page_util.py View 2 chunks +75 lines, -18 lines 0 comments Download
telemetry/telemetry/internal/snap_page_util_unittest.py View 2 chunks +11 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
nednguyen
3 years, 3 months ago (2017-09-20 18:30:02 UTC) #4
nednguyen
This CL is ready for review, PTAL
3 years, 3 months ago (2017-09-20 19:33:18 UTC) #5
wkorman
Exciting! https://codereview.chromium.org/3017573002/diff/40001/telemetry/telemetry/internal/browser/web_contents.py File telemetry/telemetry/internal/browser/web_contents.py (right): https://codereview.chromium.org/3017573002/diff/40001/telemetry/telemetry/internal/browser/web_contents.py#newcode248 telemetry/telemetry/internal/browser/web_contents.py:248: ab.EvaluateJavaScript(..., context_id=context_id). Missing 't' https://codereview.chromium.org/3017573002/diff/60001/telemetry/telemetry/internal/backends/chrome_inspector/inspector_runtime.py File telemetry/telemetry/internal/backends/chrome_inspector/inspector_runtime.py (right): ...
3 years, 3 months ago (2017-09-20 20:11:05 UTC) #6
nednguyen
On 2017/09/20 20:11:05, wkorman wrote: > Exciting! > > https://codereview.chromium.org/3017573002/diff/40001/telemetry/telemetry/internal/browser/web_contents.py > File telemetry/telemetry/internal/browser/web_contents.py (right): > ...
3 years, 3 months ago (2017-09-21 15:09:49 UTC) #7
nednguyen
On 2017/09/21 15:09:49, nednguyen wrote: > On 2017/09/20 20:11:05, wkorman wrote: > > Exciting! > ...
3 years, 2 months ago (2017-09-25 13:14:30 UTC) #8
nednguyen
https://codereview.chromium.org/3017573002/diff/60001/telemetry/telemetry/internal/backends/chrome_inspector/inspector_runtime.py File telemetry/telemetry/internal/backends/chrome_inspector/inspector_runtime.py (right): https://codereview.chromium.org/3017573002/diff/60001/telemetry/telemetry/internal/backends/chrome_inspector/inspector_runtime.py#newcode72 telemetry/telemetry/internal/backends/chrome_inspector/inspector_runtime.py:72: return self._all_context_ids On 2017/09/20 20:11:04, wkorman wrote: > Is ...
3 years, 2 months ago (2017-09-26 13:01:35 UTC) #9
wkorman
3 years, 2 months ago (2017-09-27 17:15:14 UTC) #10
lgtm

https://codereview.chromium.org/3017573002/diff/80001/telemetry/telemetry/int...
File telemetry/telemetry/internal/snap_page_util.py (right):

https://codereview.chromium.org/3017573002/diff/80001/telemetry/telemetry/int...
telemetry/telemetry/internal/snap_page_util.py:79: while k <
len(sub_dom_string):
Worth adding a unit test to validate the chunking logic?

Powered by Google App Engine
This is Rietveld 408576698