Attach a WebBluetooth to the LocalFrame and use it if it's available.

Attach a WebBluetooth to the LocalFrame and use it if it's available.

This lets us move the Bluetooth implementation from content/child to content/renderer, which lets us plumb the RenderFrame's ID through Blink.

We also add an error enum for content/ to use if the frame ID is wrong.

This is the first of a 3-patch sequence:
1. Attach a frame-specific WebBluetooth in Blink. (This patch)
2. Move Web Bluetooth from content/child/ to content/renderer/ (https://codereview.chromium.org/1234413006/)
3. Put the RenderFrame's ID in that WebBluetooth and plumb it back to the browser. (https://codereview.chromium.org/1228113004)

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

[Jeffrey Yasskin, 2015-07-15 07:22:32]

The CQ bit was checked by jyasskin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-07-15 07:22:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1228283003/1

[Jeffrey Yasskin, 2015-07-15 07:30:12]

jyasskin@chromium.org changed reviewers:
+ jochen@chromium.org, scheib@chromium.org

[Jeffrey Yasskin, 2015-07-15 07:30:13]

Jochen, you're in charge of Source/web/WebLocalFrameImpl.cpp and
public/web/WebFrameClient.h, but can you also double-check that
BluetoothSupplement implements a sane way to attach the WebBluetooth* to a
LocalFrame?

[commit-bot: I haz the power, 2015-07-15 08:42:22]

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

[commit-bot: I haz the power, 2015-07-15 08:42:22]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2015-07-15 13:45:26]

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

[jochen (gone - plz use gerrit), 2015-07-15 13:45:27]

lgtm for api bits

+haraken for oilpan question

https://codereview.chromium.org/1228283003/diff/1/Source/modules/bluetooth/Bl...
File Source/modules/bluetooth/BluetoothSupplement.h (right):

