Oilpan: cancel MIDI permission request when prefinalizing

Source/modules/webmidi/MIDIAccessInitializer.cpp

 // Copyright 2014 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/webmidi/MIDIAccessInitializer.h"
 
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "core/dom/DOMError.h"
 #include "core/dom/Document.h"
 #include "core/frame/Navigator.h"
 #include "modules/webmidi/MIDIAccess.h"
 #include "modules/webmidi/MIDIController.h"
 #include "modules/webmidi/MIDIOptions.h"
 #include "modules/webmidi/MIDIPort.h"
 
 namespace blink {
 
 MIDIAccessInitializer::MIDIAccessInitializer(ScriptState* scriptState, const MIDIOptions& options)
     : ScriptPromiseResolver(scriptState)
     , m_requestSysex(false)
 {
     if (options.hasSysex())
         m_requestSysex = options.sysex();
 }
 
 MIDIAccessInitializer::~MIDIAccessInitializer()
 {

[haraken, 2014/12/18 14:42:46]

What happens if MIDIAccessInitializer gets destructed before
ActiveDOMObject::stop() is called?

[sof, 2014/12/18 14:53:54]

A permissions request will be made with the controller as part of
MIDIAccessInitializer start()ing. At that point the ActiveDOMObject is in a
started or suspended state, right? I assume that it is not possible for this
ActiveDOMObject to be finalized before a stop() is issued by the execution
context.

[haraken, 2014/12/18 15:02:28]

I'm wondering if it's possible that the MIDIAccessInitializer object dies before
the ExecutionContext stops. For example:

(1) MIDIAccessInitializer is created.
(2) The last reference to the MIDIAccessInitializer is gone.
(3) GC collects the MIDIAccessInitializer.
(4) ExecutionContext stops.

?

[sof, 2014/12/18 15:17:57]

If that's a concern, I can rephrase this to be a reg/unreg protocol between the
controller and this object instead, when there is time. Using a controller-weak
reference, or something.

[haraken, 2014/12/18 15:44:02]

Yeah, I guess a more straightforward approach would be to have MIDIController
have a weak reference to the MIDIAccessInitializer so that the MIDIController
can call cancelSysexPermissionRequest() when the MIDIAccessInitializer is going
to be destructed.

In your approach, it's still not clear to me who keeps the MIDIAccessInitializer
object alive until the ExecutionContext stops.

-    // It is safe to cancel a request which is already finished or canceld.
+}
+
+void MIDIAccessInitializer::cancel()
+{
     Document* document = toDocument(executionContext());
     ASSERT(document);
     MIDIController* controller = MIDIController::from(document->frame());
     if (controller)
         controller->cancelSysexPermissionRequest(this);
 }
 
+void MIDIAccessInitializer::stop()
+{
+    // It is safe to cancel a request which is already finished or canceld.

[haraken, 2014/12/18 14:42:46]

canceled

+    cancel();
+    ScriptPromiseResolver::stop();
+}
+
 ScriptPromise MIDIAccessInitializer::start()
 {
     ScriptPromise promise = this->promise();
     m_accessor = MIDIAccessor::create(this);
 
     if (!m_requestSysex) {
         m_accessor->startSession();
         return promise;
     }
     Document* document = toDocument(executionContext());
@@ skipping 55 matching lines @@
 {
     return executionContext()->securityOrigin();
 }
 
 ExecutionContext* MIDIAccessInitializer::executionContext() const
 {
     return scriptState()->executionContext();
 }
 
 } // namespace blink