Summary: | REGRESSION(r207753-207755): ASSERTION FAILED: m_parsedStyleSheetCache->isInMemoryCache() | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryan Haddad <ryanhaddad> | ||||||||
Component: | New Bugs | Assignee: | youenn fablet <youennf> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, kling, koivisto, youennf | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=163870 https://bugs.webkit.org/show_bug.cgi?id=164095 https://bugs.webkit.org/show_bug.cgi?id=164257 https://bugs.webkit.org/show_bug.cgi?id=164080 https://bugs.webkit.org/show_bug.cgi?id=164246 |
||||||||||
Attachments: |
|
Description
Ryan Haddad
2016-10-24 11:58:28 PDT
Seems to have started with https://trac.webkit.org/changeset/207757 Per flakiness dashboard, this seems to be asserting on El Capitan and Sierra WK1 https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fmisc%2Facid2.html Also, http/tests/misc/acid2-pixel.html seems to fail alongside this test: http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fmisc%2Facid2-pixel.html There is one instance of these two tests failing (not asserting) with https://trac.webkit.org/log/?verbose=on&rev=207755&stop_rev=207753 I suspect this is caused by https://bugs.webkit.org/show_bug.cgi?id=161859 even though flakes started later. Created attachment 292917 [details]
Patch
(In reply to comment #6) > Created attachment 292917 [details] > Patch Patch implements https://bugs.webkit.org/show_bug.cgi?id=161859#c7. (In reply to comment #7) > (In reply to comment #6) > > Created attachment 292917 [details] > > Patch > > Patch implements https://bugs.webkit.org/show_bug.cgi?id=161859#c7. This patch has no test in it. I am not exactly sure how to test it except than checking the flakiness is disappearing. I think you should try to make a test as this really needs coverage. Something along these lines should work 1) Load a simple stylesheet with <link> 2) Load the same stylesheet in a manner that setBodyDataFrom sharing gets used (in another frame I think, I don't know what the case exactly is) 3) Call internals.beginSimulatedMemoryPressure(); Memory pressure should cause destroyDecodedData() get called for both CachedCSSStyleSheets and the second one should hit ASSERT(m_isInMemoryCache) without this patch. A test that actually produces wrong rendering would involve kicking one of the resources out from memory cache (via new Internals api maybe) and mutating the stylesheet. Thanks for the review. (In reply to comment #9) > I think you should try to make a test as this really needs coverage. > Something along these lines should work > > 1) Load a simple stylesheet with <link> > 2) Load the same stylesheet in a manner that setBodyDataFrom sharing gets > used (in another frame I think, I don't know what the case exactly is) I wrote some tests in http/tests/security/cached-cross-origin-preloaded-css-stylesheet.html and cached-cross-origin-preloading-css-stylesheet.html setBodyDataFrom is used when a request is made that matches the cache but the origin or the fetch mode is not matching. > 3) Call internals.beginSimulatedMemoryPressure(); > > Memory pressure should cause destroyDecodedData() get called for both > CachedCSSStyleSheets and the second one should hit ASSERT(m_isInMemoryCache) > without this patch. Great, I was not aware of internals.beginSimulatedMemoryPressure(). This should indeed work. I'll add this test. > Great, I was not aware of internals.beginSimulatedMemoryPressure().
> This should indeed work.
> I'll add this test.
You can also always add new Internals APIs as needed if you don't find something suitable already. They are very easy to add.
Created attachment 293000 [details]
Patch for landing
Comment on attachment 293000 [details] Patch for landing Clearing flags on attachment: 293000 Committed r207967: <http://trac.webkit.org/changeset/207967> All reviewed patches have been landed. Closing bug. Reverted r207967 for reason: This change seems to be the cause of at least one LayoutTest becoming flaky. Committed r208200: <http://trac.webkit.org/changeset/208200> *** Bug 164257 has been marked as a duplicate of this bug. *** Created attachment 293655 [details]
Patch for landing
(In reply to comment #17) > Created attachment 293655 [details] > Patch for landing The added test was calling internals.beginSimulatedMemoryPressure() but it was not calling internals.endSimulatedMemoryPressure(). This explains LayoutTests/memory/memory-pressure-simulation.html flakiness and may also explain other tests flakiness. Let's check that. Comment on attachment 293655 [details] Patch for landing Clearing flags on attachment: 293655 Committed r208279: <http://trac.webkit.org/changeset/208279> All reviewed patches have been landed. Closing bug. |