webkit_unit_tests (FontFaceCacheTest.FLAKY_MatchCombinations) failing |
|||||
Issue descriptionFiled 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
,
Aug 10
Was already marked flaky 2 days ago in Issue 871812, but now failing 100% of the time.
,
Aug 10
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.
,
Aug 10
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.
,
Aug 10
As expected, this revert didn't help. Next on the chopping block: r581836
,
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
,
Aug 10
When can we revert back crrev.com/c/1170462? I think it has nothing to do with FontFace caches.
,
Aug 10
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.
,
Aug 10
Hmm I see, it's weird. Thanks for explaining, we'll try to figure it out before relanding.
,
Aug 10
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
,
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
,
Sep 19
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by mgiuca@chromium.org
, Aug 10Labels: OS-Linux