bluetooth: Refactor Bluetooth and Stub out BluetoothInteraction.

bluetooth: Refactor Bluetooth and Stub out BluetoothInteraction.

Two changes:
1. Refactor Bluetooth to call BluetoothDiscovery's functions instead of inheriting the class.
2. Stub out BluetoothInteraction.

First step for implementing the BluetoothInteraction interface:

https://webbluetoothcg.github.io/web-bluetooth/#idl-def-bluetoothinteraction

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

[ortuno, 2015-04-23 22:04:19]

ortuno@chromium.org changed reviewers:
+ scheib@chromium.org

[ortuno, 2015-04-23 22:04:19]

@scheib please review

[scheib, 2015-04-23 22:31:21]

LGTM

[haraken, 2015-04-24 05:58:42]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2015-04-24 05:58:43]

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
File Source/modules/bluetooth/Bluetooth.h (right):

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
Source/modules/bluetooth/Bluetooth.h:11: #include "platform/heap/Visitor.h"

Include platform/heap/Handle.h (this is our convention.)

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
File Source/modules/bluetooth/BluetoothInteraction.h (right):

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
Source/modules/bluetooth/BluetoothInteraction.h:9: #include
"platform/heap/Visitor.h"

#include platform/heap/Handle.h instead of these two (this is our convention).

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
Source/modules/bluetooth/BluetoothInteraction.h:15: : public
GarbageCollectedMixin {

You need DEFINE_WRAPPERTYPEINFO() here. DEFINE_WRAPPERTYPEINFO() is needed for
each subtype.

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
Source/modules/bluetooth/BluetoothInteraction.h:18: // Interface required by
Garbage Collection:

We normally don't have this comment.

[haraken, 2015-04-24 06:19:05]

class BluetoothInteraction : public GarbageCollected<BluetoothInteraction>,
public ScriptWrappable {
  DEFINE_WRAPPERTYPEINFO()
};

class Bluetooth : public BluetoothInteraction, public BluetoothDiscovery {
  DEFINE_WRAPPERTYPEINFO()
};

Won't this work?

[ortuno, 2015-04-24 06:23:12]

Thanks for the help @haraken! As mentioned before I get an error if I include
DEFINE_WRAPPERTYPEINFO();

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
File Source/modules/bluetooth/Bluetooth.h (right):

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
Source/modules/bluetooth/Bluetooth.h:11: #include "platform/heap/Visitor.h"
On 2015/04/24 at 05:58:42, haraken wrote:
> Include platform/heap/Handle.h (this is our convention.)

Done.

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
File Source/modules/bluetooth/BluetoothInteraction.h (right):

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
Source/modules/bluetooth/BluetoothInteraction.h:9: #include
"platform/heap/Visitor.h"
On 2015/04/24 at 05:58:42, haraken wrote:
> #include platform/heap/Handle.h instead of these two (this is our convention).

Done.

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
Source/modules/bluetooth/BluetoothInteraction.h:15: : public
GarbageCollectedMixin {
On 2015/04/24 at 05:58:42, haraken wrote:
> You need DEFINE_WRAPPERTYPEINFO() here. DEFINE_WRAPPERTYPEINFO() is needed for
each subtype.

Done. But I get an error:

error: 'wrapperTypeInfo' marked 'override' but does not override any member
functions

https://codereview.chromium.org/1066933008/diff/60001/Source/modules/bluetoot...
Source/modules/bluetooth/BluetoothInteraction.h:18: // Interface required by
Garbage Collection:
On 2015/04/24 at 05:58:42, haraken wrote:
> We normally don't have this comment.

Done.

[ortuno, 2015-04-24 06:29:45]

On 2015/04/24 at 06:19:05, haraken wrote:
> class BluetoothInteraction : public GarbageCollected<BluetoothInteraction>,
public ScriptWrappable {
>   DEFINE_WRAPPERTYPEINFO()
> };
> 
> class Bluetooth : public BluetoothInteraction, public BluetoothDiscovery {
>   DEFINE_WRAPPERTYPEINFO()
> };
> 
> Won't this work?

What about BluetoothDiscovery?

[PhistucK, 2015-04-24 09:31:59]

phistuck@gmail.com changed reviewers:
+ phistuck@gmail.com

[ortuno, 2015-04-25 01:22:14]

Patchset #6 (id:100001) has been deleted

[ortuno, 2015-04-25 01:27:11]

@scheib please review. Everyone else: please let me know if you think this
should be done in a different way.

[scheib, 2015-04-25 04:01:02]

https://codereview.chromium.org/1066933008/diff/120001/Source/modules/bluetoo...
File Source/modules/bluetooth/Bluetooth.h (right):

https://codereview.chromium.org/1066933008/diff/120001/Source/modules/bluetoo...
Source/modules/bluetooth/Bluetooth.h:38: Member<BluetoothDiscovery>
m_bluetoothDiscovery;
I think these can be direct members, not requiring additional objects to be
allocated and garbage collected. See
https://www.chromium.org/blink/blink-gc#TOC-Part-classes

[ortuno, 2015-04-27 22:22:01]

@scheib: ready for review again.

https://codereview.chromium.org/1066933008/diff/120001/Source/modules/bluetoo...
File Source/modules/bluetooth/Bluetooth.h (right):

https://codereview.chromium.org/1066933008/diff/120001/Source/modules/bluetoo...
Source/modules/bluetooth/Bluetooth.h:38: Member<BluetoothDiscovery>
m_bluetoothDiscovery;
On 2015/04/25 at 04:01:01, scheib wrote:
> I think these can be direct members, not requiring additional objects to be
allocated and garbage collected. See
https://www.chromium.org/blink/blink-gc#TOC-Part-classes

Done.

[ortuno, 2015-04-27 23:56:11]

Patchset #7 (id:140001) has been deleted

[scheib, 2015-04-28 04:14:05]

LGTM

[haraken, 2015-04-28 04:34:05]

LGTM with a comment.

https://codereview.chromium.org/1066933008/diff/160001/Source/modules/bluetoo...
File Source/modules/bluetooth/BluetoothInteraction.h (right):

https://codereview.chromium.org/1066933008/diff/160001/Source/modules/bluetoo...
Source/modules/bluetooth/BluetoothInteraction.h:20: DECLARE_VIRTUAL_TRACE();

Does this need to be virtual? Also where is a definition of the trace?

I guess this should be DEFINE_INLINE_TRACE { }.

[ortuno, 2015-04-28 15:57:20]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2015-04-28 15:57:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/1066933008/#ps180001
(title: " ")

[commit-bot: I haz the power, 2015-04-28 15:57:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1066933008/180001

[ortuno, 2015-04-28 15:57:30]

https://codereview.chromium.org/1066933008/diff/160001/Source/modules/bluetoo...
File Source/modules/bluetooth/BluetoothInteraction.h (right):

https://codereview.chromium.org/1066933008/diff/160001/Source/modules/bluetoo...
Source/modules/bluetooth/BluetoothInteraction.h:20: DECLARE_VIRTUAL_TRACE();
On 2015/04/28 at 04:34:05, haraken wrote:
> Does this need to be virtual? Also where is a definition of the trace?
> 
> I guess this should be DEFINE_INLINE_TRACE { }.

Done.

[commit-bot: I haz the power, 2015-04-28 16:27:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-28 16:27:20]

Try jobs failed on following builders:
  win_blink_compile_dbg on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/bu...)

[ortuno, 2015-04-28 18:23:07]

Patchset #9 (id:200001) has been deleted

[ortuno, 2015-04-28 18:29:13]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2015-04-28 18:29:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/1066933008/#ps220001
(title: " ")

[commit-bot: I haz the power, 2015-04-28 18:29:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1066933008/220001

[commit-bot: I haz the power, 2015-04-28 19:51:12]

Committed patchset #9 (id:220001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194618