Drop [LegacyImplementedInBaseClass] attribute from WindowTimers

Drop [LegacyImplementedInBaseClass] attribute from WindowTimers

Drop [LegacyImplementedInBaseClass] IDL extended attribute from the
WindowTimers interface and move the code to a new DOMWindowTimers
file to reduce code duplication.

There is still some duplication in the custom bindings code but this
will be addressed in a later patch to limit patch size.

R=arv@chromium.org, haraken@chromium.org

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

[haraken, 2013-07-19 11:25:44]

https://codereview.chromium.org/19801002/diff/1/Source/core/page/DOMWindowTim...
File Source/core/page/DOMWindowTimers.cpp (right):

https://codereview.chromium.org/19801002/diff/1/Source/core/page/DOMWindowTim...
Source/core/page/DOMWindowTimers.cpp:41: namespace DOMWindowTimers {

Why do you use a namespace instead of a class? I expected:

  class DOMWindowTimers {
    static int setTimeout(...);
  };

this is just for consistency with how partial interfaces are implemented.

[do-not-use, 2013-07-19 11:48:03]

On 2013/07/19 11:25:44, haraken wrote:
>
https://codereview.chromium.org/19801002/diff/1/Source/core/page/DOMWindowTim...
> File Source/core/page/DOMWindowTimers.cpp (right):
> 
>
https://codereview.chromium.org/19801002/diff/1/Source/core/page/DOMWindowTim...
> Source/core/page/DOMWindowTimers.cpp:41: namespace DOMWindowTimers {
> 
> Why do you use a namespace instead of a class? I expected:
> 
>   class DOMWindowTimers {
>     static int setTimeout(...);
>   };
> 
> this is just for consistency with how partial interfaces are implemented.

I did that for DOMWindowBase64 already and you did not say anything :)

The reason I chose to use a namespace instead of a class with static methods is
because a namespace is sufficient here and more lightweight. We never need to
construct an object, we don't need encapsulation. I did a quick test and
confirmed that using a namespace leads to a slightly smaller binary size here as
well.

I got a review on WebKit side one day saying that I should prefer namespaces
over classes with static methods in such cases so I have been following this
pattern ever since.

That said, I don't mind doing either way.

[haraken, 2013-07-19 11:58:44]

> I did that for DOMWindowBase64 already and you did not say anything :)
> 
> The reason I chose to use a namespace instead of a class with static methods
is
> because a namespace is sufficient here and more lightweight. We never need to
> construct an object, we don't need encapsulation. I did a quick test and
> confirmed that using a namespace leads to a slightly smaller binary size here
as
> well.
> 
> I got a review on WebKit side one day saying that I should prefer namespaces
> over classes with static methods in such cases so I have been following this
> pattern ever since.
> 
> That said, I don't mind doing either way.

Thanks for the clarification. Your point sounds reasonable to me. LGTM.

[arv (Not doing code reviews), 2013-07-19 14:03:35]

LGTM

[commit-bot: I haz the power, 2013-07-19 14:08:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/19801002/8001

[commit-bot: I haz the power, 2013-07-19 14:18:42]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_blink_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-07-19 15:47:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/19801002/8001

[commit-bot: I haz the power, 2013-07-19 15:51:52]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_blink_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-07-19 17:11:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/19801002/8001

[do-not-use, 2013-07-19 17:38:50]

Committed patchset #3 manually as r154579 (presubmit successful).