Bug 159412 – Using dpi unit in sizes attribute raises SIGSEGV

• Component: CSS
• Status: RESOLVED FIXED

[Fujii Hironori]

Using dpi in sizes attribute fails an assertion

BuildBot of Gtk port, Debug build fails

https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Debug%20%28Tests%29/builds/9952

> imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute.html [ Crash ]

Log:

> ASSERTION FAILED: m_ptr
> ../../Source/WTF/wtf/RefPtr.h(68) : WTF::Ref<T> WTF::RefPtr<T>::releaseNonNull() [with T = WebCore::CSSValue]

Callstack:

> #0  0x00007f1db87451e1 in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:323
> #1  0x00007f1dbef02f3c in WTF::RefPtr<WebCore::CSSValue>::releaseNonNull (this=0x7fff894f7d90) at ../../Source/WTF/wtf/RefPtr.h:68
> #2  0x00007f1dbefa9022 in (anonymous namespace)::CSSParser::sourceSize(<unknown type in /home/fujii/work/webkit/w1/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0xd3361>, (anonymous namespace)::CSSParserValue &) (this=0x7fff894fa4e0, expression=<unknown type in /home/fujii/work/webkit/w1/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0xd3361>, parserValue=...) at ../../Source/WebCore/css/CSSParser.cpp:1560
> #3  0x00007f1dc02eea7c in cssyyparse (parser=0x7fff894fa4e0) at /home/fujii/work/webkit/w1/WebKitBuild/Debug/DerivedSources/WebCore/CSSGrammar.y:451
> #4  0x00007f1dbefa8d7e in (anonymous namespace)::CSSParser::parseSizesAttribute (this=0x7fff894fa4e0, string=...) at ../../Source/WebCore/css/CSSParser.cpp:1522
> #5  0x00007f1dbf088830 in (anonymous namespace)::parseSizesAttribute (document=..., sizesAttribute=...) at ../../Source/WebCore/css/SourceSizeList.cpp:77
> #6  0x00007f1dbf3eaa9e in (anonymous namespace)::HTMLImageElement::selectImageSource (this=0x7f1d4e5bae40) at ../../Source/WebCore/html/HTMLImageElement.cpp:193
> #7  0x00007f1dbf3eac82 in (anonymous namespace)::HTMLImageElement::parseAttribute (this=0x7f1d4e5bae40, name=..., value=...) at ../../Source/WebCore/html/HTMLImageElement.cpp:206
> #8  0x00007f1dbf1af5a2 in (anonymous namespace)::Element::attributeChanged (this=0x7f1d4e5bae40, name=..., oldValue=..., newValue=...) at ../../Source/WebCore/dom/Element.cpp:1276
> #9  0x00007f1dbf263f8c in (anonymous namespace)::StyledElement::attributeChanged (this=0x7f1d4e5bae40, name=..., oldValue=..., newValue=..., reason=(anonymous namespace)::Element::ModifiedDirectly) at ../../Source/WebCore/dom/StyledElement.cpp:164
> #10 0x00007f1dbf1b000e in (anonymous namespace)::Element::parserSetAttributes (this=0x7f1d4e5bae40, attributeVector=...) at ../../Source/WebCore/dom/Element.cpp:1450
> #11 0x00007f1dbf4fe3f5 in (anonymous namespace)::setAttributes (element=..., attributes=..., parserContentPolicy=(anonymous namespace)::AllowScriptingContent) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:60
> #12 0x00007f1dbf4fe42c in (anonymous namespace)::setAttributes (element=..., token=..., parserContentPolicy=(anonymous namespace)::AllowScriptingContent) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:65
> #13 0x00007f1dbf501a7a in (anonymous namespace)::HTMLConstructionSite::createHTMLElementOrFindCustomElementInterface (this=0x7f1da0893800, token=..., customElementInterface=0x0) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:692
> #14 0x00007f1dbf501b4a in (anonymous namespace)::HTMLConstructionSite::createHTMLElement (this=0x7f1da0893800, token=...) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:699
> #15 0x00007f1dbf500a4a in (anonymous namespace)::HTMLConstructionSite::insertSelfClosingHTMLElement (this=0x7f1da0893800, token=...) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:509
> #16 0x00007f1dbf532a25 in (anonymous namespace)::HTMLTreeBuilder::processStartTagForInBody (this=0x7f1da08937e0, token=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:780
> #17 0x00007f1dbf53494c in (anonymous namespace)::HTMLTreeBuilder::processStartTag (this=0x7f1da08937e0, token=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:1141
> #18 0x00007f1dbf53042b in (anonymous namespace)::HTMLTreeBuilder::processToken (this=0x7f1da08937e0, token=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:381
> #19 0x00007f1dbf530283 in (anonymous namespace)::HTMLTreeBuilder::constructTree (this=0x7f1da08937e0, token=...) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:351
> #20 0x00007f1dbf507f09 in (anonymous namespace)::HTMLDocumentParser::constructTreeFromHTMLToken (this=0x7f1d4fdda100, rawToken=...) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:324
> #21 0x00007f1dbf507acb in (anonymous namespace)::HTMLDocumentParser::pumpTokenizerLoop (this=0x7f1d4fdda100, mode=(anonymous namespace)::HTMLDocumentParser::AllowYield, parsingFragment=false, session=...) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:264
> #22 0x00007f1dbf507c73 in (anonymous namespace)::HTMLDocumentParser::pumpTokenizer (this=0x7f1d4fdda100, mode=(anonymous namespace)::HTMLDocumentParser::AllowYield) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:282
> #23 0x00007f1dbf50750d in (anonymous namespace)::HTMLDocumentParser::pumpTokenizerIfPossible (this=0x7f1d4fdda100, mode=(anonymous namespace)::HTMLDocumentParser::AllowYield) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:168
> #24 0x00007f1dbf50840e in (anonymous namespace)::HTMLDocumentParser::append(<unknown type in /home/fujii/work/webkit/w1/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x3f043>) (this=0x7f1d4fdda100, inputSource=<unknown type in /home/fujii/work/webkit/w1/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x3f043>) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:394
> #25 0x00007f1dbf135d23 in (anonymous namespace)::DecodedDataDocumentParser::appendBytes (this=0x7f1d4fdda100, writer=..., data=0x7f1d4fdb5200 "es='(}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e76 50w, /images/green-16x16.png?e76 51w' sizes='not (}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e77 50w, /images/green-16x16.png?e77 51"..., length=8192) at ../../Source/WebCore/dom/DecodedDataDocumentParser.cpp:50
> #26 0x00007f1dbf67084d in (anonymous namespace)::DocumentWriter::addData (this=0x7f1d4fdef0a0, bytes=0x7f1d4fdb5200 "es='(}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e76 50w, /images/green-16x16.png?e76 51w' sizes='not (}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e77 50w, /images/green-16x16.png?e77 51"..., length=8192) at ../../Source/WebCore/loader/DocumentWriter.cpp:249
> #27 0x00007f1dbf65d60b in (anonymous namespace)::DocumentLoader::commitData (this=0x7f1d4fdef000, bytes=0x7f1d4fdb5200 "es='(}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e76 50w, /images/green-16x16.png?e76 51w' sizes='not (}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e77 50w, /images/green-16x16.png?e77 51"..., length=8192) at ../../Source/WebCore/loader/DocumentLoader.cpp:927
> #28 0x00007f1dbe7eb73a in (anonymous namespace)::WebFrameLoaderClient::committedLoad (this=0x1d58350, loader=0x7f1d4fdef000, data=0x7f1d4fdb5200 "es='(}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e76 50w, /images/green-16x16.png?e76 51w' sizes='not (}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e77 50w, /images/green-16x16.png?e77 51"..., length=8192) at ../../Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:932
> #29 0x00007f1dbf65d1e0 in (anonymous namespace)::DocumentLoader::commitLoad (this=0x7f1d4fdef000, data=0x7f1d4fdb5200 "es='(}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e76 50w, /images/green-16x16.png?e76 51w' sizes='not (}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e77 50w, /images/green-16x16.png?e77 51"..., length=8192) at ../../Source/WebCore/loader/DocumentLoader.cpp:843
> #30 0x00007f1dbf65d861 in (anonymous namespace)::DocumentLoader::dataReceived (this=0x7f1d4fdef000, resource=0x7f1da08f6700, data=0x7f1d4fdb5200 "es='(}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e76 50w, /images/green-16x16.png?e76 51w' sizes='not (}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e77 50w, /images/green-16x16.png?e77 51"..., length=8192) at ../../Source/WebCore/loader/DocumentLoader.cpp:955
> #31 0x00007f1dbf718630 in (anonymous namespace)::CachedRawResource::notifyClientsDataWasReceived (this=0x7f1da08f6700, data=0x7f1d4fdb5200 "es='(}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e76 50w, /images/green-16x16.png?e76 51w' sizes='not (}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e77 50w, /images/green-16x16.png?e77 51"..., length=8192) at ../../Source/WebCore/loader/cache/CachedRawResource.cpp:119
> #32 0x00007f1dbf718304 in (anonymous namespace)::CachedRawResource::addDataBuffer (this=0x7f1da08f6700, data=...) at ../../Source/WebCore/loader/cache/CachedRawResource.cpp:69
> #33 0x00007f1dbf6cb432 in (anonymous namespace)::SubresourceLoader::didReceiveDataOrBuffer(const char *, int, <unknown type in /home/fujii/work/webkit/w1/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x5b13d>, long long, (anonymous namespace)::DataPayloadType) (this=0x7f1d5cbe4b80, data=0x7f1d4fdd3450 "es='(}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e76 50w, /images/green-16x16.png?e76 51w' sizes='not (}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e77 50w, /images/green-16x16.png?e77 51"..., length=8192, prpBuffer=<unknown type in /home/fujii/work/webkit/w1/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x5b13d>, encodedDataLength=8192, dataPayloadType=(anonymous namespace)::DataPayloadBytes) at ../../Source/WebCore/loader/SubresourceLoader.cpp:333
> #34 0x00007f1dbf6cb18e in (anonymous namespace)::SubresourceLoader::didReceiveData (this=0x7f1d5cbe4b80, data=0x7f1d4fdd3450 "es='(}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e76 50w, /images/green-16x16.png?e76 51w' sizes='not (}) 100vw, 1px'>\n<img srcset='/images/green-1x1.png?e77 50w, /images/green-16x16.png?e77 51"..., length=8192, encodedDataLength=8192, dataPayloadType=(anonymous namespace)::DataPayloadBytes) at ../../Source/WebCore/loader/SubresourceLoader.cpp:309
> #35 0x00007f1dbe7482aa in (anonymous namespace)::WebResourceLoader::didReceiveData (this=0x7f1da08ae6f0, data=..., encodedDataLength=8192) at ../../Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp:144
> #36 0x00007f1dbea7a0a0 in IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long), std::tuple<IPC::DataReference, long>, 0ul, 1ul>((anonymous namespace)::WebResourceLoader *, void ((anonymous namespace)::WebResourceLoader::*)((anonymous namespace)::WebResourceLoader * const, const IPC::DataReference &, long), <unknown type in /home/fujii/work/webkit/w1/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x31fc2>, std::index_sequence) (object=0x7f1da08ae6f0, function=(void ((anonymous namespace)::WebResourceLoader::*)((anonymous namespace)::WebResourceLoader * const, const IPC::DataReference &, long)) 0x7f1dbe7481c4 <(anonymous namespace)::WebResourceLoader::didReceiveData(IPC::DataReference const&, int64_t)>, args=<unknown type in /home/fujii/work/webkit/w1/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x31fc2>) at ../../Source/WebKit2/Platform/IPC/HandleMessage.h:16
> #37 0x00007f1dbea799c0 in IPC::callMemberFunction<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long), std::tuple<IPC::DataReference, long> >(<unknown type in /home/fujii/work/webkit/w1/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x31fc2>, (anonymous namespace)::WebResourceLoader *, void ((anonymous namespace)::WebResourceLoader::*)((anonymous namespace)::WebResourceLoader * const, const IPC::DataReference &, long)) (args=<unknown type in /home/fujii/work/webkit/w1/WebKitBuild/Debug/lib/libwebkit2gtk-4.0.so.37, CU 0x0, DIE 0x31fc2>, object=0x7f1da08ae6f0, function=(void ((anonymous namespace)::WebResourceLoader::*)((anonymous namespace)::WebResourceLoader * const, const IPC::DataReference &, long)) 0x7f1dbe7481c4 <(anonymous namespace)::WebResourceLoader::didReceiveData(IPC::DataReference const&, int64_t)>) at ../../Source/WebKit2/Platform/IPC/HandleMessage.h:22
> #38 0x00007f1dbea793a2 in IPC::handleMessage<Messages::WebResourceLoader::DidReceiveData, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long)> (decoder=..., object=0x7f1da08ae6f0, function=(void ((anonymous namespace)::WebResourceLoader::*)((anonymous namespace)::WebResourceLoader * const, const IPC::DataReference &, long)) 0x7f1dbe7481c4 <(anonymous namespace)::WebResourceLoader::didReceiveData(IPC::DataReference const&, int64_t)>) at ../../Source/WebKit2/Platform/IPC/HandleMessage.h:92
> #39 0x00007f1dbea78bdd in (anonymous namespace)::WebResourceLoader::didReceiveWebResourceLoaderMessage (this=0x7f1da08ae6f0, connection=..., decoder=...) at DerivedSources/WebKit2/WebResourceLoaderMessageReceiver.cpp:58
> #40 0x00007f1dbe73b768 in (anonymous namespace)::NetworkProcessConnection::didReceiveMessage (this=0x7f1da09eb120, connection=..., decoder=...) at ../../Source/WebKit2/WebProcess/Network/NetworkProcessConnection.cpp:58
> #41 0x00007f1dbe432162 in IPC::Connection::dispatchMessage (this=0x7f1da09e71c0, decoder=...) at ../../Source/WebKit2/Platform/IPC/Connection.cpp:887
> #42 0x00007f1dbe4322c6 in IPC::Connection::dispatchMessage (this=0x7f1da09e71c0, message=std::unique_ptr<IPC::MessageDecoder> containing 0x7f1da09e2cc0) at ../../Source/WebKit2/Platform/IPC/Connection.cpp:918
> #43 0x00007f1dbe4324a4 in IPC::Connection::dispatchOneMessage (this=0x7f1da09e71c0) at ../../Source/WebKit2/Platform/IPC/Connection.cpp:949
> #44 0x00007f1dbe432002 in IPC::Connection::<lambda()>::operator()(void) (__closure=0x7f1da094f108) at ../../Source/WebKit2/Platform/IPC/Connection.cpp:881
> #45 0x00007f1dbe436fd6 in WTF::Function<void()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::unique_ptr<IPC::MessageDecoder>)::<lambda()> >::call(void) (this=0x7f1da094f100) at ../../Source/WTF/wtf/Function.h:89
> #46 0x00007f1dbe401317 in WTF::Function<void()>::operator()(void) const (this=0x7fff894fc910) at ../../Source/WTF/wtf/Function.h:50
> #47 0x00007f1db87606ba in WTF::RunLoop::performWork (this=0x7f1da09f9180) at ../../Source/WTF/wtf/RunLoop.cpp:122
> #48 0x00007f1db879b2c2 in WTF::RunLoop::<lambda(gpointer)>::operator()(gpointer) const (__closure=0x0, userData=0x7f1da09f9180) at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:66
> #49 0x00007f1db879b2e6 in WTF::RunLoop::<lambda(gpointer)>::_FUN(gpointer) () at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:68
> #50 0x00007f1db879b262 in WTF::<lambda(GSource*, GSourceFunc, gpointer)>::operator()(GSource *, GSourceFunc, gpointer) const (__closure=0x0, source=0x2167510, callback=0x7f1db879b2c9 <WTF::RunLoop::<lambda(gpointer)>::_FUN(gpointer)>, userData=0x7f1da09f9180) at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:44
> #51 0x00007f1db879b291 in WTF::<lambda(GSource*, GSourceFunc, gpointer)>::_FUN(GSource *, GSourceFunc, gpointer) () at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:45
> #52 0x00007f1db19fb5f7 in g_main_dispatch () from /home/fujii/work/webkit/w1/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0
> #53 0x00007f1db19fc42e in g_main_context_dispatch () from /home/fujii/work/webkit/w1/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0
> #54 0x00007f1db19fc612 in g_main_context_iterate () from /home/fujii/work/webkit/w1/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0
> #55 0x00007f1db19fca38 in g_main_loop_run () from /home/fujii/work/webkit/w1/WebKitBuild/DependenciesGTK/Root/lib/libglib-2.0.so.0
> #56 0x00007f1db879b842 in WTF::RunLoop::run () at ../../Source/WTF/wtf/glib/RunLoopGLib.cpp:94
> #57 0x00007f1dbe9e4309 in (anonymous namespace)::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain> (argc=2, argv=0x7fff894fcd48) at ../../Source/WebKit2/Shared/unix/ChildProcessMain.h:61
> #58 0x00007f1dbe9e4170 in (anonymous namespace)::WebProcessMainUnix (argc=2, argv=0x7fff894fcd48) at ../../Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:78
> #59 0x0000000000400c5a in main (argc=2, argv=0x7fff894fcd48) at ../../Source/WebKit2/WebProcess/EntryPoint/unix/WebProcessMain.cpp:44

