Issue 1907413003: Wait until the test page is fully loaded before dumping the contents.

Issue 1907413003: Wait until the test page is fully loaded before dumping the contents. (Closed)

Created:
4 years, 8 months ago by Łukasz Anforowicz

Modified:
4 years, 8 months ago

Reviewers:
dcheng

CC:
blink-reviews, chromium-reviews, site-isolation-reviews_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

Wait until the test page is fully loaded before dumping the contents. Context - how timing of text/pixel dumps works. =============================================== Text dump or pixel dump are taken 1) immediately when testRunner.notifyDone is called or 2) when test page finishes loading (i.e. if TestRunner::topLoadingFrame was not-null when testRunner.notifyDone was called and we didn't take path (1) above - see how TestRunner::CompleteNotifyDone calls TestFinished only if !topLoadingFrame()). Context - OOPIFs support requires changes to topLoadingFrame tracking ===================================================================== Some planned refactorings change how TestRunner::topLoadingFrame works (to make sure there is only 1 top loading frame across all renderer processes). This can affect the timing of text/pixel dumps. Changes in this CL ================== So - to avoid timing problems described above, this CL makes sure the tests are explicitly timing their text/pixel dumps. BUG=595089 Committed: https://crrev.com/5cf0f0ca0e0478113e2df9814266e6291ccea925 Cr-Commit-Position: refs/heads/master@{#389670}

Patch Set 1 #

Total comments: 2

Patch Set 2 : testRunner.notifyDone from img's load event (not after testRunner.layoutAndPaintAsyncThen). #

Patch Set 3 : Tweaking a comment. #

Created: 4 years, 8 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+14 lines, -5 lines)			Patch
	M	third_party/WebKit/LayoutTests/http/tests/navigation/resources/notify-done.html	View		1 chunk	+6 lines, -3 lines	0 comments	Download
	M	third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html	View	1 2	1 chunk	+8 lines, -2 lines	0 comments	Download

Messages

Total messages: 10 (3 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

Łukasz Anforowicz

Daniel, could you please take a look to keep me honest here? 1. The tests ...

4 years, 8 months ago (2016-04-23 00:11:07 UTC) #2

dcheng

https://codereview.chromium.org/1907413003/diff/1/third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html File third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html (right): https://codereview.chromium.org/1907413003/diff/1/third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html#newcode22 third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html:22: testRunner.layoutAndPaintAsyncThen(function() { Should this just use the image's load ...

4 years, 8 months ago (2016-04-25 23:15:04 UTC) #3

Łukasz Anforowicz

Daniel, can you take another look? https://codereview.chromium.org/1907413003/diff/1/third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html File third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html (right): https://codereview.chromium.org/1907413003/diff/1/third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html#newcode22 third_party/WebKit/LayoutTests/http/tests/security/referrer-policy-attribute-img-no-referrer-when-downgrade.html:22: testRunner.layoutAndPaintAsyncThen(function() { On ...

4 years, 8 months ago (2016-04-25 23:33:50 UTC) #4

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1907413003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1907413003/40001

4 years, 8 months ago (2016-04-25 23:38:45 UTC) #7

commit-bot: I haz the power

Description was changed from ========== Wait until the test page is fully loaded before dumping ...

4 years, 8 months ago (2016-04-26 02:29:30 UTC) #9

Message was sent while issue was closed.

Description was changed from

==========
Wait until the test page is fully loaded before dumping the contents.

Context - how timing of text/pixel dumps works.
===============================================

Text dump or pixel dump are taken
1) immediately when testRunner.notifyDone is called
or
2) when test page finishes loading (i.e. if TestRunner::topLoadingFrame
   was not-null when testRunner.notifyDone was called and we didn't take
   path (1) above - see how TestRunner::CompleteNotifyDone calls TestFinished
   only if !topLoadingFrame()).


Context - OOPIFs support requires changes to topLoadingFrame tracking
=====================================================================

Some planned refactorings change how TestRunner::topLoadingFrame works
(to make sure there is only 1 top loading frame across all renderer
processes).  This can affect the timing of text/pixel dumps.


Changes in this CL
==================

So - to avoid timing problems described above, this CL makes sure
the tests are explicitly timing their text/pixel dumps.


BUG=595089
==========

to

==========
Wait until the test page is fully loaded before dumping the contents.

Context - how timing of text/pixel dumps works.
===============================================

Text dump or pixel dump are taken
1) immediately when testRunner.notifyDone is called
or
2) when test page finishes loading (i.e. if TestRunner::topLoadingFrame
   was not-null when testRunner.notifyDone was called and we didn't take
   path (1) above - see how TestRunner::CompleteNotifyDone calls TestFinished
   only if !topLoadingFrame()).

Context - OOPIFs support requires changes to topLoadingFrame tracking
=====================================================================

Some planned refactorings change how TestRunner::topLoadingFrame works
(to make sure there is only 1 top loading frame across all renderer
processes).  This can affect the timing of text/pixel dumps.

Changes in this CL
==================

So - to avoid timing problems described above, this CL makes sure
the tests are explicitly timing their text/pixel dumps.

BUG=595089

Committed: https://crrev.com/5cf0f0ca0e0478113e2df9814266e6291ccea925
Cr-Commit-Position: refs/heads/master@{#389670}
==========

commit-bot: I haz the power

4 years, 8 months ago (2016-04-26 02:29:31 UTC) #10

Message was sent while issue was closed.

Patchset 3 (id:??) landed as
https://crrev.com/5cf0f0ca0e0478113e2df9814266e6291ccea925
Cr-Commit-Position: refs/heads/master@{#389670}

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages