New issue
Advanced search Search tips

Issue 602421 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug-Regression

Blocking:
issue 477150



Sign in to add a comment

Flaky mixed content layout tests - console messages sometimes include or not include line numbers

Project Member Reported by lukasza@chromium.org, Apr 11 2016

Issue description

REPRO:

DISPLAY=:20 third_party/WebKit/Tools/Scripts/run-webkit-tests -t gn -v --additional-drt-flag=--site-per-process --no-retry-failures --additional-drt-flag=--no-sandbox --iterations=100 http/tests/security/mixedContent/insecure-srcset-in-main-frame-blocked.html

Notice that the test fails in around 40% of cases.

This fails locally and also on the Site Isolation bot: https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Linux/builds/8748


SUCCESS CASE (~60% of local runs):

Actual test output:
CONSOLE WARNING: line 8: Mixed Content: The page at ... was loaded over HTTPS, but requested an insecure ...

Callstack and printf-style debugging of Document::addConsoleMessage:
addConsoleMessage: isParsingAtLineNumber == 1
addConsoleMessage: stack: #0 0x7f0a37a5253e base::debug::StackTrace::StackTrace()
#1 0x7f0a31d2a874 blink::Document::addConsoleMessage()
#2 0x7f0a3239cff4 blink::MixedContentChecker::logToConsoleAboutFetch()
#3 0x7f0a3239d4ad blink::MixedContentChecker::shouldBlockFetch()
#4 0x7f0a3238a31a blink::FrameFetchContext::canRequestInternal()
#5 0x7f0a323897fd blink::FrameFetchContext::canRequest()
#6 0x7f0a32235b2c blink::ResourceFetcher::requestResource()
#7 0x7f0a32219dc5 blink::ImageResource::fetch()
#8 0x7f0a32394904 blink::ImageLoader::doUpdateFromElement()
#9 0x7f0a323969da blink::ImageLoader::Task::run()
#10 0x7f0a31a7dc22 blink::microtaskFunctionCallback()
#11 0x7f0a3442272e v8::internal::Isolate::RunMicrotasksInternal()
#12 0x7f0a3442234b v8::internal::Isolate::RunMicrotasks()
#13 0x7f0a31f5a80f blink::HTMLScriptRunner::runScript()
#14 0x7f0a31f5a66f blink::HTMLScriptRunner::execute()
#15 0x7f0a31f41aa6 blink::HTMLDocumentParser::runScriptsForPausedTreeBuilder()
#16 0x7f0a31f43d90 blink::HTMLDocumentParser::processParsedChunkFromBackgroundParser()
...


FAILURE CASE (~40% of local runs):

Actual test output (notice the missing "line 8: "):
CONSOLE WARNING: Mixed Content: The page at ... was loaded over HTTPS, but requested an insecure ...

Callstack and printf-style debugging of Document::addConsoleMessage:
addConsoleMessage: isParsingAtLineNumber == 0
addConsoleMessage: stack: #0 0x7f901268953e base::debug::StackTrace::StackTrace()
#1 0x7f900c961874 blink::Document::addConsoleMessage()
#2 0x7f900cfd3ff4 blink::MixedContentChecker::logToConsoleAboutFetch()
#3 0x7f900cfd44ad blink::MixedContentChecker::shouldBlockFetch()
#4 0x7f900cfc131a blink::FrameFetchContext::canRequestInternal()
#5 0x7f900cfc07fd blink::FrameFetchContext::canRequest()
#6 0x7f900ce6cb2c blink::ResourceFetcher::requestResource()
#7 0x7f900ce50dc5 blink::ImageResource::fetch()
#8 0x7f900cfcb904 blink::ImageLoader::doUpdateFromElement()
#9 0x7f900cfcd9da blink::ImageLoader::Task::run()
#10 0x7f900c6b4c22 blink::microtaskFunctionCallback()
#11 0x7f900f05972e v8::internal::Isolate::RunMicrotasksInternal()
#12 0x7f900f05934b v8::internal::Isolate::RunMicrotasks()
#13 0x7f900e3ed81e blink::(anonymous namespace)::EndOfTaskRunner::didProcessTask()
#14 0x7f900b74c5b7 scheduler::TaskQueueManager::ProcessTaskFromWorkQueue()
...
 
No repro in 1000 iterations without --site-per-process
Labels: -Type-Bug Type-Bug-Regression
This seems to be a regression - I cannot repro the flakiness at

Apr 09 16:21	bafe54b9a28cd03b55d13e089b0899c2dfe67471	build #8730

but can repro at

Apr 11 10:29	b7f41daaf554f9329ec9bab4fa6e1f87f5157ee3	build #8749

Owner: csharrison@chromium.org
Status: Assigned (was: Untriaged)
The flakiness starts in the following CL: https://codereview.chromium.org/1846143003
Charles, could you please take a look?  If this is something that cannot be reverted or fixed easily, then could you please add a temporary [ Failure ] test expectation to third_party/WebKit/LayoutTests/FlagExpectations/site-per-process?
That's really strange. Did you run the bisect manually find the source of the flakiness?

Comment 6 by dcheng@chromium.org, Apr 12 2016

See  issue 381451 , which is a previous instance of line numbers causing flakiness.
Yeah I'm taking a look. Not sure how my change could have changed this but I'll quickly investigate.
RE: #c5

