New issue
Advanced search Search tips

Issue 919833 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 18
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Flaky-Test: external/wpt/trusted-types/block-string-assignment-to-Window-open.tentative.html



Sign in to add a comment

external/wpt/trusted-types/block-string-assignment-to-Window-open.tentative.html is flaky

Project Member Reported by Findit, Jan 8

Issue description

Owner: vogelheim@chromium.org
Daniel, I don't know how to handle this (fix, revert, disable or file a bug). Can you please help?
I'm gonna revert it.
Cc: vogelheim@chromium.org
Owner: jakubvrana@google.com
Status: Assigned (was: Untriaged)
Or not. Jakub WDYT?
Cc: rsorokin@chromium.org
Revert CL is here: https://chromium-review.googlesource.com/c/chromium/src/+/1405188
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 10

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

commit 822e75f724b501832a7abb1353cf5c7e400d4aa0
Author: Roman Sorokin <rsorokin@chromium.org>
Date: Thu Jan 10 17:12:35 2019

[Sheriff] Revert "Trusted Types: Store TrustedURL and TrustedScriptURL contents as string"

This reverts commit a54a0773e501aeb3b7149271149223632c478a71.

Reason for revert: flakiness https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vYTU0YTA3NzNlNTAxYWViM2I3MTQ5MjcxMTQ5MjIzNjMyYzQ3OGE3MQw

BUG= chromium:919833 

Original change's description:
> Trusted Types: Store TrustedURL and TrustedScriptURL contents as string
> 
> Bug: 739170,  913180 
> Change-Id: I01391891d89aeb55e387059ed4c4a4b92c6dcd7b
> Reviewed-on: https://chromium-review.googlesource.com/c/1375714
> Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
> Reviewed-by: Mike West <mkwst@chromium.org>
> Commit-Queue: Daniel Vogelheim <vogelheim@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620299}

TBR=vogelheim@chromium.org,mkwst@chromium.org,jakubvrana@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 739170,  913180 
Change-Id: Ic3561942a74f6106d629c5f3a7b30014719bc7d5
Reviewed-on: https://chromium-review.googlesource.com/c/1405188
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>
Commit-Queue: Roman Sorokin <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621617}
[modify] https://crrev.com/822e75f724b501832a7abb1353cf5c7e400d4aa0/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/822e75f724b501832a7abb1353cf5c7e400d4aa0/third_party/blink/renderer/core/trustedtypes/trusted_script_url.cc
[modify] https://crrev.com/822e75f724b501832a7abb1353cf5c7e400d4aa0/third_party/blink/renderer/core/trustedtypes/trusted_script_url.h
[modify] https://crrev.com/822e75f724b501832a7abb1353cf5c7e400d4aa0/third_party/blink/renderer/core/trustedtypes/trusted_type_policy.cc
[modify] https://crrev.com/822e75f724b501832a7abb1353cf5c7e400d4aa0/third_party/blink/renderer/core/trustedtypes/trusted_types_util_test.cc
[modify] https://crrev.com/822e75f724b501832a7abb1353cf5c7e400d4aa0/third_party/blink/renderer/core/trustedtypes/trusted_url.cc
[modify] https://crrev.com/822e75f724b501832a7abb1353cf5c7e400d4aa0/third_party/blink/renderer/core/trustedtypes/trusted_url.h
[modify] https://crrev.com/822e75f724b501832a7abb1353cf5c7e400d4aa0/third_party/blink/web_tests/external/wpt/trusted-types/TrustedTypePolicy-createXXX.tentative.html
[modify] https://crrev.com/822e75f724b501832a7abb1353cf5c7e400d4aa0/third_party/blink/web_tests/external/wpt/trusted-types/TrustedTypePolicyFactory-createPolicy-createXYZTests.tentative.html
[modify] https://crrev.com/822e75f724b501832a7abb1353cf5c7e400d4aa0/third_party/blink/web_tests/external/wpt/trusted-types/block-string-assignment-to-Element-setAttribute.tentative.html
[modify] https://crrev.com/822e75f724b501832a7abb1353cf5c7e400d4aa0/third_party/blink/web_tests/external/wpt/trusted-types/block-string-assignment-to-HTMLElement-generic.tentative.html

Thanks to Roman, I was able to reproduce this locally with:

rsorokin@rsorokin:~/chromium/src$ tools/mb/mb.py gen out/Release -m chromium.memory -b 'WebKit Linux Trusty Leak'

rsorokin@rsorokin:~/chromium/src$ autoninja -C out/Release/ webkit_layout_tests && third_party/blink/tools/run_web_tests.py --seed 4 --no-show-results --zero-tests-executed-ok --clobber-old-results --exit-after-n-failures 5000 --exit-after-n-crashes-or-timeouts 100 --debug-rwt-logging   --additional-expectations ./third_party/blink/web_tests/LeakExpectations --time-out-ms 48000 --enable-leak-detection --gtest_filter=external/wpt/trusted-types/block-string-assignment-to-Window-open.tentative.html --gtest_repeat=40

The issue is with testWindowDoesntThrow(t, null, "null", document), I don't know why. I will comment-out the test with a TODO in the roll-forward.
Labels: -Sheriff-Chromium
I think I found it: The issue was in the test code, which apparently didn't always run the cleanup before the test had finished. Those non-cleaned-up objects then appeared to have leaked.

I'll upload a copy of the original CL + the fix.
Even better idea: I uploaded the diff on Jakub's 2nd patch, so the credit stays with the original author. :)
Thanks!
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 18 (4 days ago)

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

commit 08ab22533f042bf27bf609599a1ef16d5aeb1b76
Author: Daniel Vogelheim <vogelheim@chromium.org>
Date: Fri Jan 18 14:24:06 2019

Store TrustedURL and TrustedScriptURL contents as string, take 2

Reland of crrev.com/c/1375714, with fix for the leaky test in  crbug.com/919833 .

BUG=739170,  919833 

Change-Id: Id20122f815e676462446675e4cf853acb1e1e4ae
Reviewed-on: https://chromium-review.googlesource.com/c/1406709
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624108}
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/renderer/core/trustedtypes/trusted_script_url.cc
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/renderer/core/trustedtypes/trusted_script_url.h
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/renderer/core/trustedtypes/trusted_type_policy.cc
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/renderer/core/trustedtypes/trusted_types_util_test.cc
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/renderer/core/trustedtypes/trusted_url.cc
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/renderer/core/trustedtypes/trusted_url.h
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/web_tests/external/wpt/trusted-types/TrustedTypePolicy-createXXX.tentative.html
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/web_tests/external/wpt/trusted-types/TrustedTypePolicyFactory-createPolicy-createXYZTests.tentative.html
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/web_tests/external/wpt/trusted-types/block-string-assignment-to-Element-setAttribute.tentative.html
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/web_tests/external/wpt/trusted-types/block-string-assignment-to-HTMLElement-generic.tentative.html
[modify] https://crrev.com/08ab22533f042bf27bf609599a1ef16d5aeb1b76/third_party/blink/web_tests/external/wpt/trusted-types/block-string-assignment-to-Window-open.tentative.html

Comment 12 by jakubvrana@google.com, Jan 18 (4 days ago)

Status: Fixed (was: Assigned)

Sign in to add a comment