Web Inspector: Crash inspecting styles of element with mutated stylesheet

[Joseph Pecoraro]

* SUMMARY
Crash inspecting styles of element with mutated stylesheet.

* REDUCTION
<style>.foo, .bar { color: red }</style>
<div class="bar">Inspect Me</div>
<script>document.styleSheets[0].insertRule("body { padding: 10px; }", 0);</script>

* STEPS TO REPRODUCE
1. Inspect .bar in test case
  => CRASH

[Joseph Pecoraro]

<rdar://problem/19783568>

[Joseph Pecoraro]

Created attachment 246353 [details]
[PATCH] Proposed Fix

[Build Bot]

Comment on attachment 246353 [details]
[PATCH] Proposed Fix

Attachment 246353 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5035795962396672

New failing tests:
inspector/css/stylesheet-with-mutations.html

[Build Bot]

Created attachment 246361 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 246353 [details]
[PATCH] Proposed Fix

Attachment 246353 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5484304364732416

New failing tests:
inspector/css/stylesheet-with-mutations.html

[Build Bot]

Created attachment 246362 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Joseph Pecoraro]

Oh oops. Will update patch with new strings.

[Joseph Pecoraro]

Created attachment 246364 [details]
[PATCH] Proposed Fix

[Timothy Hatcher]

Comment on attachment 246364 [details]
[PATCH] Proposed Fix

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

> Source/WebCore/css/CSSStyleSheet.h:91
> +    bool hadRulesMutation() const { return m_mutatedRules; }

hasRulesMutation?

> Source/WebCore/css/CSSStyleSheet.h:92
> +    void clearHadRulesMutation() { m_mutatedRules = false; }

s/had/has/

[Chris Dumez]

Comment on attachment 246364 [details]
[PATCH] Proposed Fix

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

> Source/WebCore/inspector/InspectorStyleSheet.cpp:865
>      for (size_t i = 0, size = ranges.size(); i < size; ++i) {

nit: We could use a nice C++11 range loop here.

[Joseph Pecoraro]

(In reply to comment #9)
> Comment on attachment 246364 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=246364&action=review
> 
> > Source/WebCore/css/CSSStyleSheet.h:91
> > +    bool hadRulesMutation() const { return m_mutatedRules; }
> 
> hasRulesMutation?
> 
> > Source/WebCore/css/CSSStyleSheet.h:92
> > +    void clearHadRulesMutation() { m_mutatedRules = false; }
> 
> s/had/has/

I like "had" here. At some point, the rules mutated, they might have reverted back to the original, but all this tracks is that there was some mutation in the past.

[WebKit Commit Bot]

Comment on attachment 246364 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 246364

Committed r180005: <http://trac.webkit.org/changeset/180005>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

The new test doesn't work well on some bots, filed bug 141601.