- Revision
- 260963
- Author
- drou...@apple.com
- Date
- 2020-04-30 13:46:06 -0700 (Thu, 30 Apr 2020)
Log Message
WebKit.WebContent process crashes when web developer tools are opened in Safari
https://bugs.webkit.org/show_bug.cgi?id=210794
<rdar://problem/62214651>
Reviewed by Brian Burg.
Source/_javascript_Core:
* inspector/InjectedScriptManager.cpp:
(Inspector::InjectedScriptManager::injectedScriptFor):
Don't crash if a `TerminatedExecutionError` is thrown.
* inspector/InjectedScriptBase.cpp:
(Inspector::InjectedScriptBase::makeCall):
Report the actual error message. Check that the result has a value before attempting to make
a `JSON::Value` out of it.
Source/WebCore:
Test: inspector/worker/dom-debugger-event-after-terminate-crash.html
* inspector/agents/InspectorDOMDebuggerAgent.cpp:
(WebCore::InspectorDOMDebuggerAgent::willHandleEvent):
Don't `ASSERT(injectedScript.hasNoValue())` as it's possible for the event to be fired after
`Worker.prototype.terminate`, in which case `InjectedScriptManager::injectedScriptFor` will
now return an `InjectedScript` that would fail that `ASSERT`.
Source/WebInspectorUI:
* UserInterface/Views/QuickConsole.js:
(WI.QuickConsole.prototype._handleFrameExecutionContextsCleared):
(WI.QuickConsole.prototype._handleTargetRemoved):
If a `Worker` is removed and is not the active execution context, still check to see if the
execution context picker should be hidden, such as if there is only one other execution
context (e.g. the main page execution context). Previously, the execution context picker
would only be hidden in this case if the `Worker` was the active execution context.
LayoutTests:
* inspector/worker/resources/worker-echo.js: Added.
* inspector/worker/dom-debugger-event-after-terminate-crash.html: Added.
* inspector/worker/dom-debugger-event-after-terminate-crash-expected.txt: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (260962 => 260963)
--- trunk/LayoutTests/ChangeLog 2020-04-30 20:03:26 UTC (rev 260962)
+++ trunk/LayoutTests/ChangeLog 2020-04-30 20:46:06 UTC (rev 260963)
@@ -1,3 +1,15 @@
+2020-04-30 Devin Rousso <drou...@apple.com>
+
+ WebKit.WebContent process crashes when web developer tools are opened in Safari
+ https://bugs.webkit.org/show_bug.cgi?id=210794
+ <rdar://problem/62214651>
+
+ Reviewed by Brian Burg.
+
+ * inspector/worker/resources/worker-echo.js: Added.
+ * inspector/worker/dom-debugger-event-after-terminate-crash.html: Added.
+ * inspector/worker/dom-debugger-event-after-terminate-crash-expected.txt: Added.
+
2020-04-30 Simon Fraser <simon.fra...@apple.com>
border-radius fails to clip iframe contents
Added: trunk/LayoutTests/inspector/worker/dom-debugger-event-after-terminate-crash-expected.txt (0 => 260963)
--- trunk/LayoutTests/inspector/worker/dom-debugger-event-after-terminate-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/inspector/worker/dom-debugger-event-after-terminate-crash-expected.txt 2020-04-30 20:46:06 UTC (rev 260963)
@@ -0,0 +1,8 @@
+Test to ensure that any queued messages are not dispatched after terminate is called, which would cause a crash due to the InjectedScript throwing a TerminatedExecutionError when it's evaluated.
+
+Creating worker...
+Terminating worker...
+Creating worker...
+Terminating worker...
+PASS: Should not crash.
+
Added: trunk/LayoutTests/inspector/worker/dom-debugger-event-after-terminate-crash.html (0 => 260963)
--- trunk/LayoutTests/inspector/worker/dom-debugger-event-after-terminate-crash.html (rev 0)
+++ trunk/LayoutTests/inspector/worker/dom-debugger-event-after-terminate-crash.html 2020-04-30 20:46:06 UTC (rev 260963)
@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script>
+let testWorker = null;
+
+function createWorker() {
+ testWorker = new Worker("resources/worker-echo.js");
+ testWorker.addEventListener("message", (event) => {
+ let {echo, data} = event.data;
+ console.assert(echo, data);
+ TestPage.dispatchEventToFrontend(data);
+ });
+ testWorker.postMessage("WorkerCreated");
+}
+
+function terminateWorker() {
+ testWorker.postMessage("WorkerTerminated");
+ testWorker.terminate();
+ testWorker = null;
+}
+
+function test()
+{
+ InspectorTest.addEventListener("WorkerTerminated", (event) => {
+ InspectorTest.fail("Should not dispatch any events after a Worker is terminated.");
+ });
+
+ Promise.resolve()
+ .then(() => {
+ InspectorTest.log("Creating worker...");
+ return Promise.all([
+ InspectorTest.awaitEvent("WorkerCreated"),
+ InspectorTest.evaluateInPage(`createWorker()`),
+ ]);
+ })
+ .then(() => {
+ InspectorTest.log("Terminating worker...");
+ return InspectorTest.evaluateInPage(`terminateWorker()`);
+ })
+ .then(() => {
+ InspectorTest.log("Creating worker...");
+ return Promise.all([
+ InspectorTest.awaitEvent("WorkerCreated"),
+ InspectorTest.evaluateInPage(`createWorker()`),
+ ]);
+ })
+ .then(() => {
+ InspectorTest.log("Terminating worker...");
+ return InspectorTest.evaluateInPage(`terminateWorker()`);
+ })
+ .then(() => {
+ InspectorTest.pass("Should not crash.");
+ })
+ .finally(() => {
+ InspectorTest.completeTest();
+ });
+}
+</script>
+</head>
+<body _onload_="runTest()">
+<p>Test to ensure that any queued messages are not dispatched after terminate is called, which would cause a crash due to the InjectedScript throwing a TerminatedExecutionError when it's evaluated.</p>
+</body>
+</html>
Added: trunk/LayoutTests/inspector/worker/resources/worker-echo.js (0 => 260963)
--- trunk/LayoutTests/inspector/worker/resources/worker-echo.js (rev 0)
+++ trunk/LayoutTests/inspector/worker/resources/worker-echo.js 2020-04-30 20:46:06 UTC (rev 260963)
@@ -0,0 +1,6 @@
+self.addEventListener("message", (event) => {
+ self.postMessage({
+ echo: true,
+ data: event.data,
+ });
+});
Modified: trunk/Source/_javascript_Core/ChangeLog (260962 => 260963)
--- trunk/Source/_javascript_Core/ChangeLog 2020-04-30 20:03:26 UTC (rev 260962)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-04-30 20:46:06 UTC (rev 260963)
@@ -1,3 +1,20 @@
+2020-04-30 Devin Rousso <drou...@apple.com>
+
+ WebKit.WebContent process crashes when web developer tools are opened in Safari
+ https://bugs.webkit.org/show_bug.cgi?id=210794
+ <rdar://problem/62214651>
+
+ Reviewed by Brian Burg.
+
+ * inspector/InjectedScriptManager.cpp:
+ (Inspector::InjectedScriptManager::injectedScriptFor):
+ Don't crash if a `TerminatedExecutionError` is thrown.
+
+ * inspector/InjectedScriptBase.cpp:
+ (Inspector::InjectedScriptBase::makeCall):
+ Report the actual error message. Check that the result has a value before attempting to make
+ a `JSON::Value` out of it.
+
2020-04-29 Ross Kirsling <ross.kirsl...@sony.com>
Ensure Intl classes don't have naming conflicts with unified builds
Modified: trunk/Source/_javascript_Core/inspector/InjectedScriptBase.cpp (260962 => 260963)
--- trunk/Source/_javascript_Core/inspector/InjectedScriptBase.cpp 2020-04-30 20:03:26 UTC (rev 260962)
+++ trunk/Source/_javascript_Core/inspector/InjectedScriptBase.cpp 2020-04-30 20:46:06 UTC (rev 260963)
@@ -81,11 +81,21 @@
if (hasNoValue() || !hasAccessToInspectedScriptState())
return JSON::Value::null();
+ auto globalObject = m_injectedScriptObject.globalObject();
+
auto result = callFunctionWithEvalEnabled(function);
- if (!result)
- return JSON::Value::create("Exception while making a call.");
+ if (!result) {
+ auto& error = result.error();
+ ASSERT(error);
- RefPtr<JSON::Value> resultJSONValue = toInspectorValue(m_injectedScriptObject.globalObject(), result.value());
+ return JSON::Value::create(error->value().toWTFString(globalObject));
+ }
+
+ auto value = result.value();
+ if (!value)
+ return JSON::Value::null();
+
+ auto resultJSONValue = toInspectorValue(globalObject, value);
if (!resultJSONValue)
return JSON::Value::create(makeString("Object has too long reference chain (must not be longer than ", JSON::Value::maxDepth, ')'));
Modified: trunk/Source/_javascript_Core/inspector/InjectedScriptManager.cpp (260962 => 260963)
--- trunk/Source/_javascript_Core/inspector/InjectedScriptManager.cpp 2020-04-30 20:03:26 UTC (rev 260962)
+++ trunk/Source/_javascript_Core/inspector/InjectedScriptManager.cpp 2020-04-30 20:46:06 UTC (rev 260963)
@@ -191,6 +191,9 @@
auto& error = createResult.error();
ASSERT(error);
+ if (isTerminatedExecutionException(globalObject->vm(), error))
+ return InjectedScript();
+
unsigned line = 0;
unsigned column = 0;
auto& stack = error->stack();
Modified: trunk/Source/WebCore/ChangeLog (260962 => 260963)
--- trunk/Source/WebCore/ChangeLog 2020-04-30 20:03:26 UTC (rev 260962)
+++ trunk/Source/WebCore/ChangeLog 2020-04-30 20:46:06 UTC (rev 260963)
@@ -1,3 +1,19 @@
+2020-04-30 Devin Rousso <drou...@apple.com>
+
+ WebKit.WebContent process crashes when web developer tools are opened in Safari
+ https://bugs.webkit.org/show_bug.cgi?id=210794
+ <rdar://problem/62214651>
+
+ Reviewed by Brian Burg.
+
+ Test: inspector/worker/dom-debugger-event-after-terminate-crash.html
+
+ * inspector/agents/InspectorDOMDebuggerAgent.cpp:
+ (WebCore::InspectorDOMDebuggerAgent::willHandleEvent):
+ Don't `ASSERT(injectedScript.hasNoValue())` as it's possible for the event to be fired after
+ `Worker.prototype.terminate`, in which case `InjectedScriptManager::injectedScriptFor` will
+ now return an `InjectedScript` that would fail that `ASSERT`.
+
2020-04-30 Alex Christensen <achristen...@webkit.org>
Add SPI to change a WKWebView's CORS disabling pattern after initialization
Modified: trunk/Source/WebCore/inspector/agents/InspectorDOMDebuggerAgent.cpp (260962 => 260963)
--- trunk/Source/WebCore/inspector/agents/InspectorDOMDebuggerAgent.cpp 2020-04-30 20:03:26 UTC (rev 260962)
+++ trunk/Source/WebCore/inspector/agents/InspectorDOMDebuggerAgent.cpp 2020-04-30 20:46:06 UTC (rev 260963)
@@ -216,7 +216,9 @@
auto state = event.target()->scriptExecutionContext()->execState();
auto injectedScript = m_injectedScriptManager.injectedScriptFor(state);
- ASSERT(!injectedScript.hasNoValue());
+ if (injectedScript.hasNoValue())
+ return;
+
{
JSC::JSLockHolder lock(state);
Modified: trunk/Source/WebInspectorUI/ChangeLog (260962 => 260963)
--- trunk/Source/WebInspectorUI/ChangeLog 2020-04-30 20:03:26 UTC (rev 260962)
+++ trunk/Source/WebInspectorUI/ChangeLog 2020-04-30 20:46:06 UTC (rev 260963)
@@ -1,3 +1,19 @@
+2020-04-30 Devin Rousso <drou...@apple.com>
+
+ WebKit.WebContent process crashes when web developer tools are opened in Safari
+ https://bugs.webkit.org/show_bug.cgi?id=210794
+ <rdar://problem/62214651>
+
+ Reviewed by Brian Burg.
+
+ * UserInterface/Views/QuickConsole.js:
+ (WI.QuickConsole.prototype._handleFrameExecutionContextsCleared):
+ (WI.QuickConsole.prototype._handleTargetRemoved):
+ If a `Worker` is removed and is not the active execution context, still check to see if the
+ execution context picker should be hidden, such as if there is only one other execution
+ context (e.g. the main page execution context). Previously, the execution context picker
+ would only be hidden in this case if the `Worker` was the active execution context.
+
2020-04-30 Christopher Reid <chris.r...@sony.com>
[CMake] Don't copy inspector resources in every incremental build
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/QuickConsole.js (260962 => 260963)
--- trunk/Source/WebInspectorUI/UserInterface/Views/QuickConsole.js 2020-04-30 20:03:26 UTC (rev 260962)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/QuickConsole.js 2020-04-30 20:46:06 UTC (rev 260963)
@@ -366,8 +366,10 @@
let {committingProvisionalLoad, contexts} = event.data;
let hasActiveExecutionContext = contexts.some((context) => context === WI.runtimeManager.activeExecutionContext);
- if (!hasActiveExecutionContext)
+ if (!hasActiveExecutionContext) {
+ this._updateActiveExecutionContextDisplay();
return;
+ }
// If this frame is navigating and it is selected in the UI we want to reselect its new item after navigation.
if (committingProvisionalLoad && !this._restoreSelectedExecutionContextForFrame) {
@@ -375,6 +377,8 @@
// As a fail safe, if the frame never gets an execution context, clear the restore value.
setTimeout(() => {
+ if (this._restoreSelectedExecutionContextForFrame)
+ this._updateActiveExecutionContextDisplay();
this._restoreSelectedExecutionContextForFrame = null;
}, 10);
return;
@@ -402,8 +406,10 @@
_handleTargetRemoved(event)
{
let {target} = event.data;
- if (target !== WI.runtimeManager.activeExecutionContext)
+ if (target !== WI.runtimeManager.activeExecutionContext) {
+ this._updateActiveExecutionContextDisplay();
return;
+ }
this._useExecutionContextOfInspectedNode = InspectorBackend.hasDomain("DOM");
this._setActiveExecutionContext(this._resolveDesiredActiveExecutionContext());
