Oilpan: Prepare to move form control ua shadow elements to Oilpan heap.

Oilpan: Prepare to move form control ua shadow elements to Oilpan heap.

BUG=357163
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=173628

[haraken, 2014-05-08 08:16:05]

LGTM

[tkent, 2014-05-08 08:19:30]

lgtm

[keishi, 2014-05-08 08:21:21]

The CQ bit was checked by keishi@chromium.org

[commit-bot: I haz the power, 2014-05-08 08:21:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/keishi@chromium.org/267283007/1

[Mads Ager (chromium), 2014-05-08 08:22:39]

https://codereview.chromium.org/267283007/diff/1/Source/core/html/forms/TextF...
File Source/core/html/forms/TextFieldInputType.h (right):

https://codereview.chromium.org/267283007/diff/1/Source/core/html/forms/TextF...
Source/core/html/forms/TextFieldInputType.h:44:
WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(TextFieldInputType);
You should implement trace here to make sure that you get the SpinButtonOwner
traced:

SpinButtonElement::SpinButtonOwner::trace(visitor);
InputType::trace(visitor);

[keishi, 2014-05-08 08:22:59]

The CQ bit was unchecked by keishi@chromium.org

[haraken, 2014-05-08 08:26:11]

https://codereview.chromium.org/267283007/diff/1/Source/core/html/forms/TextF...
File Source/core/html/forms/TextFieldInputType.h (right):

https://codereview.chromium.org/267283007/diff/1/Source/core/html/forms/TextF...
Source/core/html/forms/TextFieldInputType.h:44:
WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(TextFieldInputType);
On 2014/05/08 08:22:39, Mads Ager (chromium) wrote:
> You should implement trace here to make sure that you get the SpinButtonOwner
> traced:
> 
> SpinButtonElement::SpinButtonOwner::trace(visitor);
> InputType::trace(visitor);

Nice catch. Can this be detected by the plugin?

[Mads Ager (chromium), 2014-05-08 08:27:53]

Ian, this illustrates something where we have to be careful and we should see if
we can make the plugin catch this.

If you do not have any members that need to be traced but you have a mixin that
does need tracing you need to implement trace so that the mixin's trace method
is called.

[zerny-chromium, 2014-05-08 08:32:45]

On 2014/05/08 08:27:53, Mads Ager (chromium) wrote:
> Ian, this illustrates something where we have to be careful and we should see
if
> we can make the plugin catch this.
> 
> If you do not have any members that need to be traced but you have a mixin
that
> does need tracing you need to implement trace so that the mixin's trace method
> is called.

Good catch! I'll add this requirement to the plugin asap.

[keishi, 2014-05-08 09:28:15]

https://codereview.chromium.org/267283007/diff/1/Source/core/html/forms/TextF...
File Source/core/html/forms/TextFieldInputType.h (right):

https://codereview.chromium.org/267283007/diff/1/Source/core/html/forms/TextF...
Source/core/html/forms/TextFieldInputType.h:44:
WILL_BE_USING_GARBAGE_COLLECTED_MIXIN(TextFieldInputType);
On 2014/05/08 08:22:39, Mads Ager (chromium) wrote:
> You should implement trace here to make sure that you get the SpinButtonOwner
> traced:
> 
> SpinButtonElement::SpinButtonOwner::trace(visitor);
> InputType::trace(visitor);

Done.

Added SpinButtonOwner::trace, TextFieldInputType::trace and
BaseMultipleFieldsDateAndTimeInputType::trace

[Mads Ager (chromium), 2014-05-08 09:37:06]

I'm sorry for the noise here. :( You actually don't have to call the
SpinButtonOwner::trace method because there are no fields and nothing to trace.
This is a pure interface; is misread the code.

So, feel free to undo this and go back to the way it was before. Ian is
implementing the correct checks in the plugin so that we will catch if tracing
becomes needed here at some point.

Sorry again for the noise; your original patch was correct.

LGTM

[keishi, 2014-05-08 09:41:03]

The CQ bit was checked by keishi@chromium.org

[commit-bot: I haz the power, 2014-05-08 09:41:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/keishi@chromium.org/267283007/1

[commit-bot: I haz the power, 2014-05-08 09:42:34]

Change committed as 173628