New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 873015 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 2
Type: ----



Sign in to add a comment

webkit_unit_tests (FontFaceCacheTest.FLAKY_MatchCombinations) failing

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Aug 10

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of mgiuca@chromium.org

webkit_unit_tests failing on chromium.linux/Linux Tests (dbg)(1)(32)

Builders failed on: 
- Linux Tests (dbg)(1)(32): 
  https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests%20%28dbg%29%281%29%2832%29


 
Components: Blink>Fonts
Labels: OS-Linux
> Findit (?) found culprit r581828 with 85% confidence.

I don't see any reason for this CL to be the culprit. Change seems to be a no-op, and unrelated to the failing test.

Failing test:

[ RUN      ] FontFaceCacheTest.FLAKY_MatchCombinations
Received signal 11 SEGV_MAPERR 00002a2a2a2e
#0 0x0000f7536ba9 base::debug::StackTrace::StackTrace()
#1 0x0000f724519e base::debug::StackTrace::StackTrace()
#2 0x0000f75365ad base::debug::(anonymous namespace)::StackDumpSignalHandler()
#3 0x0000f76f8cc0 ([vdso]+0xcbf)
#4 0x0000e41aae06 blink::ThreadState::Heap()
#5 0x0000e41aad53 blink::ThreadState::IsOnThreadHeap()
#6 0x0000e41aa7f8 blink::MemberBase<>::CheckPointer()
#7 0x0000e41cdbf6 blink::MemberBase<>::operator=<>()
#8 0x0000e41e183d blink::Member<>::operator=<>()
#9 0x0000e46b5726 WTF::HashMapTranslator<>::Translate<>()
#10 0x0000e46b534e WTF::HashTable<>::insert<>()
#11 0x0000e46b4e3c WTF::HashMap<>::InlineAdd<>()
#12 0x0000e46b33ac WTF::HashMap<>::insert<>()
#13 0x0000e46b1e85 blink::FontFaceCache::Add()
#14 0x00000988b864 blink::FontFaceCacheTest::AppendTestFaceForCapabilities()
#15 0x00000988de79 blink::FontFaceCacheTest_FLAKY_MatchCombinations_Test::TestBody()
#16 0x00000863503b testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#17 0x000008626972 testing::internal::HandleExceptionsInMethodIfSupported<>()
#18 0x000008609a88 testing::Test::Run()
#19 0x00000860a5a2 testing::TestInfo::Run()
#20 0x00000860b070 testing::TestCase::Run()
#21 0x00000861deea testing::internal::UnitTestImpl::RunAllTests()
#22 0x0000086350cb testing::internal::HandleSehExceptionsInMethodIfSupported<>()
#23 0x0000086281f2 testing::internal::HandleExceptionsInMethodIfSupported<>()
#24 0x00000861da6e testing::UnitTest::Run()
#25 0x00000b752678 RUN_ALL_TESTS()
#26 0x00000b74ef46 base::TestSuite::Run()
#27 0x0000083e0fad (anonymous namespace)::runHelper()
#28 0x0000083e2ace _ZN4base8internal13FunctorTraitsIPFiPNS_9TestSuiteEEvE6InvokeIRKS5_JS3_EEEiOT_DpOT0_
#29 0x0000083e2a41 _ZN4base8internal12InvokeHelperILb0EiE8MakeItSoIRKPFiPNS_9TestSuiteEEJS5_EEEiOT_DpOT0_
#30 0x0000083e29f7 _ZN4base8internal7InvokerINS0_9BindStateIPFiPNS_9TestSuiteEEJNS0_17UnretainedWrapperIS3_EEEEEFivEE7RunImplIRKS6_RKNSt3__15tupleIJS8_EEEJLj0EEEEiOT_OT0_NSF_16integer_sequenceIjJXspT1_EEEE
#31 0x0000083e28ba _ZN4base8internal7InvokerINS0_9BindStateIPFiPNS_9TestSuiteEEJNS0_17UnretainedWrapperIS3_EEEEEFivEE3RunEPNS0_13BindStateBaseE
#32 0x00000b7602b7 _ZNO4base12OnceCallbackIFivEE3RunEv
#33 0x00000b758ecf base::(anonymous namespace)::LaunchUnitTestsInternal()
#34 0x00000b758ca6 base::LaunchUnitTests()
#35 0x0000083e0f35 main
#36 0x0000dc8ffaf3 __libc_start_main
#37 0x0000083ddb72 <unknown>
  gs: 00000063  fs: 00000000  es: 0000002b  ds: 0000002b
 edi: 245e1c70 esi: 2a2a2a2a ebp: ffe696a8 esp: ffe6966c
 ebx: e7829044 edx: ffe69201 ecx: 2a2a2a2e eax: 2a2a2a2a
 trp: 0000000e err: 00000004  ip: e41aae06  cs: 00000023
 efl: 00210206 usp: ffe6966c  ss: 0000002b
