Bug 156512 - Crash at com.apple.JavaScriptCore: bool WTF::startsWith<WTF::StringImpl, WTF::StringImpl> + 8
Summary: Crash at com.apple.JavaScriptCore: bool WTF::startsWith<WTF::StringImpl, WTF:...
Status: RESOLVED DUPLICATE of bug 156515
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-04-12 13:17 PDT by Antti Koivisto
Modified: 2016-04-13 08:49 PDT (History)
3 users (show)

See Also:


Attachments
patch (7.29 KB, patch)
2016-04-12 13:32 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2016-04-12 13:17:15 PDT
<style> [class^="randomstring"] { background: white; } </style>
<div class="" id="test"></div>
<script> document.getElementById("test").attributes.class.value = null; </script>
Comment 1 Antti Koivisto 2016-04-12 13:17:42 PDT
rdar://problem/24220567
Comment 2 Antti Koivisto 2016-04-12 13:18:38 PDT
We crash in compiled selector code.
Comment 3 Antti Koivisto 2016-04-12 13:32:19 PDT
Created attachment 276270 [details]
patch
Comment 4 Benjamin Poulain 2016-04-12 13:38:49 PDT
Comment on attachment 276270 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=276270&action=review

> Source/WebCore/cssjit/SelectorCompiler.cpp:2823
> +    if (attribute->value().isEmpty())
> +        return false;

Looks like this should be merged with the "AtomicStringImpl& valueImpl = *attribute->value().impl();" below.
Comment 5 Antti Koivisto 2016-04-13 01:30:01 PDT
Looks like Attr.value path that Chris fixed was the only way to make a null attribute value. All normal paths treat null value as "remove the attribute". I think we can do without the null check, I'll just land the test case here.

*** This bug has been marked as a duplicate of bug 156515 ***
Comment 6 Antti Koivisto 2016-04-13 01:38:59 PDT
Landed the test in https://trac.webkit.org/r199428
Comment 7 Darin Adler 2016-04-13 08:21:00 PDT
Comment on attachment 276270 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=276270&action=review

>> Source/WebCore/cssjit/SelectorCompiler.cpp:2823
>> +        return false;
> 
> Looks like this should be merged with the "AtomicStringImpl& valueImpl = *attribute->value().impl();" below.

Checking for the null string would be more efficient than checking for the empty string for the non-null, non-empty case. So unless we want to optimize the empty string case, I suggest using isNull or checking if impl() is nullptr instead.
Comment 8 Darin Adler 2016-04-13 08:21:30 PDT
Oh, looks like we decided not to land this change and instead prevent null from ever getting here. Great!
Comment 9 Darin Adler 2016-04-13 08:49:11 PDT
Comment on attachment 276270 [details]
patch

Clearing the review flag because we decided not to land this.