Bug 85970 - ElementShadow should minimize the usage of "ShadowRoot" name
Summary: ElementShadow should minimize the usage of "ShadowRoot" name
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on:
Blocks: 85263
  Show dependency treegraph
 
Reported: 2012-05-09 02:22 PDT by Hajime Morrita
Modified: 2012-05-10 22:34 PDT (History)
6 users (show)

See Also:


Attachments
Patch (41.50 KB, patch)
2012-05-09 23:23 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (45.56 KB, patch)
2012-05-10 00:31 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (47.33 KB, patch)
2012-05-10 01:59 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch for landing (48.17 KB, patch)
2012-05-10 17:22 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch for landing (48.17 KB, patch)
2012-05-10 17:49 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch for landing (48.17 KB, patch)
2012-05-10 18:01 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 2012-05-09 02:22:00 PDT
Since ElementShadow exists for encapsulate the shadow dom concept, it doesn't makes sense to have these names in API.
Comment 1 Hajime Morrita 2012-05-09 23:23:33 PDT
Created attachment 141091 [details]
Patch
Comment 2 Hajime Morrita 2012-05-09 23:24:30 PDT
Hi Dimitri, could you take a look at this cleanup exercise?
Comment 3 Build Bot 2012-05-09 23:50:43 PDT
Comment on attachment 141091 [details]
Patch

Attachment 141091 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12648773
Comment 4 Build Bot 2012-05-09 23:53:01 PDT
Comment on attachment 141091 [details]
Patch

Attachment 141091 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12644953
Comment 5 Hajime Morrita 2012-05-10 00:31:49 PDT
Created attachment 141099 [details]
Patch
Comment 6 Ryosuke Niwa 2012-05-10 00:36:17 PDT
Comment on attachment 141091 [details]
Patch

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

Looks reasonable.

> Source/WebCore/dom/ElementShadow.cpp:93
>      // Dont protect this ref count.

This comment can be improved by explaining why.

> Source/WebCore/html/HTMLMediaElement.cpp:4077
> +    ElementShadow* shadow = this->shadow();

Should we name this variable elementShadow so that you don't have to use this pointer?

> Source/WebCore/inspector/InspectorDOMAgent.cpp:1234
> +        ElementShadow* shadow = element->shadow();
> +        if (shadow) {

You should declare shadow inside the if.
Comment 7 Ryosuke Niwa 2012-05-10 00:37:10 PDT
Comment on attachment 141099 [details]
Patch

Please address the comment I posted above.
Comment 8 Build Bot 2012-05-10 01:00:43 PDT
Comment on attachment 141099 [details]
Patch

Attachment 141099 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12664249
Comment 9 Hajime Morrita 2012-05-10 01:59:39 PDT
Created attachment 141111 [details]
Patch
Comment 10 Hajime Morrita 2012-05-10 17:22:40 PDT
Created attachment 141298 [details]
Patch for landing
Comment 11 Hajime Morrita 2012-05-10 17:23:37 PDT
Ryosuke, Dimitri, thanks for reviewing!
The landing patch addresses Ryosuke's comment.
Comment 12 Hajime Morrita 2012-05-10 17:49:47 PDT
Created attachment 141300 [details]
Patch for landing
Comment 13 Hajime Morrita 2012-05-10 18:01:19 PDT
Created attachment 141303 [details]
Patch for landing
Comment 14 WebKit Review Bot 2012-05-10 22:34:31 PDT
Comment on attachment 141303 [details]
Patch for landing

Clearing flags on attachment: 141303

Committed r116730: <http://trac.webkit.org/changeset/116730>
Comment 15 WebKit Review Bot 2012-05-10 22:34:36 PDT
All reviewed patches have been landed.  Closing bug.