Move btoa() / atob() implementation out of DOMWindow

Move btoa() / atob() implementation out of DOMWindow

Move btoa() / atob() implementation out of DOMWindow and move it to a
new DOMWindowBase64 namespace. Those functions are now static and can
easily be reused by WorkerGlobalScope later on when we want to expose
them to workers (as per the specification).

Implements statements now generate the same code as partial
interfaces, meaning that the functions are expected to be static, in
class / namespace of the same name as the supplemental interface.
A [LegacyImplementedInBaseClass] extended attribute is introduced to
maintain compatibility with existing supplemental interfaces and so
that their code can be gradually updated.

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

[abarth-chromium, 2013-07-18 08:17:14]

https://codereview.chromium.org/19675008/diff/1/Source/core/page/DOMWindow.h
File Source/core/page/DOMWindow.h (right):

https://codereview.chromium.org/19675008/diff/1/Source/core/page/DOMWindow.h#...
Source/core/page/DOMWindow.h:91: class DOMWindow : public RefCounted<DOMWindow>,
public ScriptWrappable, public EventTarget, public FrameDestructionObserver,
public DOMWindowBase64, public Supplementable<DOMWindow> {
Why do we need to subclass?  Can't we just use a partial interface and make
these static functions?

[do-not-use, 2013-07-18 08:24:54]

On 2013/07/18 08:17:14, abarth wrote:
> https://codereview.chromium.org/19675008/diff/1/Source/core/page/DOMWindow.h
> File Source/core/page/DOMWindow.h (right):
> 
>
https://codereview.chromium.org/19675008/diff/1/Source/core/page/DOMWindow.h#...
> Source/core/page/DOMWindow.h:91: class DOMWindow : public
RefCounted<DOMWindow>,
> public ScriptWrappable, public EventTarget, public FrameDestructionObserver,
> public DOMWindowBase64, public Supplementable<DOMWindow> {
> Why do we need to subclass?  Can't we just use a partial interface and make
> these static functions?

The spec [1] does not use a partial interface. Instead, it uses IDL implements
statements. The reason for that is that it is implemented by both Window and
WorkerGlobalScope (https://code.google.com/p/chromium/issues/detail?id=143502).
So this is why I'm moving this to a new class, the plan is to have
WorkerGlobalScope inherit it as well in a follow up patch.

[1]
http://www.whatwg.org/specs/web-apps/current-work/multipage/webappapis.html#w...
   
http://www.whatwg.org/specs/web-apps/current-work/multipage/workers.html#apis...

[haraken, 2013-07-18 08:28:02]

> The spec [1] does not use a partial interface. Instead, it uses IDL implements
> statements. The reason for that is that it is implemented by both Window and
> WorkerGlobalScope
(https://code.google.com/p/chromium/issues/detail?id=143502).
> So this is why I'm moving this to a new class, the plan is to have
> WorkerGlobalScope inherit it as well in a follow up patch.

Would it be possible to implement the IDL 'implements' just like how we
implement 'partial' interfaces? Then we can use static functions and avoid
increasing multiple inheritance of DOMWindow.

[do-not-use, 2013-07-18 08:29:59]

On 2013/07/18 08:28:02, haraken wrote:
> > The spec [1] does not use a partial interface. Instead, it uses IDL
implements
> > statements. The reason for that is that it is implemented by both Window and
> > WorkerGlobalScope
> (https://code.google.com/p/chromium/issues/detail?id=143502).
> > So this is why I'm moving this to a new class, the plan is to have
> > WorkerGlobalScope inherit it as well in a follow up patch.
> 
> Would it be possible to implement the IDL 'implements' just like how we
> implement 'partial' interfaces? Then we can use static functions and avoid
> increasing multiple inheritance of DOMWindow.

This is an idea worth investigating. This will likely lead to quite a bit of
refactoring though because of existing implements statements (like a lot in SVG
and we use inheritance there).

[haraken, 2013-07-18 08:32:14]

> > Would it be possible to implement the IDL 'implements' just like how we
> > implement 'partial' interfaces? Then we can use static functions and avoid
> > increasing multiple inheritance of DOMWindow.
> 
> This is an idea worth investigating. This will likely lead to quite a bit of
> refactoring though because of existing implements statements (like a lot in
SVG
> and we use inheritance there).

Yeah, given that we don't want to increase multiple inheritance in the Blink
side just for IDL issues, the refactoring might be worth doing.

[do-not-use, 2013-07-18 08:37:46]

On 2013/07/18 08:32:14, haraken wrote:
> > > Would it be possible to implement the IDL 'implements' just like how we
> > > implement 'partial' interfaces? Then we can use static functions and avoid
> > > increasing multiple inheritance of DOMWindow.
> > 
> > This is an idea worth investigating. This will likely lead to quite a bit of
> > refactoring though because of existing implements statements (like a lot in
> SVG
> > and we use inheritance there).
> 
> Yeah, given that we don't want to increase multiple inheritance in the Blink
> side just for IDL issues, the refactoring might be worth doing.

Agreed. I'll look into it.

[do-not-use, 2013-07-18 11:31:11]

I updated the CL as suggested. I introduced a new "Legacy*" extended attribute
for supplemental interfaces whose code needs to be refactored so that it can be
done incrementally.

Note that I used a namespace for DOMWindowBase64 instead of a class as we don't
really need a class IMHO and it compiles with the same generated code. I'm fine
changing either way though.

[haraken, 2013-07-18 12:55:29]

LGTM. This looks better.

https://codereview.chromium.org/19675008/diff/12001/Source/bindings/scripts/I...
File Source/bindings/scripts/IDLAttributes.txt (right):

https://codereview.chromium.org/19675008/diff/12001/Source/bindings/scripts/I...
Source/bindings/scripts/IDLAttributes.txt:64: LegacyInDestinationClass

Maybe [LegacyImplementedInBaseClass] sounds better.

Also, shall we add a FIXME that says this IDL attribute is going to be removed
soon?

[do-not-use, 2013-07-18 13:01:01]

On 2013/07/18 12:55:29, haraken wrote:
> LGTM. This looks better.
> 
>
https://codereview.chromium.org/19675008/diff/12001/Source/bindings/scripts/I...
> File Source/bindings/scripts/IDLAttributes.txt (right):
> 
>
https://codereview.chromium.org/19675008/diff/12001/Source/bindings/scripts/I...
> Source/bindings/scripts/IDLAttributes.txt:64: LegacyInDestinationClass
> 
> Maybe [LegacyImplementedInBaseClass] sounds better.
> 
> Also, shall we add a FIXME that says this IDL attribute is going to be removed
> soon?

Done.

[commit-bot: I haz the power, 2013-07-18 13:42:02]

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

[commit-bot: I haz the power, 2013-07-18 15:48:37]

Change committed as 154492

[arv (Not doing code reviews), 2013-07-18 19:24:32]

LGTM

Very nice