bluetooth: Change BluetoothSupplement::from(ScriptState*) to from(LocalFrame*)

third_party/WebKit/Source/modules/bluetooth/BluetoothSupplement.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "modules/bluetooth/BluetoothSupplement.h"
 
 #include "core/frame/LocalDOMWindow.h"
-#include "public/platform/Platform.h"
 
 namespace blink {
 
 const char* BluetoothSupplement::supplementName()
 {
     return "BluetoothSupplement";
 }
 
 BluetoothSupplement::BluetoothSupplement(WebBluetooth* bluetooth)
     : m_bluetooth(bluetooth)
 {
 }
 
 void BluetoothSupplement::provideTo(LocalFrame& frame, WebBluetooth* bluetooth)
 {
     OwnPtrWillBeRawPtr<BluetoothSupplement> bluetoothSupplement = adoptPtrWillBeNoop(new BluetoothSupplement(bluetooth));
     WillBeHeapSupplement<LocalFrame>::provideTo(frame, supplementName(), bluetoothSupplement.release());
 };
 
-WebBluetooth* BluetoothSupplement::from(ScriptState* scriptState)
+WebBluetooth* BluetoothSupplement::from(LocalFrame* frame)
+{
+    BluetoothSupplement* supplement = static_cast<BluetoothSupplement*>(WillBeHeapSupplement<LocalFrame>::from(frame, supplementName()));
+    if (supplement && supplement->m_bluetooth)

[haraken, 2015/10/10 14:09:57]

If the supplement is 0, don't you need to create a new BluetoothSupplement and
provide it to the frame?

c.f., A common programming pattern of Supplement::from is like this:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[ortuno, 2015/10/12 17:33:52]

We provide the Supplement to the frame here:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

So we should always find the Supplement. The other classes behind a flag in that
function follow the same pattern:


https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Changed the code to make it clearer.

+        return supplement->m_bluetooth;
+    return nullptr;
+}
+
+WebBluetooth* BluetoothSupplement::getFromScriptState(ScriptState* scriptState)

[haraken, 2015/10/10 14:09:57]

getFromScriptState => fromScriptState (Blink doesn't use a "get" prefix).

[ortuno, 2015/10/12 17:33:52]

Done.

 {
     LocalDOMWindow* window = scriptState->domWindow();
     if (window && window->frame()) {

[haraken, 2015/10/10 14:09:57]

It is unsafe to use window->frame() directly. You need to use 
toFrameIfNotDetached() in V8Binding.h.

[ortuno, 2015/10/12 17:33:52]

As suggested in the other CL I changed it to get the frame from the
executionContext.

Why do we need to check !executionContext->isDocument()?

[Jeffrey Yasskin, 2015/10/12 21:57:24]

Thanks Kentaro for catching my mistakes in writing this class. Was there any way
for me to discover that window->frame() was unsafe? e.g. neither
https://code.google.com/p/chromium/codesearch/#chromium/src/third_party/WebKi...
nor
https://code.google.com/p/chromium/codesearch/#chromium/src/third_party/WebKi...
has a comment saying anything's unsafe.
The static_cast<Document*>(executionContext) is only valid if executionContext
is a Document.

[haraken, 2015/10/12 23:29:25]

Sorry for not having a comment about it. We had to check
isCurrentlyDisplayedInFrame() before accessing window->frame() (c.f.,
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...),
but dcheng@ recently removed the restriction. (So actually it's now safe to
access window->frame().)

Sorry about the confusion. I'll clean up the code and add a good comment.

-        BluetoothSupplement* supplement = static_cast<BluetoothSupplement*>(WillBeHeapSupplement<LocalFrame>::from(window->frame(), supplementName()));
-        if (supplement && supplement->m_bluetooth)
-            return supplement->m_bluetooth;
+        return BluetoothSupplement::from(window->frame());
     }
-    return Platform::current()->bluetooth();
+    return nullptr;
 }
 
 DEFINE_TRACE(BluetoothSupplement)
 {
     WillBeHeapSupplement<LocalFrame>::trace(visitor);
 }
 
 }; // blink