Bug 77612

Summary: Stop calling Element::ensureShadowRoot in Internals.
Product: WebKit Reporter: Shinya Kawanaka <shinyak>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gustavo.noronha, gustavo, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 77608    
Attachments:
Description Flags
Test For Building
none
Patch
none
Patch
none
Patch none

Description Shinya Kawanaka 2012-02-02 00:35:03 PST 
We want to remove Element::ensureShadowRoot.

Let's try to stop calling Element::ensureShadowRoot in Internals first, because changing Internals often causes build error.
Comment 1 Shinya Kawanaka 2012-02-02 00:38:44 PST 
Created attachment 125092 [details]
Test For Building
Comment 2 Collabora GTK+ EWS bot 2012-02-02 09:47:10 PST 
Comment on attachment 125092 [details]
Test For Building

Attachment 125092 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11396858
Comment 3 Shinya Kawanaka 2012-02-02 18:38:56 PST 
Created attachment 125230 [details]
Patch
Comment 4 Gustavo Noronha (kov) 2012-02-02 20:28:17 PST 
Comment on attachment 125230 [details]
Patch

Attachment 125230 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11424020
Comment 5 Shinya Kawanaka 2012-02-02 20:54:00 PST 
Created attachment 125251 [details]
Patch
Comment 6 Hajime Morrita 2012-02-02 21:17:17 PST 
Comment on attachment 125251 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Instead of ensureShadowRoot, we check the existence of shadow root and create one if there is not.

Could you explain why we need this change?

> Source/WebCore/testing/Internals.cpp:199
> +    return ShadowRoot::create(host, ec).get();

This is actually safe. But its safety isn't obvious at a glance. 
Could you change the return type to RefPtr, to make this clearly safe?
Comment 7 Shinya Kawanaka 2012-02-02 22:30:36 PST 
> > Source/WebCore/testing/Internals.cpp:199
> > +    return ShadowRoot::create(host, ec).get();
> 
> This is actually safe. But its safety isn't obvious at a glance. 
> Could you change the return type to RefPtr, to make this clearly safe?

Changing PassRefPtr breaks tests actually.
So let me leave this as is.
Comment 8 Shinya Kawanaka 2012-02-02 22:35:38 PST 
Created attachment 125262 [details]
Patch
Comment 9 Hajime Morrita 2012-02-02 22:49:31 PST 
Comment on attachment 125262 [details]
Patch

> Changing PassRefPtr breaks tests actually.
> So let me leave this as is.
Wow.
Comment 10 WebKit Review Bot 2012-02-03 00:16:47 PST 
Comment on attachment 125262 [details]
Patch

Clearing flags on attachment: 125262

Committed r106635: <http://trac.webkit.org/changeset/106635>
Comment 11 WebKit Review Bot 2012-02-03 00:16:52 PST 
All reviewed patches have been landed.  Closing bug.