Bug 239460 – Inline Element::shadowRoot()

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Chris Dumez]

Inline Element::shadowRoot() given that it is small and shows up in Speedometer profiles.

[Chris Dumez]

Created attachment 457818 [details]
Patch

[Chris Dumez]

Created attachment 457823 [details]
Patch

[Chris Dumez]

Created attachment 457828 [details]
Patch

[Chris Dumez]

Created attachment 457834 [details]
Patch

[Chris Dumez]

Created attachment 457839 [details]
Patch

[Chris Dumez]

Created attachment 457842 [details]
Patch

[Chris Dumez]

Created attachment 457848 [details]
Patch

[Chris Dumez]

Created attachment 457849 [details]
Patch

[Chris Dumez]

Created attachment 457850 [details]
Patch

[Chris Dumez]

Created attachment 457854 [details]
Patch

[Darin Adler]

Comment on attachment 457854 [details]
Patch

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

> Source/WebCore/dom/Element.h:318
> +    ShadowRoot* shadowRoot() const;

If we add the inline keyword to the declaration of shadowRoot (like replace WEBCORE_EXPORT with inline) then, I seem to recall, we get compiler errors rather than linker errors when we call it without remembering to include ElementRareData.h. So I suggest including that keyword. We should use that technique more! I think Jer Noble is the one who pointed this out to me.

> Source/WebCore/editing/cocoa/DataDetection.mm:694
> +std::optional<std::pair<Ref<HTMLElement>, IntRect>> DataDetection::findDataDetectionResultElementInImageOverlay(const FloatPoint& location, const HTMLElement& imageOverlayHost)

This seems like a really good change, but not mentioned in change log.

[Chris Dumez]

(In reply to Darin Adler from comment #11)
> Comment on attachment 457854 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457854&action=review
> 
> > Source/WebCore/dom/Element.h:318
> > +    ShadowRoot* shadowRoot() const;
> 
> If we add the inline keyword to the declaration of shadowRoot (like replace
> WEBCORE_EXPORT with inline) then, I seem to recall, we get compiler errors
> rather than linker errors when we call it without remembering to include
> ElementRareData.h. So I suggest including that keyword. We should use that
> technique more! I think Jer Noble is the one who pointed this out to me.
> 
> > Source/WebCore/editing/cocoa/DataDetection.mm:694
> > +std::optional<std::pair<Ref<HTMLElement>, IntRect>> DataDetection::findDataDetectionResultElementInImageOverlay(const FloatPoint& location, const HTMLElement& imageOverlayHost)
> 
> This seems like a really good change, but not mentioned in change log.

Ok, I'll mention in the changelog. The issue is that we'd need to include ElementRareData.h from the WebKit layer otherwise. And this is currently not possible because this header is not exported.

[Chris Dumez]

(In reply to Darin Adler from comment #11)
> Comment on attachment 457854 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=457854&action=review
> 
> > Source/WebCore/dom/Element.h:318
> > +    ShadowRoot* shadowRoot() const;
> 
> If we add the inline keyword to the declaration of shadowRoot (like replace
> WEBCORE_EXPORT with inline) then, I seem to recall, we get compiler errors
> rather than linker errors when we call it without remembering to include
> ElementRareData.h. So I suggest including that keyword. We should use that
> technique more! I think Jer Noble is the one who pointed this out to me.

Nice trick. This is good to know, thanks. I'll adopt in this patch.

[Chris Dumez]

Created attachment 457913 [details]
Patch

[Chris Dumez]

Created attachment 457921 [details]
Patch

[EWS]

Committed r293042 (249781@main): <https://commits.webkit.org/249781@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 457921 [details].

[Radar WebKit Bug Importer]

<rdar://problem/91992406>