- Revision
- 284264
- Author
- [email protected]
- Date
- 2021-10-15 12:16:17 -0700 (Fri, 15 Oct 2021)
Log Message
[Cocoa] Web Inspector: handle Promise objects returned from evaluateScriptInExtensionTab
https://bugs.webkit.org/show_bug.cgi?id=231709
<rdar://problem/84224179>
Reviewed by Timothy Hatcher.
Source/WebCore:
New API test: WKInspectorExtension.EvaluateScriptInExtensionTabCanReturnPromises
* inspector/InspectorFrontendAPIDispatcher.cpp:
(WebCore::InspectorFrontendAPIDispatcher::evaluateOrQueueExpression):
Add JSLockHolders for when we are poking at the result value.
Source/WebInspectorUI:
This patch improves the handling of promises and exceptions that come from
InspectorFrontendHost.evaluateScriptInExtensionTab().
For promises, wait for the returned promise to settle before fulfilling our returned value.
For errors, perform a better stringification of the Error instance. The is needed because
the default toString implementation leaves out most of the relevant details of the error.
* UserInterface/Controllers/WebInspectorExtensionController.js:
(WI.WebInspectorExtensionController.prototype.evaluateScriptInExtensionTab):
* UserInterface/Protocol/InspectorFrontendAPI.js:
Fix incorrect header docs for InspectorFrontendAPI.evaluateScriptForExtension.
Copy the improved header doc for InspectorFrontendAPI.evaluateScriptInExtensionTab
since it now has the same calling convention.
Source/WebKit:
* WebProcess/Inspector/WebInspectorUIExtensionController.cpp:
(WebKit::WebInspectorUIExtensionController::evaluateScriptForExtension):
(WebKit::WebInspectorUIExtensionController::evaluateScriptInExtensionTab):
Add JSLockHolders for when we are poking at the result value.
Drive-by, fix a typo in logging code.
Tools:
Add a new test:
WKInspectorExtension.EvaluateScriptInExtensionTabCanReturnPromises
* TestWebKitAPI/Tests/WebKitCocoa/WKInspectorExtension.mm:
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (284263 => 284264)
--- trunk/Source/WebCore/ChangeLog 2021-10-15 19:14:53 UTC (rev 284263)
+++ trunk/Source/WebCore/ChangeLog 2021-10-15 19:16:17 UTC (rev 284264)
@@ -1,3 +1,17 @@
+2021-10-15 BJ Burg <[email protected]>
+
+ [Cocoa] Web Inspector: handle Promise objects returned from evaluateScriptInExtensionTab
+ https://bugs.webkit.org/show_bug.cgi?id=231709
+ <rdar://problem/84224179>
+
+ Reviewed by Timothy Hatcher.
+
+ New API test: WKInspectorExtension.EvaluateScriptInExtensionTabCanReturnPromises
+
+ * inspector/InspectorFrontendAPIDispatcher.cpp:
+ (WebCore::InspectorFrontendAPIDispatcher::evaluateOrQueueExpression):
+ Add JSLockHolders for when we are poking at the result value.
+
2021-10-15 Commit Queue <[email protected]>
Unreviewed, reverting r284244.
Modified: trunk/Source/WebCore/inspector/InspectorFrontendAPIDispatcher.cpp (284263 => 284264)
--- trunk/Source/WebCore/inspector/InspectorFrontendAPIDispatcher.cpp 2021-10-15 19:14:53 UTC (rev 284263)
+++ trunk/Source/WebCore/inspector/InspectorFrontendAPIDispatcher.cpp 2021-10-15 19:16:17 UTC (rev 284264)
@@ -176,7 +176,9 @@
optionalResultHandler(makeUnexpected(EvaluationError::ContextDestroyed));
return;
}
-
+
+ JSC::JSLockHolder lock(globalObject);
+
auto& vm = globalObject->vm();
auto* castedPromise = JSC::jsDynamicCast<JSC::JSPromise*>(vm, result.value());
if (!castedPromise) {
Modified: trunk/Source/WebInspectorUI/ChangeLog (284263 => 284264)
--- trunk/Source/WebInspectorUI/ChangeLog 2021-10-15 19:14:53 UTC (rev 284263)
+++ trunk/Source/WebInspectorUI/ChangeLog 2021-10-15 19:16:17 UTC (rev 284264)
@@ -1,3 +1,26 @@
+2021-10-15 BJ Burg <[email protected]>
+
+ [Cocoa] Web Inspector: handle Promise objects returned from evaluateScriptInExtensionTab
+ https://bugs.webkit.org/show_bug.cgi?id=231709
+ <rdar://problem/84224179>
+
+ Reviewed by Timothy Hatcher.
+
+ This patch improves the handling of promises and exceptions that come from
+ InspectorFrontendHost.evaluateScriptInExtensionTab().
+
+ For promises, wait for the returned promise to settle before fulfilling our returned value.
+ For errors, perform a better stringification of the Error instance. The is needed because
+ the default toString implementation leaves out most of the relevant details of the error.
+
+ * UserInterface/Controllers/WebInspectorExtensionController.js:
+ (WI.WebInspectorExtensionController.prototype.evaluateScriptInExtensionTab):
+
+ * UserInterface/Protocol/InspectorFrontendAPI.js:
+ Fix incorrect header docs for InspectorFrontendAPI.evaluateScriptForExtension.
+ Copy the improved header doc for InspectorFrontendAPI.evaluateScriptInExtensionTab
+ since it now has the same calling convention.
+
2021-10-14 Myles C. Maxfield <[email protected]>
All the SDKVariant.xcconfig files should match
Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController.js (284263 => 284264)
--- trunk/Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController.js 2021-10-15 19:14:53 UTC (rev 284263)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/WebInspectorExtensionController.js 2021-10-15 19:16:17 UTC (rev 284264)
@@ -243,11 +243,31 @@
return WI.WebInspectorExtension.ErrorCode.InvalidRequest;
}
- try {
- return {result: InspectorFrontendHost.evaluateScriptInExtensionTab(iframe, scriptSource)};
- } catch (error) {
- return {error: error.message};
- }
+ return new Promise((resolve, reject) => {
+ try {
+ // If `result` is a promise, then it came from a different frame and `instanceof Promise` won't work.
+ let result = InspectorFrontendHost.evaluateScriptInExtensionTab(iframe, scriptSource);
+ if (result?.then) {
+ result.then((resolvedValue) => resolve({result: resolvedValue}), (errorValue) => reject({error: errorValue}));
+ return;
+ }
+
+ resolve({result});
+ } catch (error) {
+ // Include more context in the stringification of the error.
+ const stackIndent = " ";
+ let stackLines = (error.stack?.split("\n") || []).map((line) => `${stackIndent}${line}`);
+ let formattedMessage = [
+ `Caught Exception: ${error.name}`,
+ `at ${error.sourceURL || "(unknown)"}:${error.line || 0}:${error.column || 0}:`,
+ error.message,
+ "",
+ "Backtrace:",
+ ...stackLines,
+ ].join("\n");
+ reject({error: formattedMessage});
+ }
+ });
}
// Private
Modified: trunk/Source/WebInspectorUI/UserInterface/Protocol/InspectorFrontendAPI.js (284263 => 284264)
--- trunk/Source/WebInspectorUI/UserInterface/Protocol/InspectorFrontendAPI.js 2021-10-15 19:14:53 UTC (rev 284263)
+++ trunk/Source/WebInspectorUI/UserInterface/Protocol/InspectorFrontendAPI.js 2021-10-15 19:16:17 UTC (rev 284264)
@@ -218,8 +218,10 @@
},
// Returns a string (WI.WebInspectorExtension.ErrorCode) if an error occurred that prevented evaluation.
- // Returns an object with a 'result' key and value that is the result of the script evaluation.
- // Returns an object with an 'error' key and value in the case that an exception was thrown.
+ // Returns a Promise that is resolved if the evaluation completes and rejected if there was an internal error.
+ // When the promise is fulfilled, it will be either:
+ // - resolved with an object containing a 'result' key and value that is the result of the script evaluation.
+ // - rejected with an object containing an 'error' key and value that is the exception that was thrown while evaluating script.
evaluateScriptForExtension(extensionID, scriptSource, {frameURL, contextSecurityOrigin, useContentScriptContext} = {})
{
return WI.sharedApp.extensionController.evaluateScriptForExtension(extensionID, scriptSource, {frameURL, contextSecurityOrigin, useContentScriptContext});
@@ -238,8 +240,10 @@
},
// Returns a string (WI.WebInspectorExtension.ErrorCode) if an error occurred that prevented evaluation.
- // Returns an object with a 'result' key and value that is the result of the script evaluation.
- // Returns an object with an 'error' key and value in the case that an exception was thrown.
+ // Returns a Promise that is resolved if the evaluation completes and rejected if there was an internal error.
+ // When the promise is fulfilled, it will be either:
+ // - resolved with an object containing a 'result' key and value that is the result of the script evaluation.
+ // - rejected with an object containing an 'error' key and value that is the exception that was thrown while evaluating script.
evaluateScriptInExtensionTab(extensionTabID, scriptSource)
{
return WI.sharedApp.extensionController.evaluateScriptInExtensionTab(extensionTabID, scriptSource);
Modified: trunk/Source/WebKit/ChangeLog (284263 => 284264)
--- trunk/Source/WebKit/ChangeLog 2021-10-15 19:14:53 UTC (rev 284263)
+++ trunk/Source/WebKit/ChangeLog 2021-10-15 19:16:17 UTC (rev 284264)
@@ -1,3 +1,17 @@
+2021-10-15 BJ Burg <[email protected]>
+
+ [Cocoa] Web Inspector: handle Promise objects returned from evaluateScriptInExtensionTab
+ https://bugs.webkit.org/show_bug.cgi?id=231709
+ <rdar://problem/84224179>
+
+ Reviewed by Timothy Hatcher.
+
+ * WebProcess/Inspector/WebInspectorUIExtensionController.cpp:
+ (WebKit::WebInspectorUIExtensionController::evaluateScriptForExtension):
+ (WebKit::WebInspectorUIExtensionController::evaluateScriptInExtensionTab):
+ Add JSLockHolders for when we are poking at the result value.
+ Drive-by, fix a typo in logging code.
+
2021-10-15 Wenson Hsieh <[email protected]>
REGRESSION (r284079): [Debug] IPCTestingAPI.CanReceiveSharedMemory is timing out
Modified: trunk/Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.cpp (284263 => 284264)
--- trunk/Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.cpp 2021-10-15 19:14:53 UTC (rev 284263)
+++ trunk/Source/WebKit/WebProcess/Inspector/WebInspectorUIExtensionController.cpp 2021-10-15 19:16:17 UTC (rev 284264)
@@ -245,6 +245,8 @@
completionHandler({ }, std::nullopt, Inspector::ExtensionError::ContextDestroyed);
return;
}
+
+ JSC::JSLockHolder lock(frontendGlobalObject);
if (auto parsedError = weakThis->parseExtensionErrorFromEvaluationResult(result)) {
auto exceptionDetails = result.value().error();
@@ -399,7 +401,7 @@
// Expected result is either an ErrorString or {result: <any>} or {error: string}.
auto objectResult = weakThis->unwrapEvaluationResultAsObject(result);
if (!objectResult) {
- LOG(Inspector, "Unexpected non-object value returned from InspectorFrontendAPI.createTabForExtension().");
+ LOG(Inspector, "Unexpected non-object value returned from InspectorFrontendAPI.evaluateScriptInExtensionTab().");
completionHandler({ }, std::nullopt, Inspector::ExtensionError::InternalError);
return;
}
Modified: trunk/Tools/ChangeLog (284263 => 284264)
--- trunk/Tools/ChangeLog 2021-10-15 19:14:53 UTC (rev 284263)
+++ trunk/Tools/ChangeLog 2021-10-15 19:16:17 UTC (rev 284264)
@@ -1,3 +1,17 @@
+2021-10-15 BJ Burg <[email protected]>
+
+ [Cocoa] Web Inspector: handle Promise objects returned from evaluateScriptInExtensionTab
+ https://bugs.webkit.org/show_bug.cgi?id=231709
+ <rdar://problem/84224179>
+
+ Reviewed by Timothy Hatcher.
+
+ Add a new test:
+ WKInspectorExtension.EvaluateScriptInExtensionTabCanReturnPromises
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WKInspectorExtension.mm:
+ (TEST):
+
2021-10-15 Jonathan Bedard <[email protected]>
[webkitscmpy] Refactor PR branch management (Follow-up fix)
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKInspectorExtension.mm (284263 => 284264)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKInspectorExtension.mm 2021-10-15 19:14:53 UTC (rev 284263)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKInspectorExtension.mm 2021-10-15 19:16:17 UTC (rev 284264)
@@ -306,4 +306,92 @@
TestWebKitAPI::Util::run(&pendingCallbackWasCalled);
}
+TEST(WKInspectorExtension, EvaluateScriptInExtensionTabCanReturnPromises)
+{
+ resetGlobalState();
+
+ auto webViewConfiguration = adoptNS([WKWebViewConfiguration new]);
+ webViewConfiguration.get().preferences._developerExtrasEnabled = YES;
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 600) configuration:webViewConfiguration.get()]);
+ auto uiDelegate = adoptNS([UIDelegateForTestingInspectorExtension new]);
+
+ [webView setUIDelegate:uiDelegate.get()];
+ [webView loadHTMLString:@"<head><title>Test page to be inspected</title></head><body><p>Filler content</p></body>" baseURL:[NSURL URLWithString:@"http://example.com/"]];
+
+ [[webView _inspector] show];
+ TestWebKitAPI::Util::run(&didAttachLocalInspectorCalled);
+
+ auto extensionID = [NSUUID UUID].UUIDString;
+ auto extensionDisplayName = @"ThirdExtension";
+
+ // Register the test extension.
+ pendingCallbackWasCalled = false;
+ [[webView _inspector] registerExtensionWithID:extensionID displayName:extensionDisplayName completionHandler:^(NSError * _Nullable error, _WKInspectorExtension * _Nullable extension) {
+ EXPECT_NULL(error);
+ EXPECT_NOT_NULL(extension);
+ sharedInspectorExtension = extension;
+
+ pendingCallbackWasCalled = true;
+ }];
+ TestWebKitAPI::Util::run(&pendingCallbackWasCalled);
+
+ auto extensionDelegate = adoptNS([InspectorExtensionDelegateForTestingInspectorExtension new]);
+ [sharedInspectorExtension setDelegate:extensionDelegate.get()];
+
+ // Create and show an extension tab.
+ auto iconURL = [NSURL URLWithString:@"test-resource://ThirdExtension/InspectorExtension-TabIcon-30x30.png"];
+ auto sourceURL = [NSURL URLWithString:@"test-resource://ThirdExtension/InspectorExtension-basic-tab.html"];
+
+ pendingCallbackWasCalled = false;
+ [sharedInspectorExtension createTabWithName:@"ThirdExtension-Tab" tabIconURL:iconURL sourceURL:sourceURL completionHandler:^(NSError * _Nullable error, NSString * _Nullable extensionTabIdentifier) {
+ EXPECT_NULL(error);
+ EXPECT_NOT_NULL(extensionTabIdentifier);
+ sharedExtensionTabIdentifier = extensionTabIdentifier;
+
+ pendingCallbackWasCalled = true;
+ }];
+ TestWebKitAPI::Util::run(&pendingCallbackWasCalled);
+
+ pendingCallbackWasCalled = false;
+ didShowExtensionTabWasCalled = false;
+ [[webView _inspector] showExtensionTabWithIdentifier:sharedExtensionTabIdentifier.get() completionHandler:^(NSError * _Nullable error) {
+ EXPECT_NULL(error);
+
+ pendingCallbackWasCalled = true;
+ }];
+ TestWebKitAPI::Util::run(&pendingCallbackWasCalled);
+ TestWebKitAPI::Util::run(&didShowExtensionTabWasCalled);
+
+ auto secretString = @"Open Sesame";
+ auto scriptSource = [NSString stringWithFormat:@"Promise.resolve(\"%@\")", secretString];
+
+ pendingCallbackWasCalled = false;
+ [sharedInspectorExtension _evaluateScript:scriptSource inExtensionTabWithIdentifier:sharedExtensionTabIdentifier.get() completionHandler:^(NSError * _Nullable error, NSDictionary * _Nullable result) {
+ EXPECT_NULL(error);
+ EXPECT_NOT_NULL(result);
+ EXPECT_NS_EQUAL(result, secretString);
+
+ pendingCallbackWasCalled = true;
+ }];
+ TestWebKitAPI::Util::run(&pendingCallbackWasCalled);
+
+ pendingCallbackWasCalled = false;
+ [sharedInspectorExtension _evaluateScript:@"(((" inExtensionTabWithIdentifier:sharedExtensionTabIdentifier.get() completionHandler:^(NSError * _Nullable error, NSDictionary * _Nullable result) {
+ EXPECT_NOT_NULL(error);
+ EXPECT_NULL(result);
+
+ pendingCallbackWasCalled = true;
+ }];
+ TestWebKitAPI::Util::run(&pendingCallbackWasCalled);
+
+ // Unregister the test extension.
+ pendingCallbackWasCalled = false;
+ [[webView _inspector] unregisterExtension:sharedInspectorExtension.get() completionHandler:^(NSError * _Nullable error) {
+ EXPECT_NULL(error);
+
+ pendingCallbackWasCalled = true;
+ }];
+ TestWebKitAPI::Util::run(&pendingCallbackWasCalled);
+}
+
#endif // ENABLE(INSPECTOR_EXTENSIONS)