[end of stack trace]
Summary: webkit_unit_tests (FontFaceCacheTest.FLAKY_MatchCombinations) failing on chromium.linux/Linux Tests (dbg)(1)(32) (was: webkit_unit_tests failing on chromium.linux/Linux Tests (dbg)(1)(32))
Was already marked flaky 2 days ago in Issue 871812, but now failing 100% of the time.
Blamelist: r581826 .. r581842.

Nothing relevant to fonts. Only two CLs modified blink: r581833 and r581836.
I looked in detail at both of those CLs (r581833 and r581836); they both seem innocuous (just minor cleanup in both cases). They both relate to CSS, which is used in the test, but nothing that would seem to impact this.

Maybe FindIt is correct after all, and the very small change made in r581828 is having an impact here. How can I know when FindIt doesn't tell me anything about what process it used to come by this decision?

- It blamed r581828 with "85% confidence". But why?
- No info is provided about what methodology was actually used to find this. (Bisection? Guess based on files modified? Words mentioned in the CL description?)
- "Swarming rerun" links to a page of diagnostics about the run itself, nothing explaining any bisecting or anything else that would help a sheriff.
- "Reason" "Try-job run" links to https://ci.chromium.org/p/chromium/builders/luci.chromium.findit/findit_variable/13293 which is a 404 error.

This entire page is completely useless to a sheriff (unless the found CL is correct).

I have no idea whether to revert r581828, r581833, r581836, or all of the above.
Agh, I forgot to link the CL to the bug. I reverted r581828 to start with:

Revert "Remove unnecessary NetworkService flag check in ResourceDispatcher"

This reverts commit 3d493dc6c1e44b5423d6b55d2bbac0413f68ccae.

Reason for revert: FindIt blamed this CL with 85% confidence.

Sorry. I don't see anything wrong with this CL, but I also don't see
anything wrong with any other CL in the blamelist, so I'm starting
here. See  https://crbug.com/873015  for analysis.

Original change's description:
> Remove unnecessary NetworkService flag check in ResourceDispatcher
> 
> blink::ServiceWorkerUtils::IsServicificationEnabled() checks
> whether NetworkService is enabled, so we don't need the check
> in ResourceDispatcher::StartAsync().
> 
> Bug: N/A
> Change-Id: I06bab175489f42dc0cefa2a257806db29192eba1
> Reviewed-on: https://chromium-review.googlesource.com/1168707
> Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581828}

TBR=yhirano@chromium.org,bashi@chromium.org,shimazu@chromium.org

Change-Id: Ia89cf9c0523395c7b331482f39e708f283cbc3ec
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: N/A
Reviewed-on: https://chromium-review.googlesource.com/1170402
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582023}

Will watch the bot to see if this has any effect. I'm not confident.
Cc: -mgiuca@chromium.org
Owner: mgiuca@chromium.org
Status: Started (was: Available)
As expected, this revert didn't help.

Next on the chopping block: r581836
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 10

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

commit 2472878c1953dba5360a183647d077cd1634db81
Author: Matt Giuca <mgiuca@chromium.org>
Date: Fri Aug 10 04:26:31 2018

Revert "Delete constructor for creating empty CSSStyleSheets"

This reverts commit af00b29ef10cda1bec17f975930ac23de6375da9.

Reason for revert: Suspect for  https://crbug.com/873015 

I have low confidence that this is the culprit but it's my best guess.

Original change's description:
> Delete constructor for creating empty CSSStyleSheets
> 
> Currently, empty CSSStyleSheets can be constructed either with a constructor or with Document.createEmptyCSSStyleSheet.
> This CL deletes the constructor so that they can only be produced by Document.createEmptyCSSStyleSheet.
> 
> Document.createEmptyCSSStyleSheet is considered to be more desirable, as CSSStyleSheets produced by Document.createEmptyCSSStyleSheet can be tied to documents in the future. This means that their use can be limited in the documents where they were produced, resulting in higher security.
> 
> Note:
> The constructed CSSStyleSheet is not currently tied to the Document yet
> 
> Link to related comments in discussion:
> https://github.com/WICG/construct-stylesheets/issues/23#issuecomment-379180786
> https://github.com/WICG/construct-stylesheets/issues/15#issuecomment-391216056
> 
> Bug: 807560
> Change-Id: I767e15e83e1f31eb278bc81233c8b579d0f511c7
> Reviewed-on: https://chromium-review.googlesource.com/1164876
> Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
> Reviewed-by: Hayato Ito <hayato@chromium.org>
> Commit-Queue: Momoko Sumida <momon@google.com>
> Cr-Commit-Position: refs/heads/master@{#581836}

TBR=hayato@chromium.org,rakina@chromium.org,momon@google.com

Change-Id: Iea70ff4dcc3a5ecf3a806b417572fd8f88e2e58b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 807560,  873015 
Reviewed-on: https://chromium-review.googlesource.com/1170462
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582054}
[modify] https://crrev.com/2472878c1953dba5360a183647d077cd1634db81/third_party/WebKit/LayoutTests/external/wpt/css/cssom/interfaces-expected.txt
[modify] https://crrev.com/2472878c1953dba5360a183647d077cd1634db81/third_party/WebKit/LayoutTests/fast/css/CSSStyleSheet-constructable.html
[modify] https://crrev.com/2472878c1953dba5360a183647d077cd1634db81/third_party/WebKit/LayoutTests/fast/dom/dom-constructors-expected.txt
[modify] https://crrev.com/2472878c1953dba5360a183647d077cd1634db81/third_party/blink/renderer/core/css/css_style_sheet.cc
[modify] https://crrev.com/2472878c1953dba5360a183647d077cd1634db81/third_party/blink/renderer/core/css/css_style_sheet.h
[modify] https://crrev.com/2472878c1953dba5360a183647d077cd1634db81/third_party/blink/renderer/core/css/css_style_sheet.idl
[modify] https://crrev.com/2472878c1953dba5360a183647d077cd1634db81/third_party/blink/renderer/core/css/css_style_sheet_test.cc
[modify] https://crrev.com/2472878c1953dba5360a183647d077cd1634db81/third_party/blink/renderer/core/html/custom/custom_element_registry_test.cc

