[ Mac WK1 ] REGRESSION (r251261): Layout Test inspector/console/webco…

…re-logging.html is consistently Failing https://bugs.webkit.org/show_bug.cgi?id=203173 <rdar://problem/56424721> Source/JavaScriptCore: Hold a strong reference to JSGlobalOjbect in ConsoleMessage so that object is not garbage collected before WebConsoleAgent::frameWindowDiscarded. Covered by existing test: inspector/console/webcore-logging.html. Reviewed by Geoffrey Garen. * inspector/ConsoleMessage.cpp: (Inspector::ConsoleMessage::ConsoleMessage): (Inspector::ConsoleMessage::clear): * inspector/ConsoleMessage.h: LayoutTests: Reviewed by Geoffrey Garen. play() returns a promise and the promise can be rejected by a later pause(). We didn't handle that case so we could receive a type JavaScript message for the unhandled rejected promise. * inspector/console/webcore-logging.html: * platform/mac-wk1/TestExpectations: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251482 268f45cc-cd09-0410-ab3c-d52691b4dbfc
yury-s · Oct 23, 2019 · 43e0e14 · 43e0e14
1 parent b6ab4b8
commit 43e0e14
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 6 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2019-10-23  Sihui Liu  <[email protected]>
+
+        [ Mac WK1 ] REGRESSION (r251261): Layout Test inspector/console/webcore-logging.html is consistently Failing
+        https://bugs.webkit.org/show_bug.cgi?id=203173
+        <rdar://problem/56424721>
+
+        Reviewed by Geoffrey Garen.
+
+        play() returns a promise and the promise can be rejected by a later pause(). We didn't handle
+        that case so we could receive a type JavaScript message for the unhandled rejected promise.
+
+        * inspector/console/webcore-logging.html:
+        * platform/mac-wk1/TestExpectations:
+
 2019-10-22  Simon Fraser  <[email protected]>
 
         wpt/css/css-images/gradient/color-stops-parsing.html fails

diff --git a/LayoutTests/inspector/console/webcore-logging.html b/LayoutTests/inspector/console/webcore-logging.html
@@ -17,7 +17,7 @@
 function play()
 {
     video.currentTime = 0;
-    video.play();
+    video.play().catch((err) => { });
     TestPage.dispatchEventToFrontend('PlayEvent', {count: 1});
 }
 

diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -801,7 +801,5 @@ webkit.org/b/200002 [ Mojave+ Debug ] imported/blink/storage/indexeddb/blob-basi
 # Skip IsLoggedIn
 http/tests/is-logged-in/ [ Skip ]
 
-webkit.org/b/203173 inspector/console/webcore-logging.html [ Pass Failure ]
-
 webkit.org/b/203176 [ Debug ] fast/scrolling/latching/scroll-select-bottom-test.html [ Pass Failure ]
 
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,21 @@
+2019-10-23  Sihui Liu  <[email protected]>
+
+        [ Mac WK1 ] REGRESSION (r251261): Layout Test inspector/console/webcore-logging.html is consistently Failing
+        https://bugs.webkit.org/show_bug.cgi?id=203173
+        <rdar://problem/56424721>
+
+        Hold a strong reference to JSGlobalOjbect in ConsoleMessage so that object is not garbage collected before
+        WebConsoleAgent::frameWindowDiscarded.
+
+        Covered by existing test: inspector/console/webcore-logging.html.
+
+        Reviewed by Geoffrey Garen.
+
+        * inspector/ConsoleMessage.cpp:
+        (Inspector::ConsoleMessage::ConsoleMessage):
+        (Inspector::ConsoleMessage::clear):
+        * inspector/ConsoleMessage.h:
+
 2019-10-22  Yusuke Suzuki  <[email protected]>
 
         Make `JSGlobalObject*` threading change more stabilized by adding tests and assertions

diff --git a/Source/JavaScriptCore/inspector/ConsoleMessage.cpp b/Source/JavaScriptCore/inspector/ConsoleMessage.cpp
@@ -117,9 +117,11 @@ ConsoleMessage::ConsoleMessage(MessageSource source, MessageType type, MessageLe
     , m_type(type)
     , m_level(level)
     , m_url()
-    , m_globalObject(globalObject)
     , m_requestId(IdentifiersFactory::requestId(requestIdentifier))
 {
+    if (globalObject)
+        m_globalObject = { globalObject->vm(), globalObject };
+
     if (!messages.size())
         return;
 
@@ -340,6 +342,9 @@ void ConsoleMessage::clear()
 
     if (m_arguments)
         m_arguments = nullptr;
+
+    if (m_globalObject)
+        m_globalObject.clear();
 }
 
 JSC::JSGlobalObject* ConsoleMessage::globalObject() const
@@ -348,7 +353,7 @@ JSC::JSGlobalObject* ConsoleMessage::globalObject() const
         return m_arguments->globalObject();
 
     if (m_globalObject)
-        return m_globalObject;
+        return m_globalObject.get();
 
     return nullptr;
 }

diff --git a/Source/JavaScriptCore/inspector/ConsoleMessage.h b/Source/JavaScriptCore/inspector/ConsoleMessage.h
@@ -31,6 +31,7 @@
 #pragma once
 
 #include "ConsoleTypes.h"
+#include "Strong.h"
 #include <wtf/FastMalloc.h>
 #include <wtf/Forward.h>
 #include <wtf/Logger.h>
@@ -94,7 +95,7 @@ class JS_EXPORT_PRIVATE ConsoleMessage {
     RefPtr<ScriptCallStack> m_callStack;
     Vector<JSONLogValue> m_jsonLogValues;
     String m_url;
-    JSC::JSGlobalObject* m_globalObject { nullptr };
+    JSC::Strong<JSC::JSGlobalObject> m_globalObject;
     unsigned m_line { 0 };
     unsigned m_column { 0 };
     unsigned m_repeatCount { 1 };