Bug 113768 - Web Inspector: crash in WebCore::InspectorLayerTreeAgent::buildObjectForLayer if a layer is created for an anonymous RenderObject (:first-letter)
Summary: Web Inspector: crash in WebCore::InspectorLayerTreeAgent::buildObjectForLayer...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Antoine Quint
URL: http://sahilparikh.com/post/467416897...
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-04-02 04:27 PDT by Antoine Quint
Modified: 2013-04-03 04:05 PDT (History)
12 users (show)

See Also:

Attachments
Patch (12.75 KB, patch)
2013-04-02 09:57 PDT, Antoine Quint 		no flags Details | Formatted Diff | Diff
Patch for landing (12.73 KB, patch)
2013-04-03 03:11 PDT, Antoine Quint 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2013-04-02 04:27:17 PDT 
Steps to reproduce:

1. Load <http://sahilparikh.com/post/46741689726/building-a-saas-business-2007-vs-2013#> in a WebKit nightly
2. Open the Safari Web Inspector
3. Click on the layers pane
4. *crash*

The issue here is that a CSS rule using the :first-letter pseudo-element creates a composited layer (due to "backface-visilibity: hidden") and the InspectorLayerTreeAgent doesn't know how to handle such "special" RenderObjects. As a result, we get a null Node and when we ask for its nodeId, we eventually crash. We should be both more protective of unknown nodes and handle anonymous RenderObjects specifically.
Comment 1 Radar WebKit Bug Importer 2013-04-02 04:28:01 PDT 
<rdar://problem/13555463>
Comment 2 Antoine Quint 2013-04-02 09:57:06 PDT 
Created attachment 196169 [details]
Patch
Comment 3 David Kilzer (:ddkilzer) 2013-04-02 10:18:16 PDT 
Comment on attachment 196169 [details]
Patch

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

> Source/WebCore/inspector/InspectorLayerTreeAgent.cpp:206
> +        RenderStyle* style = renderer->style();
> +        if (style) {

FWIW, this can be simplified to:

    if (RenderStyle* style = renderer->style()) {
Comment 4 Simon Fraser (smfr) 2013-04-02 10:54:41 PDT 
Comment on attachment 196169 [details]
Patch

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

> Source/WebCore/inspector/InspectorLayerTreeAgent.cpp:175
> +    else if (isReflection || isAnonymous)
>          node = renderer->parent()->node();

Does this always work? What about a :first-letter inside a :first-line?
Comment 5 Antoine Quint 2013-04-03 02:51:40 PDT 
<rdar://problem/13555463>
Comment 6 Antoine Quint 2013-04-03 03:04:27 PDT 
(In reply to comment #4)
> (From update of attachment 196169 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=196169&action=review
> 
> > Source/WebCore/inspector/InspectorLayerTreeAgent.cpp:175
> > +    else if (isReflection || isAnonymous)
> >          node = renderer->parent()->node();
> 
> Does this always work? What about a :first-letter inside a :first-line?

Per my testing, that works too. I haven't found a way to have a ::first-letter be composited though, animations don't apply, neither do transforms, etc.
Comment 7 Antoine Quint 2013-04-03 03:11:57 PDT 
Created attachment 196313 [details]
Patch for landing
Comment 8 Antoine Quint 2013-04-03 03:12:24 PDT 
<rdar://problem/13544596>
Comment 9 WebKit Review Bot 2013-04-03 04:05:17 PDT 
Comment on attachment 196313 [details]
Patch for landing

Clearing flags on attachment: 196313

Committed r147538: <http://trac.webkit.org/changeset/147538>
Comment 10 WebKit Review Bot 2013-04-03 04:05:22 PDT 
All reviewed patches have been landed.  Closing bug.