[Fujii Hironori]

Created attachment 282755 [details]
simplified test case

[Fujii Hironori]

CSSParserValue::createCSSValue returns null for a dpi value.

This code was introduced in:

https://trac.webkit.org/changeset/179476
Bug 141026 – REGRESSION (r170576): Storage leaks in parsing of CSS image sizes

[Fujii Hironori]

Created attachment 282837 [details]
parse-a-sizes-attribute-crash-log.txt

Not only assertion failure, but also crash in release build.

> imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/sizes/parse-a-sizes-attribute.html [ Crash ]

https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Tests%29/builds/16838/steps/layout-test/logs/stdio

[Fujii Hironori]

Created attachment 282840 [details]
Patch

[Yoav Weiss]

Debugged this issue and ended up with a local patch that looks very similar to what you came up with. I'm not a Reviewer, but r+ from me.

[Michael Catanzaro]

We also need to check on the status of bug #146434 (Dean?) before unskipping this test, since it wasn't skipped due to this crash.

[Fujii Hironori]

H Michael,

Thank you for review my patch.
The build flag PICTURE_SIZES does not exist anymore.
It is always usable for all ports.

  Bug 144679 – Remove the PICTURE_SIZES build flag

[Fujii Hironori]

(In reply to comment #7)
>   Bug 144679 – Remove the PICTURE_SIZES build flag

Sorry. Wrong bug id.

  Bug 150275 – Rename the PICTURE_SIZES flag to CURRENTSRC
  Bug 153545 – Remove ENABLE_CURRENTSRC

[Darin Adler]

I am not sure the patch is OK. Seems a bit sloppy to turn any invalid value into CSS_UNKOWN.

What is the correct behavior when this kind of invalid value is specified for a source size? Should it be a parsing error? Should the size be silently omitted from the parsed result?

I looked at the original code before I fixed the storage leak. As far as I can tell, that original code still had the same issue. In the old code a null pointer dereference would happen inside the computeLength function in the SourceSizeList.cpp file.

https://trac.webkit.org/browser/trunk/Source/WebCore/css/SourceSizeList.cpp?rev=176719

Assuming that we want to silently ignore the single size, I think the cleanest way to write the sourceSize function would be to have it return Optional<CSSParser::SourceSize> and return no source size at all when the value is invalid rather than instead using a size with a CSS_UNKNOWN primitive value in it.

The call site in CSSGrammar.y.in could just not append a size in that case.

But I am not sure what the desired behavior is in this error case.

[Darin Adler]

Comment on attachment 282840 [details]
Patch

It’s OK to land this patch,but I think a follow up might be needed to address the comments I made.

[WebKit Commit Bot]

Comment on attachment 282840 [details]
Patch

Clearing flags on attachment: 282840

Committed r203060: <http://trac.webkit.org/changeset/203060>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Yoav Weiss]

https://html.spec.whatwg.org/multipage/embedded-content.html#parse-a-sizes-attribute has the full algorithm governing the parsing of the sizes attribute.

In this case, we encounter an invalid source-size-value and should skip to the next iteration of the algorithm (so skip to the next source-size/source-size-value).

As you say, returning an Optional from CSSParser::sourceSize for this case and avoiding to add that to the SourceSize vector in source_size_list: would be a better way to handle that scenario.

[Fujii Hironori]

Thank you for reveiw. I confirmed GTK and EFL BuildBot stops crashing at the test case.

(In reply to comment #9)
> I looked at the original code before I fixed the storage leak. As far as I
> can tell, that original code still had the same issue. In the old code a
> null pointer dereference would happen inside the computeLength function in
> the SourceSizeList.cpp file.

Ah, you are right. My comment#2 is wrong.

(In reply to comment #10)
> It’s OK to land this patch,but I think a follow up might be needed to
> address the comments I made.

Filed.

  Bug 159666 – Change CSSParser::sourceSize returning Optional<CSSParser::SourceSize>