Bug 163905 – REGRESSION(r207753-207755): ASSERTION FAILED: m_parsedStyleSheetCache->isInMemoryCache()

• Component: New Bugs
• Status: RESOLVED FIXED

[Ryan Haddad]

Flaky assertion failure seen with http/tests/misc/acid2.html

ASSERTION FAILED: m_parsedStyleSheetCache->isInMemoryCache()
/Volumes/Data/slave/elcapitan-debug/build/Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp(167) : PassRefPtr<WebCore::StyleSheetContents> WebCore::CachedCSSStyleSheet::restoreParsedStyleSheet(const WebCore::CSSParserContext &, WebCore::CachePolicy)
1   0x10fe69310 WTFCrash
2   0x1153778fd WebCore::CachedCSSStyleSheet::restoreParsedStyleSheet(WebCore::CSSParserContext const&, WebCore::CachePolicy)
3   0x115e261f5 WebCore::HTMLLinkElement::setCSSStyleSheet(WTF::String const&, WebCore::URL const&, WTF::String const&, WebCore::CachedCSSStyleSheet const*)
4   0x115e265c7 non-virtual thunk to WebCore::HTMLLinkElement::setCSSStyleSheet(WTF::String const&, WebCore::URL const&, WTF::String const&, WebCore::CachedCSSStyleSheet const*)
5   0x1153770ea WebCore::CachedCSSStyleSheet::didAddClient(WebCore::CachedResourceClient&)
6   0x11538e430 WebCore::CachedResource::addClient(WebCore::CachedResourceClient&)
7   0x115e25541 WebCore::HTMLLinkElement::process()
8   0x115e25e2c WebCore::HTMLLinkElement::insertedInto(WebCore::ContainerNode&)
9   0x1154af9c8 WebCore::notifyNodeInsertedIntoDocument(WebCore::ContainerNode&, WebCore::Node&, WTF::Vector<WTF::Ref<WebCore::Node>, 11ul, WTF::CrashOnOverflow, 16ul>&)
10  0x1154afdf0 WebCore::notifyChildNodeInserted(WebCore::ContainerNode&, WebCore::Node&, WTF::Vector<WTF::Ref<WebCore::Node>, 11ul, WTF::CrashOnOverflow, 16ul>&)
11  0x1154a09a6 WebCore::ContainerNode::notifyChildInserted(WebCore::Node&, WebCore::ContainerNode::ChildChangeSource)
12  0x11549fa2c WebCore::ContainerNode::parserAppendChild(WebCore::Node&)
13  0x115d94a63 WebCore::insert(WebCore::HTMLConstructionSiteTask&)
14  0x115d946ee WebCore::executeInsertTask(WebCore::HTMLConstructionSiteTask&)
15  0x115d90ed2 WebCore::executeTask(WebCore::HTMLConstructionSiteTask&)
16  0x115d90e66 WebCore::HTMLConstructionSite::executeQueuedTasks()
17  0x115ee4f0e WebCore::HTMLTreeBuilder::constructTree(WebCore::AtomicHTMLToken&)
18  0x115dc45e3 WebCore::HTMLDocumentParser::constructTreeFromHTMLToken(WebCore::HTMLTokenizer::TokenPtr&)
19  0x115dc44ef WebCore::HTMLDocumentParser::pumpTokenizerLoop(WebCore::HTMLDocumentParser::SynchronousMode, bool, WebCore::PumpSession&)
20  0x115dc31ed WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode)
21  0x115dc2dee WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode)
22  0x115dc4fd3 WebCore::HTMLDocumentParser::append(WTF::RefPtr<WTF::StringImpl>&&)
23  0x1157ff2f5 WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter&, char const*, unsigned long)
24  0x115941e93 WebCore::DocumentWriter::addData(char const*, unsigned long)
25  0x1158fd01e WebCore::DocumentLoader::commitData(char const*, unsigned long)
26  0x11ec903fd -[WebFrame(WebInternal) _commitData:]
27  0x11ecce610 -[WebHTMLRepresentation receivedData:withDataSource:]
28  0x11ec6cbea -[WebDataSource(WebInternal) _receivedData:]
29  0x11eca09d7 WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int)
30  0x1158ffb30 WebCore::DocumentLoader::commitLoad(char const*, int)
31  0x1158ffa56 WebCore::DocumentLoader::dataReceived(char const*, int)

[Ryan Haddad]

Seems to have started with https://trac.webkit.org/changeset/207757

[Ryan Haddad]

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

[Ryan Haddad]

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

[Ryan Haddad]

There is one instance of these two tests failing (not asserting) with https://trac.webkit.org/log/?verbose=on&rev=207755&stop_rev=207753

[Antti Koivisto]

I suspect this is caused by https://bugs.webkit.org/show_bug.cgi?id=161859 even though flakes started later.

[youenn fablet]

Created attachment 292917 [details]
Patch

[youenn fablet]

(In reply to comment #6)
> Created attachment 292917 [details]
> Patch

Patch implements https://bugs.webkit.org/show_bug.cgi?id=161859#c7.

[youenn fablet]

(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.

[Antti Koivisto]

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.

[youenn fablet]

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.

[Antti Koivisto]

> 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.

[youenn fablet]

Created attachment 293000 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 293000 [details]
Patch for landing

Clearing flags on attachment: 293000

Committed r207967: <http://trac.webkit.org/changeset/207967>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryan Haddad]

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>

[Ryan Haddad]

*** Bug 164257 has been marked as a duplicate of this bug. ***

[youenn fablet]

Created attachment 293655 [details]
Patch for landing

[youenn fablet]

(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.

[WebKit Commit Bot]

Comment on attachment 293655 [details]
Patch for landing

Clearing flags on attachment: 293655

Committed r208279: <http://trac.webkit.org/changeset/208279>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.