Remove single observer assumption in WebUSBClient.

third_party/WebKit/Source/modules/webusb/USB.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 "modules/webusb/USB.h"
 
 #include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/DOMException.h"
@@ skipping 61 matching lines @@
     }
 };
 
 } // namespace
 
 USB::USB(LocalFrame& frame)
     : LocalFrameLifecycleObserver(&frame)
     , m_client(USBController::from(frame).client())
 {
     if (m_client)
-        m_client->setObserver(this);
+        m_client->addObserver(this);
 }
 
 USB::~USB()
 {
     if (m_client)
-        m_client->setObserver(nullptr);
+        m_client->removeObserver(this);

[haraken, 2016/03/30 23:35:23]

Is it possible that USB's destructor is called without
USB::willDetachFrameHost() getting called?

If that is possible, I think you need to make sure that
m_client->removeObserver(this) is called before GC's sweeping phase starts.
Otherwise, the following scenario can happen:

1) GC is triggered. USB becomes unreachable.
2) Lazy sweeping starts. It destructs some unreachable object. Assume that USB
isn't yet destructed.
3) m_client accesses the already destructed object.

To avoid that from happening, you need to make sure that
m_client->removeObserver(this) is called before the sweeping phase starts.
Specifically, you need to add USING_PRE_FINALIZER() and call
m_client->removeObserver(this) in the pre-finalizer.

[Reilly Grant (use Gerrit), 2016/03/30 23:38:10]

USB::willDetachFrameHost() doesn't appear to be being called at all. Do you know
why? It violates assumptions I had about LocalFrame lifetimes.
Okay.

[haraken, 2016/03/30 23:44:05]

It must be called. Is the USB object associated with a valid frame?

[Reilly Grant (use Gerrit), 2016/03/31 23:12:57]

I have tested again and confirmed that USB::willDetachFrameHost() is not called
before USB::~USB(). How would I know if the frame was valid? The
LocalFrameLifecycleObserver is initialized using the LocalFrame passed to
USB::USB() which is Navigator::frame() and guaranteed to be non-null.
Looking at this again I'm not sure I understand why a pre-finalizer is
necessary. Are you saying that USB::onDeviceAdded or USB::onDeviceRemoved may
touch an unreachable object because after USB is declared unreachable an object
it refers to may be destroyed but it itself may not be?

[haraken, 2016/03/31 23:48:16]

Maybe your USB object is destroyed before the frame gets detached? I think it
can happen and it's totally valid.

 }
 
 ScriptPromise USB::getDevices(ScriptState* scriptState)
 {
     if (!m_client)
         return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(NotSupportedError));
 
     String errorMessage;
     if (!scriptState->getExecutionContext()->isSecureContext(errorMessage))
         return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(SecurityError, errorMessage));
@@ skipping 33 matching lines @@
 }
 
 const AtomicString& USB::interfaceName() const
 {
     return EventTargetNames::USB;
 }
 
 void USB::willDetachFrameHost()
 {
     if (m_client)
-        m_client->setObserver(nullptr);
+        m_client->removeObserver(this);
     m_client = nullptr;
 }
 
 void USB::onDeviceConnected(WebPassOwnPtr<WebUSBDevice> device)
 {
     dispatchEvent(USBConnectionEvent::create(EventTypeNames::connect, USBDevice::create(device.release(), getExecutionContext())));
 }
 
 void USB::onDeviceDisconnected(WebPassOwnPtr<WebUSBDevice> device)
 {
     dispatchEvent(USBConnectionEvent::create(EventTypeNames::disconnect, USBDevice::create(device.release(), getExecutionContext())));
 }
 
 DEFINE_TRACE(USB)
 {
     RefCountedGarbageCollectedEventTargetWithInlineData<USB>::trace(visitor);
     LocalFrameLifecycleObserver::trace(visitor);
 }
 
 } // namespace blink