https://codereview.chromium.org/1228283003/diff/1/Source/modules/bluetooth/Bl...
Source/modules/bluetooth/BluetoothSupplement.h:14: class BLINK_EXPORT
BluetoothSupplement : public NoBaseWillBeGarbageCollected<BluetoothSupplement>,
public WillBeHeapSupplement<LocalFrame> {
i thought stuff in modules/ already uses oilpan, so no need for WillBe types

[haraken, 2015-07-15 13:55:04]

oilpan part LGTM

https://codereview.chromium.org/1228283003/diff/1/Source/modules/bluetooth/Bl...
File Source/modules/bluetooth/BluetoothSupplement.h (right):

https://codereview.chromium.org/1228283003/diff/1/Source/modules/bluetooth/Bl...
Source/modules/bluetooth/BluetoothSupplement.h:14: class BLINK_EXPORT
BluetoothSupplement : public NoBaseWillBeGarbageCollected<BluetoothSupplement>,
public WillBeHeapSupplement<LocalFrame> {
On 2015/07/15 13:45:27, jochen wrote:
> i thought stuff in modules/ already uses oilpan, so no need for WillBe types

Yeah, but this needs to be WillBe because the supplementable object (i.e.,
LocalFrame) is still WillBe. We need to drop the WillBe from the supplement
object and the supplementable object at the same time.

https://codereview.chromium.org/1228283003/diff/1/Source/modules/bluetooth/Bl...
Source/modules/bluetooth/BluetoothSupplement.h:32:
BluetoothSupplement(WebBluetooth*);

Add explicit.

[Jeffrey Yasskin, 2015-07-15 17:19:33]

https://codereview.chromium.org/1228283003/diff/1/Source/modules/bluetooth/Bl...
File Source/modules/bluetooth/BluetoothSupplement.h (right):

https://codereview.chromium.org/1228283003/diff/1/Source/modules/bluetooth/Bl...
Source/modules/bluetooth/BluetoothSupplement.h:14: class BLINK_EXPORT
BluetoothSupplement : public NoBaseWillBeGarbageCollected<BluetoothSupplement>,
public WillBeHeapSupplement<LocalFrame> {
On 2015/07/15 13:55:04, haraken wrote:
> On 2015/07/15 13:45:27, jochen wrote:
> > i thought stuff in modules/ already uses oilpan, so no need for WillBe types
> 
> Yeah, but this needs to be WillBe because the supplementable object (i.e.,
> LocalFrame) is still WillBe. We need to drop the WillBe from the supplement
> object and the supplementable object at the same time.

Thanks.

https://codereview.chromium.org/1228283003/diff/1/Source/modules/bluetooth/Bl...
Source/modules/bluetooth/BluetoothSupplement.h:32:
BluetoothSupplement(WebBluetooth*);
On 2015/07/15 13:55:04, haraken wrote:
> 
> Add explicit.

Oops, done.

[scheib, 2015-07-16 01:03:03]

LGTM w some requests for readability.

https://codereview.chromium.org/1228283003/diff/20001/Source/modules/bluetoot...
File Source/modules/bluetooth/BluetoothSupplement.h (right):

https://codereview.chromium.org/1228283003/diff/20001/Source/modules/bluetoot...
Source/modules/bluetooth/BluetoothSupplement.h:14: class BLINK_EXPORT
BluetoothSupplement : public NoBaseWillBeGarbageCollected<BluetoothSupplement>,
public WillBeHeapSupplement<LocalFrame> {
Class comment please to increase readability for those learning the components.
Supplement is sadly missing overview doc explaining what it does, but even with
that we should explain here the concept that when running in a context with a
frame, we obtain a WebBluetooth implementation from that frame that will be able
to show UI on that frame. It would simplify code understanding to have a trace
of the construction flow that causes the supplement to exist there.

https://codereview.chromium.org/1228283003/diff/20001/Source/modules/bluetoot...
Source/modules/bluetooth/BluetoothSupplement.h:23: static WebBluetooth*
from(LocalFrame*);
We only expect users to call from(ScriptState*), right? Consider moving this to
private?

[Jeffrey Yasskin, 2015-07-16 22:27:14]

I updated this to support moving content/child/bluetooth to
content/renderer/bluetooth. Once the content/ implementation is in renderer/, we
can move its attachment from Platform to WebFrameClient, which requires that
every use of WebBluetooth use BluetoothSupplement instead of
Platform->bluetooth() directly.

https://codereview.chromium.org/1228283003/diff/20001/Source/modules/bluetoot...
File Source/modules/bluetooth/BluetoothSupplement.h (right):

https://codereview.chromium.org/1228283003/diff/20001/Source/modules/bluetoot...
Source/modules/bluetooth/BluetoothSupplement.h:14: class BLINK_EXPORT
BluetoothSupplement : public NoBaseWillBeGarbageCollected<BluetoothSupplement>,
public WillBeHeapSupplement<LocalFrame> {
On 2015/07/16 01:03:03, scheib wrote:
> Class comment please to increase readability for those learning the
components.
> Supplement is sadly missing overview doc explaining what it does, but even
with
> that we should explain here the concept that when running in a context with a
> frame, we obtain a WebBluetooth implementation from that frame that will be
able
> to show UI on that frame. It would simplify code understanding to have a trace
> of the construction flow that causes the supplement to exist there.

How's this?

https://codereview.chromium.org/1228283003/diff/20001/Source/modules/bluetoot...
Source/modules/bluetooth/BluetoothSupplement.h:23: static WebBluetooth*
from(LocalFrame*);
On 2015/07/16 01:03:03, scheib wrote:
> We only expect users to call from(ScriptState*), right? Consider moving this
to
> private? 

I've inlined and deleted it.

[scheib, 2015-07-16 23:04:17]

LGTM

https://codereview.chromium.org/1228283003/diff/20001/Source/modules/bluetoot...
File Source/modules/bluetooth/BluetoothSupplement.h (right):

https://codereview.chromium.org/1228283003/diff/20001/Source/modules/bluetoot...
Source/modules/bluetooth/BluetoothSupplement.h:14: class BLINK_EXPORT
BluetoothSupplement : public NoBaseWillBeGarbageCollected<BluetoothSupplement>,
public WillBeHeapSupplement<LocalFrame> {
On 2015/07/16 22:27:14, Jeffrey Yasskin wrote:
> On 2015/07/16 01:03:03, scheib wrote:
> > Class comment please to increase readability for those learning the
> components.
> > Supplement is sadly missing overview doc explaining what it does, but even
> with
> > that we should explain here the concept that when running in a context with
a
> > frame, we obtain a WebBluetooth implementation from that frame that will be
> able
> > to show UI on that frame. It would simplify code understanding to have a
trace
> > of the construction flow that causes the supplement to exist there.
> 
> How's this?

Great, thank you.

[Jeffrey Yasskin, 2015-07-16 23:08:01]

The CQ bit was checked by jyasskin@chromium.org

[Jeffrey Yasskin, 2015-07-16 23:08:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/1228283003/#ps60001
(title: "Always use BluetoothSupplement to get the WebBluetooth instance.")

[commit-bot: I haz the power, 2015-07-16 23:08:29]

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

[commit-bot: I haz the power, 2015-07-17 00:29:57]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199067