Bug 112262 - [V8] Get rid of more function-level static FunctionTemplates and ObjectTemplates in bindings
Summary: [V8] Get rid of more function-level static FunctionTemplates and ObjectTempla...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Marja Hölttä
URL:
Keywords:
Depends on:
Blocks: 111724
  Show dependency treegraph
 
Reported: 2013-03-13 08:41 PDT by Marja Hölttä
Modified: 2013-03-13 17:19 PDT (History)
7 users (show)

See Also:


Attachments
Patch (8.25 KB, patch)
2013-03-13 10:00 PDT, Marja Hölttä
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marja Hölttä 2013-03-13 08:41:10 PDT
Bug 111971 wasn't quite enough, there is more. :/

This will also make some tests pass even if we add the original (reverted) patch from bug 111724 (but not all, so more fixes are needed).
Comment 1 Marja Hölttä 2013-03-13 10:00:13 PDT
Created attachment 192940 [details]
Patch
Comment 2 Build Bot 2013-03-13 11:46:49 PDT
Comment on attachment 192940 [details]
Patch

Attachment 192940 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17134546

New failing tests:
fast/forms/range/slider-delete-while-dragging-thumb.html
Comment 3 jochen 2013-03-13 14:46:16 PDT
Comment on attachment 192940 [details]
Patch

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

> Source/WebCore/bindings/v8/custom/V8HTMLDocumentCustom.cpp:58
> +    static const char* shadowTemplateUniqueKey = "wrapInShadowObjectShadowTemplate";

why are you not just duplicating the static as in V8DOMWindow::GetShadowObjectTemplate?
Comment 4 Marja Hölttä 2013-03-13 14:54:01 PDT
Comment on attachment 192940 [details]
Patch

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

>> Source/WebCore/bindings/v8/custom/V8HTMLDocumentCustom.cpp:58
>> +    static const char* shadowTemplateUniqueKey = "wrapInShadowObjectShadowTemplate";
> 
> why are you not just duplicating the static as in V8DOMWindow::GetShadowObjectTemplate?

Because we have a mechanism for storing these kind of "private" FunctionTemplates (but not private ObjectTemplates). And this is (supposedly) more sane than having non-trivial statics inside funcs.
Comment 5 jochen 2013-03-13 15:25:36 PDT
Comment on attachment 192940 [details]
Patch

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

>>> Source/WebCore/bindings/v8/custom/V8HTMLDocumentCustom.cpp:58
>>> +    static const char* shadowTemplateUniqueKey = "wrapInShadowObjectShadowTemplate";
>> 
>> why are you not just duplicating the static as in V8DOMWindow::GetShadowObjectTemplate?
> 
> Because we have a mechanism for storing these kind of "private" FunctionTemplates (but not private ObjectTemplates). And this is (supposedly) more sane than having non-trivial statics inside funcs.

wouldn't it be more sane to introduce private object templates then?
Comment 6 Marja Hölttä 2013-03-13 15:34:12 PDT
Comment on attachment 192940 [details]
Patch

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

>>>> Source/WebCore/bindings/v8/custom/V8HTMLDocumentCustom.cpp:58
>>>> +    static const char* shadowTemplateUniqueKey = "wrapInShadowObjectShadowTemplate";
>>> 
>>> why are you not just duplicating the static as in V8DOMWindow::GetShadowObjectTemplate?
>> 
>> Because we have a mechanism for storing these kind of "private" FunctionTemplates (but not private ObjectTemplates). And this is (supposedly) more sane than having non-trivial statics inside funcs.
> 
> wouldn't it be more sane to introduce private object templates then?

For only this one case?
Comment 7 jochen 2013-03-13 15:43:13 PDT
Comment on attachment 192940 [details]
Patch

ok
Comment 8 WebKit Review Bot 2013-03-13 15:52:25 PDT
Comment on attachment 192940 [details]
Patch

Clearing flags on attachment: 192940

Committed r145765: <http://trac.webkit.org/changeset/145765>
Comment 9 WebKit Review Bot 2013-03-13 15:52:29 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Kentaro Hara 2013-03-13 17:19:30 PDT
Comment on attachment 192940 [details]
Patch

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

>>>>> Source/WebCore/bindings/v8/custom/V8HTMLDocumentCustom.cpp:58
>>>>> +    static const char* shadowTemplateUniqueKey = "wrapInShadowObjectShadowTemplate";
>>>> 
>>>> why are you not just duplicating the static as in V8DOMWindow::GetShadowObjectTemplate?
>>> 
>>> Because we have a mechanism for storing these kind of "private" FunctionTemplates (but not private ObjectTemplates). And this is (supposedly) more sane than having non-trivial statics inside funcs.
>> 
>> wouldn't it be more sane to introduce private object templates then?
> 
> For only this one case?

The way wrapInShadowObject() is called is very special and weird... I'm going to move the logic to V8HTMLDocument::wrap().