Bug 59228

Summary:

Remove IsShadowRootFlag from Node

Product:

WebKit

Reporter:

Dimitri Glazkov (Google) <dglazkov>

Component:

DOM

Assignee:

Dimitri Glazkov (Google) <dglazkov>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

dominicc, morrita, rolandsteiner

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Bug Depends on:

48698

Bug Blocks:

72352, 59802

Attachments:

Description	Flags
Patch	none

Dimitri Glazkov (Google)

Reported 2011-04-22 14:06:38 PDT

Once shadow DOM always starts with the ShadowRoot element, we no longer need this flag.

Attachments
Patch (17.51 KB, patch) 2011-06-07 14:55 PDT, Dimitri Glazkov (Google)	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Dimitri Glazkov (Google)

Comment 1 2011-06-07 14:55:22 PDT

Created attachment 96302 [details] Patch

Darin Adler

Comment 2 2011-06-07 16:24:11 PDT

Comment on attachment 96302 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96302&action=review > Source/WebCore/dom/Node.h:213 > - bool isShadowRoot() const { return getFlag(IsShadowRootFlag); } > - // FIXME: Remove this when all shadow roots are ShadowRoots. > - virtual bool isShadowBoundary() const { return false; } > + virtual bool isShadowRoot() const { return false; } A virtual function is not as fast as checking a bit. And in the past we have optimized various code paths by replacing virtual functions with a bit in Node. Are you sure that this does not create a slowdown?

Dimitri Glazkov (Google)

Comment 3 2011-06-07 17:13:59 PDT

(In reply to comment #2) > (From update of attachment 96302 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96302&action=review > > > Source/WebCore/dom/Node.h:213 > > - bool isShadowRoot() const { return getFlag(IsShadowRootFlag); } > > - // FIXME: Remove this when all shadow roots are ShadowRoots. > > - virtual bool isShadowBoundary() const { return false; } > > + virtual bool isShadowRoot() const { return false; } > > A virtual function is not as fast as checking a bit. And in the past we have optimized various code paths by replacing virtual functions with a bit in Node. > > Are you sure that this does not create a slowdown? I will check. I wrote a microbench for this a while back.

Dimitri Glazkov (Google)

Comment 4 2011-06-07 17:14:22 PDT

Comment on attachment 96302 [details] Patch Actually, this patch sucks. I can do better.

Dimitri Glazkov (Google)

Comment 5 2011-06-08 12:46:42 PDT

This patch has at least two parts. I need to split out the simple ones.

Dominic Cooney

Comment 6 2011-11-20 17:58:35 PST

Actually we tried replacing this with a virtual, but it regressed performance because parentNode needs to test isShadowRoot; see bug 69266. For now I think we assume that the node flag stays.

Note You need to log in before you can comment on or make changes to this bug.