Bug 77612 - Stop calling Element::ensureShadowRoot in Internals.
Summary: Stop calling Element::ensureShadowRoot in Internals.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 77608
  Show dependency tree / graph
 
Reported: 2012-02-02 00:35 PST by Shinya Kawanaka
Modified: 2012-02-03 00:16 PST (History)
4 users (show)

See Also:

Attachments
Test For Building (1.33 KB, patch)
2012-02-02 00:38 PST, Shinya Kawanaka 		no flags Details | Formatted Diff | Diff
Patch (5.87 KB, patch)
2012-02-02 18:38 PST, Shinya Kawanaka 		no flags Details | Formatted Diff | Diff
Patch (5.87 KB, patch)
2012-02-02 20:54 PST, Shinya Kawanaka 		no flags Details | Formatted Diff | Diff
Patch (6.15 KB, patch)
2012-02-02 22:35 PST, Shinya Kawanaka 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) 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 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.