When can we revert back crrev.com/c/1170462?  I think it has nothing to do with FontFace caches.
Cc: hayato@chromium.org rakina@chromium.org mgiuca@chromium.org
Components: Blink>CSS
Labels: -Sheriff-Chromium OS-Windows
Owner: momon@google.com
Status: Assigned (was: Started)
Summary: webkit_unit_tests (FontFaceCacheTest.FLAKY_MatchCombinations) failing (was: webkit_unit_tests (FontFaceCacheTest.FLAKY_MatchCombinations) failing on chromium.linux/Linux Tests (dbg)(1)(32))
Oh this is also failing on Windows, I just noticed (changing title).

> When can we revert back crrev.com/c/1170462?  I think it has nothing to do with FontFace caches.

It looks like this was the culprit (since the test started passing after I reverted). I agree, it doesn't seem related, but note that the FontFaceCacheTest does internally use CSS methods, so there is somewhere to start investigating. In general, we would wait until a test run completes after reverting a CL, to see whether it continues to fail, or starts passing. In this case, it started passing.

The test has passed on this run, after I reverted r581836. So I think this was it:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests%20%28dbg%29%281%29%2832%29/51972

Removing Sheriff-Chromium and assigning to Momoko for investigation.

NOTE: I was unable to crash FontFaceCacheTest locally when investigating this. It seems to only crash on Linux and Windows tests on the tree. Still, you should try and figure out why this change caused it to crash, and only re-land when you are reasonably confident that this isn't going to break again.
Hmm I see, it's weird. Thanks for explaining, we'll try to figure it out before relanding.
Note: The corresponding Windows build (passing after my revert):
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20Tests%20%28dbg%29%281%29/70793
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 15

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

commit 0ecac34302d2d95059c3ea716f118c0b1cfcedd2
Author: Momoko Sumida <momon@google.com>
Date: Wed Aug 15 07:49:22 2018

Revert "Revert "Delete constructor for creating empty CSSStyleSheets""

This reverts commit 2472878c1953dba5360a183647d077cd1634db81.

The following CL was reverted as a suspect for  https://crbug.com/873015 :
crrev.com/c/1164876

This CL reverts the revert as the failing test is not related to CSSStyleSheet.
The flaky test has been disabled in crrev.com/c/1171681

Bug:  873015 
Change-Id: I54cefa72e4f82b1075550d6efb4a0ca8efe98383
Reviewed-on: https://chromium-review.googlesource.com/1174150
Commit-Queue: Momoko Sumida <momon@google.com>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583191}
[modify] https://crrev.com/0ecac34302d2d95059c3ea716f118c0b1cfcedd2/third_party/WebKit/LayoutTests/external/wpt/css/cssom/interfaces-expected.txt
[modify] https://crrev.com/0ecac34302d2d95059c3ea716f118c0b1cfcedd2/third_party/WebKit/LayoutTests/fast/css/CSSStyleSheet-constructable.html
[modify] https://crrev.com/0ecac34302d2d95059c3ea716f118c0b1cfcedd2/third_party/WebKit/LayoutTests/fast/dom/dom-constructors-expected.txt
[modify] https://crrev.com/0ecac34302d2d95059c3ea716f118c0b1cfcedd2/third_party/blink/renderer/core/css/css_style_sheet.cc
[modify] https://crrev.com/0ecac34302d2d95059c3ea716f118c0b1cfcedd2/third_party/blink/renderer/core/css/css_style_sheet.h
[modify] https://crrev.com/0ecac34302d2d95059c3ea716f118c0b1cfcedd2/third_party/blink/renderer/core/css/css_style_sheet.idl
[modify] https://crrev.com/0ecac34302d2d95059c3ea716f118c0b1cfcedd2/third_party/blink/renderer/core/css/css_style_sheet_test.cc
[modify] https://crrev.com/0ecac34302d2d95059c3ea716f118c0b1cfcedd2/third_party/blink/renderer/core/html/custom/custom_element_registry_test.cc

Status: Fixed (was: Assigned)

Sign in to add a comment