Don't report promise rejection events during the microtask checkpoint

third_party/WebKit/Source/bindings/core/v8/RejectedPromises.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 "bindings/core/v8/RejectedPromises.h"
 
 #include "bindings/core/v8/ScopedPersistent.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "bindings/core/v8/ScriptValue.h"
 #include "bindings/core/v8/V8Binding.h"
 #include "core/dom/ExecutionContext.h"
 #include "core/events/EventTarget.h"
 #include "core/events/PromiseRejectionEvent.h"
 #include "core/inspector/ConsoleMessage.h"
 #include "core/inspector/ScriptArguments.h"
 #include "platform/RuntimeEnabledFeatures.h"
+#include "platform/Task.h"
+#include "public/platform/Platform.h"
+#include "public/platform/WebTaskRunner.h"
+#include "public/platform/WebThread.h"
+#include "wtf/Functional.h"
 
 namespace blink {
 
 static const unsigned maxReportedHandlersPendingResolution = 1000;
 
 class RejectedPromises::Message final : public NoBaseWillBeGarbageCollectedFinalized<RejectedPromises::Message> {
 public:
     static PassOwnPtrWillBeRawPtr<Message> create(ScriptState* scriptState, v8::Local<v8::Promise> promise, const ScriptValue& exception, const String& errorMessage, const String& resourceName, int scriptId, int lineNumber, int columnNumber, PassRefPtrWillBeRawPtr<ScriptCallStack> callStack)
     {
         return adoptPtrWillBeNoop(new Message(scriptState, promise, exception, errorMessage, resourceName, scriptId, lineNumber, columnNumber, callStack));
@@ skipping 89 matching lines @@
             target->dispatchEvent(event);
         }
 
         if (m_shouldLogToConsole) {
             RefPtrWillBeRawPtr<ConsoleMessage> consoleMessage = ConsoleMessage::create(JSMessageSource, RevokedErrorMessageLevel, "Handler added to rejected promise");
             consoleMessage->setRelatedMessageId(m_consoleMessageId);
             executionContext->addConsoleMessage(consoleMessage.release());
         }
     }
 
+    void makePromiseWeak()
+    {
+        ASSERT(!m_promise.isEmpty() && !m_promise.isWeak());
+        m_promise.setWeak(this, &Message::didCollectPromise);
+    }
+
+    void makePromiseStrong()
+    {
+        ASSERT(!m_promise.isEmpty() && m_promise.isWeak());
+        m_promise.clearWeak();
+    }
+
 private:
     Message(ScriptState* scriptState, v8::Local<v8::Promise> promise, const ScriptValue& exception, const String& errorMessage, const String& resourceName, int scriptId, int lineNumber, int columnNumber, PassRefPtrWillBeRawPtr<ScriptCallStack> callStack)
         : m_scriptState(scriptState)
         , m_promise(scriptState->isolate(), promise)
         , m_exception(exception)
         , m_errorMessage(errorMessage)
         , m_resourceName(resourceName)
         , m_scriptId(scriptId)
         , m_lineNumber(lineNumber)
         , m_columnNumber(columnNumber)
         , m_callStack(callStack)
         , m_consoleMessageId(0)
         , m_collected(false)
         , m_shouldLogToConsole(true)
     {
-        m_promise.setWeak(this, &Message::didCollectPromise);
     }
 
     static void didCollectPromise(const v8::WeakCallbackInfo<Message>& data)
     {
         data.GetParameter()->m_collected = true;
         data.GetParameter()->m_promise.clear();
     }
 
     ScriptState* m_scriptState;
     ScopedPersistent<v8::Promise> m_promise;
@@ skipping 33 matching lines @@
         if (!(*it)->isCollected() && (*it)->hasPromise(data.GetPromise())) {
             m_queue.remove(it);
             return;
         }
     }
 
     // Then look it up in the reported errors.
     for (size_t i = 0; i < m_reportedAsErrors.size(); ++i) {
         auto& message = m_reportedAsErrors.at(i);
         if (!message->isCollected() && message->hasPromise(data.GetPromise())) {
-            message->revoke();
+            message->makePromiseStrong();
+            Platform::current()->currentThread()->taskRunner()->postTask(FROM_HERE, new Task(bind(&RejectedPromises::revokeNow, this, message.release())));
             m_reportedAsErrors.remove(i);
             return;
         }
     }
 }
 
 void RejectedPromises::dispose()
 {
-    processQueue();
+    processQueueNow(m_queue);
 }
 
 void RejectedPromises::processQueue()
 {
+    if (m_queue.isEmpty())
+        return;
+    WillBeHeapDeque<OwnPtrWillBeMember<Message>> queue;
+    queue.swap(m_queue);
+    Platform::current()->currentThread()->taskRunner()->postTask(FROM_HERE, new Task(bind(&RejectedPromises::processQueueNow, this, queue)));

[jochen (gone - plz use gerrit), 2015/09/30 14:31:42]

hiroshige@

this line doesn't even compile, so I guess it's wrong :)

any suggestion what to do here?

[hiroshige, 2015/10/01 03:59:25]

WTF::bind()'s argument should be:
- Something copyable,
- PassOwnPtr<>,
- T* or RawPtr<T> if T is GCed,
- (or so)

Non-Oilpan:
Compilation fails because Deque<OwnPtr<Message>> is not copyable.
PassOwnPtr<Deque<OwnPtr<Message>>> can be used.

Oilpan (not so sure though):
HeapDeque<Member<Message>> is a GCed object, so
RawPtr<HeapDeque<Member<Message>>> should be used.

(Background info: This is because HeapDeque<Member<Message>> should be stored as
Persistent<HeapDeque<Member<Message>>> in PartBoundFunctionImpl, because
PartBoundFunctionImpl is not GCed object. WTF::bind() converts RawPtr<...> to
CrossThreadPersistent automatically via  ParamStorageTraits<RawPtr<T>> -- in
this case Persistent is sufficient but CrossThreadPersistent is used because the
conversion code can be used for cross-thread cases.)


So, OwnPtrWillBeRawPtr<WillBeHeapDeque<OwnPtrWillBeMember<Message>>> can be
used??
However,
- I'm still not sure how to move |m_queue| into
OwnPtrWillBeRawPtr<WillBeHeapDeque<OwnPtrWillBeMember<Message>>>.
- Quite long type name.

haraken@, could you double check for oilpan issues?

+}
+
+void RejectedPromises::processQueueNow(WillBeHeapDeque<OwnPtrWillBeMember<Message>>& queue)
+{
     // Remove collected handlers.
     for (size_t i = 0; i < m_reportedAsErrors.size();) {
         if (m_reportedAsErrors.at(i)->isCollected())
             m_reportedAsErrors.remove(i);
         else
             ++i;
     }
 
-    while (!m_queue.isEmpty()) {
-        OwnPtrWillBeRawPtr<Message> message = m_queue.takeFirst();
-        if (message->isCollected())
-            continue;
+    while (!queue.isEmpty()) {
+        OwnPtrWillBeRawPtr<Message> message = queue.takeFirst();
+        ASSERT(!message->isCollected());
 
         message->report();
+        message->makePromiseWeak();
         m_reportedAsErrors.append(message.release());
         if (m_reportedAsErrors.size() > maxReportedHandlersPendingResolution)
             m_reportedAsErrors.remove(0, maxReportedHandlersPendingResolution / 10);
     }
 }
 
+void RejectedPromises::revokeNow(PassOwnPtrWillBeRawPtr<Message> message)
+{
+    message->revoke();
+}
+
 } // namespace blink