Bug 69406 - IsShadowRootFlag should not depend on whether the ShadowRoot is attached to a host
Summary: IsShadowRootFlag should not depend on whether the ShadowRoot is attached to a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dominic Cooney
URL:
Keywords:
Depends on:
Blocks: 69266
  Show dependency tree / graph
 
Reported: 2011-10-05 00:58 PDT by Dominic Cooney
Modified: 2011-10-05 19:26 PDT (History)
3 users (show)

See Also:

Attachments
Patch (3.82 KB, patch)
2011-10-05 01:16 PDT, Dominic Cooney 		no flags Details | Formatted Diff | Diff
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 Dominic Cooney 2011-10-05 00:58:45 PDT 
To improve performance I am proposing we use IsShadowRootFlag and isSVGShadowRoot virtual slightly differently:

getFlag(IsShadowRootFlag) => ShadowRoot or SVGShadowRoot
getFlag(IsShadowRootFlag) && !getFlag(IsSVGElementFlag) => ShadowRoot (ie new style)
getFlag(IsShadowRootFlag) && getFlag(IsSVGElementFlag) => SVGShadowRoot

This will fix a performance regression in parentNode (see bug 69266) and remove a virtual from Node (isSVGShadowRoot can just consult flags.)

One way isSVGShadowRoot and getFlag(IsShadowRootFlag) vary today is that isSVGShadowRoot is associated with the SVGShadowRoot type the vtable; it never changes for a given instance. But ShadowRoot sets and clears getFlag(IsShadowRootFlag) when a ShadowRoot is attached/detached to a host. I think this is needless variation. I think we should always set IsShadowRootFlag for ShadowRoot instances. If the caller wants to find out whether it is associated with a host, it can test null-ness of shadowHost().
Comment 1 Dominic Cooney 2011-10-05 01:16:03 PDT 
Created attachment 109757 [details]
Patch
Comment 2 Hajime Morrita 2011-10-05 18:46:20 PDT 
Comment on attachment 109757 [details]
Patch

Looks sane.
Comment 3 WebKit Review Bot 2011-10-05 19:26:41 PDT 
Comment on attachment 109757 [details]
Patch

Clearing flags on attachment: 109757

Committed r96782: <http://trac.webkit.org/changeset/96782>
Comment 4 WebKit Review Bot 2011-10-05 19:26:45 PDT 
All reviewed patches have been landed.  Closing bug.