Yes - I have run the bisect manually (I have had trouble hooking up layout test results into automatic bisect run) + I've confirmed by manually trying the repro (100 iterations) right before and after https://codereview.chromium.org/1846143003
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4dae3aaa783d7a657aaaf9356d9e09e82d329fbb

commit 4dae3aaa783d7a657aaaf9356d9e09e82d329fbb
Author: csharrison <csharrison@chromium.org>
Date: Tue Apr 12 14:16:00 2016

Add TestExpectations for flaky test under --site-per-process

BUG= 602421 

Review URL: https://codereview.chromium.org/1880893002

Cr-Commit-Position: refs/heads/master@{#386676}

[modify] https://crrev.com/4dae3aaa783d7a657aaaf9356d9e09e82d329fbb/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

There is one more occurrence of the flaky line numbers in:

https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Linux/builds/8756:

http/tests/security/mixedContent/insecure-empty-srcset-in-main-frame-blocked.html


OTOH, so far the bot has seen only one failure in this test + I cannot repro the flakiness locally (200 iterations), so I am not sure if it is worth it to add another test exception.  If needed, I drafted the exception at https://codereview.chromium.org/1877393002


Cc: csharrison@chromium.org dcheng@chromium.org
Owner: lukasza@chromium.org
Status: Started (was: Assigned)
Daniel has pointed out to me how to address the flakiness (by replacing static, inline-img element with one dynamically added from document's onload event - this way the image is deterministically loaded with no associated line number).  I've tried this locally and it does get rid of the flakiness - so, let me take the bug and upload a CL with this fix.
Project Member

Comment 12 by bugdroid1@chromium.org, Apr 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3c3cc057a1ac5c2a9ab5a1f4932f494569db46c3

commit 3c3cc057a1ac5c2a9ab5a1f4932f494569db46c3
Author: lukasza <lukasza@chromium.org>
Date: Wed Apr 13 18:30:15 2016

Remove the "line-number flakiness" by adding an img element dynamically.

This CL makes sure that when image resource is loaded it never has an
associated line number (previously, it might be associated with a line
number depending at what stage the image is loaded - i.e.  depending on
whether the HTML document parser is still on the callstack).

BUG= 602421 

Review URL: https://codereview.chromium.org/1882893002

Cr-Commit-Position: refs/heads/master@{#387040}

[modify] https://crrev.com/3c3cc057a1ac5c2a9ab5a1f4932f494569db46c3/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process
[modify] https://crrev.com/3c3cc057a1ac5c2a9ab5a1f4932f494569db46c3/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/insecure-empty-srcset-in-main-frame-blocked-expected.txt
[modify] https://crrev.com/3c3cc057a1ac5c2a9ab5a1f4932f494569db46c3/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/insecure-srcset-in-main-frame-blocked-expected.txt
[modify] https://crrev.com/3c3cc057a1ac5c2a9ab5a1f4932f494569db46c3/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/resources/frame-with-insecure-empty-srcset.html
[modify] https://crrev.com/3c3cc057a1ac5c2a9ab5a1f4932f494569db46c3/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/resources/frame-with-insecure-srcset.html

Labels: Merge-Request-51
lukasza@, mind if we merge this to M51?

That is, the real fix: https://codereview.chromium.org/1882893002
Sure - let's go ahead with the merge (probably LayoutTests/FlagExpectations/site-per-process won't merge cleanly / will have to be excluded from the merge).
Status: Fixed (was: Started)
Marking as fixed - the flakiness didn't repro on the Site Isolation FYI bot since build #8771.

Comment 16 by tin...@google.com, Apr 14 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
I'll go ahead and merge this (- test expectation file) now.
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 15 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e2269d37d95336b8037b65ab261a34cefa00083c

commit e2269d37d95336b8037b65ab261a34cefa00083c
Author: csharrison <csharrison@chromium.org>
Date: Fri Apr 15 13:40:10 2016

Remove the "line-number flakiness" by adding an img element dynamically.

[Merge to M51]

This CL makes sure that when image resource is loaded it never has an
associated line number (previously, it might be associated with a line
number depending at what stage the image is loaded - i.e.  depending on
whether the HTML document parser is still on the callstack).

BUG= 602421 
TBR=lukasza@chromium.org
NOTRY=true
NOPRESUBMIT=true

Review URL: https://codereview.chromium.org/1882893002

Cr-Commit-Position: refs/heads/master@{#387040}
(cherry picked from commit 3c3cc057a1ac5c2a9ab5a1f4932f494569db46c3)

Review URL: https://codereview.chromium.org/1890953003

Cr-Commit-Position: refs/branch-heads/2704@{#72}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/e2269d37d95336b8037b65ab261a34cefa00083c/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/insecure-empty-srcset-in-main-frame-blocked-expected.txt
[modify] https://crrev.com/e2269d37d95336b8037b65ab261a34cefa00083c/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/insecure-srcset-in-main-frame-blocked-expected.txt
[modify] https://crrev.com/e2269d37d95336b8037b65ab261a34cefa00083c/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/resources/frame-with-insecure-empty-srcset.html
[modify] https://crrev.com/e2269d37d95336b8037b65ab261a34cefa00083c/third_party/WebKit/LayoutTests/http/tests/security/mixedContent/resources/frame-with-insecure-srcset.html

Labels: Test-Layout

Sign in to add a comment