Title: [225950] trunk
Revision
225950
Author
bb...@apple.com
Date
2017-12-14 17:39:11 -0800 (Thu, 14 Dec 2017)

Log Message

Web Inspector: InspectorTest.evaluateInPage should unwrap primitive values by default
https://bugs.webkit.org/show_bug.cgi?id=180831

Reviewed by Joseph Pecoraro.

Source/WebInspectorUI:

* UserInterface/Test/FrontendTestHarness.js:
(FrontendTestHarness.prototype.evaluateInPage.resultObjectToReturn):
(FrontendTestHarness.prototype.evaluateInPage):
Unwrap the resulting RemoteObject's value if it is a primitive.
Add an `options` dictionary so this behavior can be opted out.

* UserInterface/Test/TestHarness.js:
Add documentation of how evaluateInPage works.

LayoutTests:

* inspector/unit-tests/test-harness-evaluate-in-page-expected.txt: Added.
* inspector/unit-tests/test-harness-evaluate-in-page.html: Added.

Add test coverage for InspectorTest.evaluateInPage. Only the promise-returning
variant is tested here, because the callback variant is well-used in existing
tests and I plan to remove the callback variant entirely in later patches.

* inspector/console/js-isLikelyStackTrace-expected.txt:
* inspector/console/js-isLikelyStackTrace.html:
Fix some bad tests and rebaseline. A bug was filed for the remaining failing assertion.

* http/tests/inspector/dom/shapes-test.js:
(TestPage.registerInitializer.InspectorTest.Shapes.receivedHighlightObject):
(TestPage.registerInitializer.InspectorTest.Shapes.getShapeOutsideInfoForSelector):
* http/tests/inspector/paymentrequest/payment-request-internal-properties.https.html:
* inspector/console/command-line-api-copy.html:
* inspector/console/console-log-proxy.html:
* inspector/debugger/js-stacktrace.html:
* inspector/dom/hideHighlight.html:
* inspector/dom/highlightFrame.html:
* inspector/dom/highlightNode.html:
* inspector/dom/highlightNodeList.html:
* inspector/dom/highlightQuad.html:
* inspector/dom/highlightRect.html:
* inspector/dom/highlightSelector.html:
* inspector/page/setEmulatedMedia.html:
* inspector/runtime/getPreview.html:
Fix existing tests to opt out of unwrapping or use the unwrapped value directly.

* inspector/dom/setEventListenerDisabled-expected.txt:
Rebaseline results. This is caused by an extra promise tick in evaluateInPage.

* inspector/injected-script/observable-expected.txt:
* inspector/injected-script/observable.html:
Fix a typo and rebaseline.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (225949 => 225950)


--- trunk/LayoutTests/ChangeLog	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/ChangeLog	2017-12-15 01:39:11 UTC (rev 225950)
@@ -1,3 +1,47 @@
+2017-12-14  Brian Burg  <bb...@apple.com>
+
+        Web Inspector: InspectorTest.evaluateInPage should unwrap primitive values by default
+        https://bugs.webkit.org/show_bug.cgi?id=180831
+
+        Reviewed by Joseph Pecoraro.
+
+        * inspector/unit-tests/test-harness-evaluate-in-page-expected.txt: Added.
+        * inspector/unit-tests/test-harness-evaluate-in-page.html: Added.
+
+        Add test coverage for InspectorTest.evaluateInPage. Only the promise-returning
+        variant is tested here, because the callback variant is well-used in existing
+        tests and I plan to remove the callback variant entirely in later patches.
+
+        * inspector/console/js-isLikelyStackTrace-expected.txt:
+        * inspector/console/js-isLikelyStackTrace.html:
+        Fix some bad tests and rebaseline. A bug was filed for the remaining failing assertion.
+
+        * http/tests/inspector/dom/shapes-test.js:
+        (TestPage.registerInitializer.InspectorTest.Shapes.receivedHighlightObject):
+        (TestPage.registerInitializer.InspectorTest.Shapes.getShapeOutsideInfoForSelector):
+        * http/tests/inspector/paymentrequest/payment-request-internal-properties.https.html:
+        * inspector/console/command-line-api-copy.html:
+        * inspector/console/console-log-proxy.html:
+        * inspector/debugger/js-stacktrace.html:
+        * inspector/dom/hideHighlight.html:
+        * inspector/dom/highlightFrame.html:
+        * inspector/dom/highlightNode.html:
+        * inspector/dom/highlightNodeList.html:
+        * inspector/dom/highlightQuad.html:
+        * inspector/dom/highlightRect.html:
+        * inspector/dom/highlightSelector.html:
+        * inspector/page/setEmulatedMedia.html:
+        * inspector/runtime/getPreview.html:
+        Fix existing tests to opt out of unwrapping or use the unwrapped value directly.
+
+        * inspector/dom/setEventListenerDisabled-expected.txt:
+        Rebaseline results. This is caused by an extra promise tick in evaluateInPage.
+
+        * inspector/injected-script/observable-expected.txt:
+        * inspector/injected-script/observable.html:
+        Fix a typo and rebaseline.
+
+
 2017-12-14  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, rebaseline compositing/repaint/iframes/composited-iframe-with-fixed-background-doc-repaint.html after r225897.

Modified: trunk/LayoutTests/http/tests/inspector/dom/shapes-test.js (225949 => 225950)


--- trunk/LayoutTests/http/tests/inspector/dom/shapes-test.js	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/http/tests/inspector/dom/shapes-test.js	2017-12-15 01:39:11 UTC (rev 225950)
@@ -30,11 +30,11 @@
             InspectorTest.evaluateInPage("window.internals.inspectorHighlightObject()", receivedHighlightObject);
         }
 
-        function receivedHighlightObject(error, payload, wasThrown) {
+        function receivedHighlightObject(error, value, wasThrown) {
             InspectorTest.assert(!error, "When evaluating code, received unexpected error:" + error);
             InspectorTest.assert(!error, "When evaluating code, an exception was thrown:" + wasThrown);
 
-            var data = ""
+            var data = ""
             callback(data[0].elementData.shapeOutsideData);
         }
     },

Modified: trunk/LayoutTests/http/tests/inspector/paymentrequest/payment-request-internal-properties.https.html (225949 => 225950)


--- trunk/LayoutTests/http/tests/inspector/paymentrequest/payment-request-internal-properties.https.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/http/tests/inspector/paymentrequest/payment-request-internal-properties.https.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -133,11 +133,10 @@
         suite.addTestCase({
             name, description,
             async test() {
-                let [result] = await Promise.all([
-                    InspectorTest.evaluateInPage(_expression_),
+                let [remoteObject, _] = await Promise.all([
+                    InspectorTest.evaluateInPage(_expression_, null, {remoteObjectOnly: true}),
                     InspectorTest.awaitEvent("Continue"),
                 ]);
-                let remoteObject = WI.RemoteObject.fromPayload(result.result);
                 let [propertyDescriptors] = await promisify((cb) => { remoteObject.getAllPropertyDescriptors(cb); });
                 for (let propertyDescriptor of propertyDescriptors.reverse()) {
                     if (propertyDescriptor.isInternalProperty) {

Modified: trunk/LayoutTests/inspector/console/command-line-api-copy.html (225949 => 225950)


--- trunk/LayoutTests/inspector/console/command-line-api-copy.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/console/command-line-api-copy.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -24,10 +24,9 @@
     function commandLineAPICopyAndPaste(_expression_, callback) {
         InspectorTest.assert(typeof _expression_ === "string", "Test requires string _expression_ to evaluate on the page.");
         WI.runtimeManager.evaluateInInspectedWindow(`copy(${_expression_})`, {objectGroup: "test", includeCommandLineAPI: true}, () => {
-            InspectorTest.evaluateInPage("pasteAndReturnString()", (error, remoteObjectPayload) => {
-                let remoteObject = WI.RemoteObject.fromPayload(remoteObjectPayload);
-                InspectorTest.assert(remoteObject.type === "string", "RemoteObject from pasteAndReturnString should return a string.");
-                callback(remoteObject.value);
+            InspectorTest.evaluateInPage(`pasteAndReturnString()`, (error, result) => {
+                InspectorTest.assert(typeof result === "string", "Result from pasteAndReturnString should be a string.")
+                callback(result);
             });
         });
     }

Modified: trunk/LayoutTests/inspector/console/console-log-proxy.html (225949 => 225950)


--- trunk/LayoutTests/inspector/console/console-log-proxy.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/console/console-log-proxy.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -42,8 +42,7 @@
         test(resolve, reject) {
             InspectorTest.evaluateInPage("triggerProxyConsoleLog()", () => {
                 InspectorTest.evaluateInPage("window.accessedHandlerGet", (error, result) => {
-                    let value = WI.RemoteObject.fromPayload(result).value;
-                    InspectorTest.expectEqual(value, false, "Logging Proxy objects should not have triggered get trap.");
+                    InspectorTest.expectFalse(result, "Logging Proxy objects should not have triggered get trap.");
                     resolve();
                 });
             });
@@ -56,8 +55,7 @@
         test(resolve, reject) {
             InspectorTest.evaluateInPage("triggerProxyAndPrimitiveConsoleLog()", () => {
                 InspectorTest.evaluateInPage("window.accessedHandlerGet", (error, result) => {
-                    let value = WI.RemoteObject.fromPayload(result).value;
-                    InspectorTest.expectEqual(value, false, "Logging Proxy objects and primitives should not have triggered get trap.");
+                    InspectorTest.expectFalse(result, "Logging Proxy objects and primitives should not have triggered get trap.");
                     resolve();
                 });
             });

Modified: trunk/LayoutTests/inspector/console/js-isLikelyStackTrace-expected.txt (225949 => 225950)


--- trunk/LayoutTests/inspector/console/js-isLikelyStackTrace-expected.txt	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/console/js-isLikelyStackTrace-expected.txt	2017-12-15 01:39:11 UTC (rev 225950)
@@ -2,14 +2,18 @@
 
 == Running test suite: WI.StackTrace.isLikelyStackTrace
 -- Running test case: notStacktrace
-ASSERT: Should NOT be a stacktrace.
+PASS: Should NOT be a stacktrace.
 
 -- Running test case: typeErrorWrap
-ASSERT: Should be a stacktrace.
+FAIL: Should be a stacktrace.
+    Expected: truthy
+    Actual: false
 
 -- Running test case: getAnonymousFunctionError
-ASSERT: Should be a stacktrace.
+PASS: Should be a stacktrace.
 
 -- Running test case: testWithNativeCallInBetween
-ASSERT: Should be a stacktrace.
+FAIL: Should be a stacktrace.
+    Expected: truthy
+    Actual: false
 

Modified: trunk/LayoutTests/inspector/console/js-isLikelyStackTrace.html (225949 => 225950)


--- trunk/LayoutTests/inspector/console/js-isLikelyStackTrace.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/console/js-isLikelyStackTrace.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -46,8 +46,8 @@
     suite.addTestCase({
         name: "notStacktrace",
         test(resolve, reject) {
-            var result = WI.StackTrace.isLikelyStackTrace("Messages:42\nUnread:16");
-            InspectorTest.assert(result, "Should NOT be a stacktrace.");
+            let result = WI.StackTrace.isLikelyStackTrace("Messages:42\nUnread:16");
+            InspectorTest.expectFalse(result, "Should NOT be a stacktrace.");
             resolve();
         }
     });
@@ -59,7 +59,7 @@
                 if (error)
                     reject(error);
 
-                InspectorTest.assert(WI.StackTrace.isLikelyStackTrace(result), "Should be a stacktrace.");
+                InspectorTest.expectThat(WI.StackTrace.isLikelyStackTrace(result), "Should be a stacktrace.");
 
                 resolve();
             });
@@ -73,7 +73,8 @@
                 if (error)
                     reject(error);
 
-                InspectorTest.assert(WI.StackTrace.isLikelyStackTrace(result), "Should be a stacktrace.");
+                // FIXME: this test case fails. <https://bugs.webkit.org/show_bug.cgi?id=180664>
+                InspectorTest.expectThat(WI.StackTrace.isLikelyStackTrace(result), "Should be a stacktrace.");
 
                 resolve();
             });
@@ -87,7 +88,7 @@
                 if (error)
                     reject(error);
 
-                InspectorTest.assert(WI.StackTrace.isLikelyStackTrace(result), "Should be a stacktrace.");
+                InspectorTest.expectThat(WI.StackTrace.isLikelyStackTrace(result), "Should be a stacktrace.");
 
                 resolve();
             });

Modified: trunk/LayoutTests/inspector/debugger/js-stacktrace.html (225949 => 225950)


--- trunk/LayoutTests/inspector/debugger/js-stacktrace.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/debugger/js-stacktrace.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -106,7 +106,7 @@
                 InspectorTest.evaluateInPage(_expression_, (error, result) => {
                     InspectorTest.assert(!error, error);
                     InspectorTest.log("\nError object:");
-                    let stackTrace = stripFilePaths(stripPayloadAfterGlobalCode(WI.StackTrace._parseStackTrace(result.value)));
+                    let stackTrace = stripFilePaths(stripPayloadAfterGlobalCode(WI.StackTrace._parseStackTrace(result)));
                     InspectorTest.log(JSON.stringify(stackTrace, null, 4));
                     resolve();
                 });

Modified: trunk/LayoutTests/inspector/dom/hideHighlight.html (225949 => 225950)


--- trunk/LayoutTests/inspector/dom/hideHighlight.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/dom/hideHighlight.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -9,10 +9,10 @@
     const outlineColor = undefined;
 
     function getHighlightRects(callback) {
-        InspectorTest.evaluateInPage("JSON.stringify(Array.from(window.internals.inspectorHighlightRects()))", (error, payload, wasThrown) => {
+        InspectorTest.evaluateInPage("JSON.stringify(Array.from(window.internals.inspectorHighlightRects()))", (error, value, wasThrown) => {
             InspectorTest.assert(!error, "Unexpected error dumping highlight: " + error);
             InspectorTest.assert(!wasThrown, "Unexpected exception when dumping highlight.");
-            callback(JSON.parse(payload.value));
+            callback(JSON.parse(value));
         });
     }
 
@@ -25,10 +25,10 @@
     }
 
     function getHighlight(callback) {
-        InspectorTest.evaluateInPage("window.internals.inspectorHighlightObject()", (error, payload, wasThrown) => {
+        InspectorTest.evaluateInPage("window.internals.inspectorHighlightObject()", (error, value, wasThrown) => {
             InspectorTest.assert(!error, "Unexpected error dumping highlight: " + error);
             InspectorTest.assert(!wasThrown, "Unexpected exception when dumping highlight.");
-            callback(JSON.parse(payload.value));
+            callback(JSON.parse(value));
         });
     }
 

Modified: trunk/LayoutTests/inspector/dom/highlightFrame.html (225949 => 225950)


--- trunk/LayoutTests/inspector/dom/highlightFrame.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/dom/highlightFrame.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -12,10 +12,10 @@
     InspectorTest.expectEqual(childFrames.length, 2, "Page should have subframes.");
 
     function getHighlight(callback) {
-        InspectorTest.evaluateInPage("window.internals.inspectorHighlightObject()", (error, payload, wasThrown) => {
+        InspectorTest.evaluateInPage("window.internals.inspectorHighlightObject()", (error, value, wasThrown) => {
             InspectorTest.assert(!error, "Unexpected error dumping highlight: " + error);
             InspectorTest.assert(!wasThrown, "Unexpected exception when dumping highlight.");
-            callback(JSON.parse(payload.value));
+            callback(JSON.parse(value));
         });
     }
 

Modified: trunk/LayoutTests/inspector/dom/highlightNode.html (225949 => 225950)


--- trunk/LayoutTests/inspector/dom/highlightNode.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/dom/highlightNode.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -20,10 +20,10 @@
     };
 
     function getHighlight(callback) {
-        InspectorTest.evaluateInPage("window.internals.inspectorHighlightObject()", (error, payload, wasThrown) => {
+        InspectorTest.evaluateInPage("window.internals.inspectorHighlightObject()", (error, value, wasThrown) => {
             InspectorTest.assert(!error, "Unexpected error dumping highlight: " + error);
             InspectorTest.assert(!wasThrown, "Unexpected exception when dumping highlight.");
-            callback(JSON.parse(payload.value));
+            callback(JSON.parse(value));
         });
     }
 

Modified: trunk/LayoutTests/inspector/dom/highlightNodeList.html (225949 => 225950)


--- trunk/LayoutTests/inspector/dom/highlightNodeList.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/dom/highlightNodeList.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -20,10 +20,10 @@
     };
 
     function getHighlight(callback) {
-        InspectorTest.evaluateInPage("window.internals.inspectorHighlightObject()", (error, payload, wasThrown) => {
+        InspectorTest.evaluateInPage("window.internals.inspectorHighlightObject()", (error, value, wasThrown) => {
             InspectorTest.assert(!error, "Unexpected error dumping highlight: " + error);
             InspectorTest.assert(!wasThrown, "Unexpected exception when dumping highlight.");
-            callback(JSON.parse(payload.value));
+            callback(JSON.parse(value));
         });
     }
 

Modified: trunk/LayoutTests/inspector/dom/highlightQuad.html (225949 => 225950)


--- trunk/LayoutTests/inspector/dom/highlightQuad.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/dom/highlightQuad.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -11,10 +11,10 @@
     const usePageCoordinates = true;
 
     function getHighlightRects(callback) {
-        InspectorTest.evaluateInPage("JSON.stringify(Array.from(window.internals.inspectorHighlightRects()))", (error, payload, wasThrown) => {
+        InspectorTest.evaluateInPage("JSON.stringify(Array.from(window.internals.inspectorHighlightRects()))", (error, value, wasThrown) => {
             InspectorTest.assert(!error, "Unexpected error dumping highlight: " + error);
             InspectorTest.assert(!wasThrown, "Unexpected exception when dumping highlight.");
-            callback(JSON.parse(payload.value));
+            callback(JSON.parse(value));
         });
     }
 

Modified: trunk/LayoutTests/inspector/dom/highlightRect.html (225949 => 225950)


--- trunk/LayoutTests/inspector/dom/highlightRect.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/dom/highlightRect.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -11,10 +11,10 @@
     const usePageCoordinates = true;
 
     function getHighlightRects(callback) {
-        InspectorTest.evaluateInPage("JSON.stringify(Array.from(window.internals.inspectorHighlightRects()))", (error, payload, wasThrown) => {
+        InspectorTest.evaluateInPage("JSON.stringify(Array.from(window.internals.inspectorHighlightRects()))", (error, value, wasThrown) => {
             InspectorTest.assert(!error, "Unexpected error dumping highlight: " + error);
             InspectorTest.assert(!wasThrown, "Unexpected exception when dumping highlight.");
-            callback(JSON.parse(payload.value));
+            callback(JSON.parse(value));
         });
     }
 

Modified: trunk/LayoutTests/inspector/dom/highlightSelector.html (225949 => 225950)


--- trunk/LayoutTests/inspector/dom/highlightSelector.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/dom/highlightSelector.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -101,10 +101,10 @@
     }
 
     function getHighlight(callback) {
-        InspectorTest.evaluateInPage("window.internals.inspectorHighlightObject()", (error, payload, wasThrown) => {
+        InspectorTest.evaluateInPage("window.internals.inspectorHighlightObject()", (error, value, wasThrown) => {
             InspectorTest.assert(!error, "Unexpected error dumping highlight: " + error);
             InspectorTest.assert(!wasThrown, "Unexpected exception when dumping highlight.");
-            callback(JSON.parse(payload.value));
+            callback(JSON.parse(value));
         });
     }
 

Modified: trunk/LayoutTests/inspector/dom/setEventListenerDisabled-expected.txt (225949 => 225950)


--- trunk/LayoutTests/inspector/dom/setEventListenerDisabled-expected.txt	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/dom/setEventListenerDisabled-expected.txt	2017-12-15 01:39:11 UTC (rev 225950)
@@ -5,15 +5,15 @@
 -- Running test case: DOM.setEventListenerDisabled.DisabledClickEvent
 Click event listener is enabled.
 Disabling event listener...
+PASS: Click event listener did not fire.
 <body> clicked.
-PASS: Click event listener did not fire.
 Click event listener is disabled.
 
 -- Running test case: DOM.setEventListenerDisabled.ReenabledClickEvent
 Click event listener is disabled.
 Enabling event listener...
+PASS: Click event listener fired.
 <body> clicked.
-PASS: Click event listener fired.
 Click event listener is enabled.
 
 -- Running test case: DOM.setEventListenerDisabled.Invalid

Modified: trunk/LayoutTests/inspector/injected-script/observable-expected.txt (225949 => 225950)


--- trunk/LayoutTests/inspector/injected-script/observable-expected.txt	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/injected-script/observable-expected.txt	2017-12-15 01:39:11 UTC (rev 225950)
@@ -6,5 +6,5 @@
 -- Running test case: InjectedScript.Observable:Array.prototype[Symbol.iterator]
 PASS: Paused.
 PASS: Resumed.
-PASS: 1
+PASS: Array.prototype[Symbol.iterator] call count should be 1.
 

Modified: trunk/LayoutTests/inspector/injected-script/observable.html (225949 => 225950)


--- trunk/LayoutTests/inspector/injected-script/observable.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/injected-script/observable.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -41,7 +41,7 @@
         description: "Array iteration in Injected Script should not be observable",
         test(resolve, reject) {
             InspectorTest.evaluateInPage(`triggerIteration()`, (error, result) => {
-                InspectorTest.expectThat(result.value, 1, "Array.prototype[Symbol.iterator] call count should be 1.");
+                InspectorTest.expectEqual(result, 1, "Array.prototype[Symbol.iterator] call count should be 1.");
                 resolve();
             });
 

Modified: trunk/LayoutTests/inspector/page/setEmulatedMedia.html (225949 => 225950)


--- trunk/LayoutTests/inspector/page/setEmulatedMedia.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/page/setEmulatedMedia.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -23,10 +23,10 @@
         description: "Initial media type should match screen.",
         test(resolve, reject) {
             InspectorTest.evaluateInPage("mediaQueryList.matches", (error, result) => {
-                InspectorTest.expectEqual(result.value, false, "Page should not match print media.");
+                InspectorTest.expectEqual(result, false, "Page should not match print media.");
             });
             InspectorTest.evaluateInPage("getComputedStyle(document.body).color", (error, result) => {
-                InspectorTest.expectEqual(result.value, "rgb(0, 0, 255)", "Body color should be blue.");
+                InspectorTest.expectEqual(result, "rgb(0, 0, 255)", "Body color should be blue.");
                 resolve();
             });
         }
@@ -38,10 +38,10 @@
         test(resolve, reject) {
             PageAgent.setEmulatedMedia("print", (error) => {
                 InspectorTest.evaluateInPage("mediaQueryList.matches", (error, result) => {
-                    InspectorTest.expectEqual(result.value, true, "Page should now match print media.");
+                    InspectorTest.expectEqual(result, true, "Page should now match print media.");
                 });
                 InspectorTest.evaluateInPage("getComputedStyle(document.body).color", (error, result) => {
-                    InspectorTest.expectEqual(result.value, "rgb(0, 128, 0)", "Body color should be green.");
+                    InspectorTest.expectEqual(result, "rgb(0, 128, 0)", "Body color should be green.");
                     resolve();
                 });
             });
@@ -54,10 +54,10 @@
         test(resolve, reject) {
             PageAgent.setEmulatedMedia("", (error) => {
                 InspectorTest.evaluateInPage("mediaQueryList.matches", (error, result) => {
-                    InspectorTest.expectEqual(result.value, false, "Page should now not match print media.");
+                    InspectorTest.expectEqual(result, false, "Page should now not match print media.");
                 });
                 InspectorTest.evaluateInPage("getComputedStyle(document.body).color", (error, result) => {
-                    InspectorTest.expectEqual(result.value, "rgb(0, 0, 255)", "Page should now match print media.");
+                    InspectorTest.expectEqual(result, "rgb(0, 0, 255)", "Page should now match print media.");
                     resolve();
                 });
             });

Modified: trunk/LayoutTests/inspector/runtime/getPreview.html (225949 => 225950)


--- trunk/LayoutTests/inspector/runtime/getPreview.html	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/LayoutTests/inspector/runtime/getPreview.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -74,8 +74,7 @@
         name: "RemoteObject.updatePreview.Symbol",
         description: "Non-object RemoteObjects should not request a preview.",
         test(resolve, reject) {
-            InspectorTest.evaluateInPage(`Symbol.iterator`, (error, remoteObjectPayload) => {
-                let remoteObject = WI.RemoteObject.fromPayload(remoteObjectPayload);
+            InspectorTest.evaluateInPage(`Symbol.iterator`, (error, remoteObject) => {
                 InspectorTest.expectThat(!remoteObject.canLoadPreview(), `Should not be able to load a preview for a ${remoteObject.type} RemoteObject.`);
                 remoteObject.updatePreview((preview) => {
                     InspectorTest.expectNull(preview, `RemoteObject.updatePreview should return null for a ${remoteObject.type} RemoteObject.`);
@@ -89,8 +88,7 @@
         name: "RemoteObject.updatePreview.Function",
         description: "Non-object RemoteObjects should not request a preview.",
         test(resolve, reject) {
-            InspectorTest.evaluateInPage(`function f() {}; f`, (error, remoteObjectPayload) => {
-                let remoteObject = WI.RemoteObject.fromPayload(remoteObjectPayload);
+            InspectorTest.evaluateInPage(`function f() {}; f`, (error, remoteObject) => {
                 InspectorTest.expectThat(!remoteObject.canLoadPreview(), `Should not be able to load a preview for a ${remoteObject.type} RemoteObject.`);
                 remoteObject.updatePreview((preview) => {
                     InspectorTest.expectNull(preview, `RemoteObject.updatePreview should return null for a ${remoteObject.type} RemoteObject.`);
@@ -104,14 +102,13 @@
         name: "RemoteObject.updatePreview.Null",
         description: "Non-object RemoteObjects should not request a preview.",
         test(resolve, reject) {
-            InspectorTest.evaluateInPage(`null`, (error, remoteObjectPayload) => {
-                let remoteObject = WI.RemoteObject.fromPayload(remoteObjectPayload);
+            InspectorTest.evaluateInPage(`null`, (error, remoteObject) => {
                 InspectorTest.expectThat(!remoteObject.canLoadPreview(), `Should not be able to load a preview for a ${remoteObject.subtype} RemoteObject.`);
                 remoteObject.updatePreview((preview) => {
                     InspectorTest.expectNull(preview, `RemoteObject.updatePreview should return null for a ${remoteObject.subtype} RemoteObject.`);
                     resolve();
                 });
-            });
+            }, {remoteObjectOnly: true});
         }
     });
 
@@ -121,8 +118,7 @@
         name: "Runtime.getPreview.Symbol",
         description: "Runtime.getPreview should return an empty preview for non-objects.",
         test(resolve, reject) {
-            InspectorTest.evaluateInPage(`Symbol.iterator`, (error, remoteObjectPayload) => {
-                let remoteObject = WI.RemoteObject.fromPayload(remoteObjectPayload);
+            InspectorTest.evaluateInPage(`Symbol.iterator`, (error, remoteObject) => {
                 RuntimeAgent.getPreview(remoteObject.objectId, (error, result) => {
                     InspectorTest.log(JSON.stringify(result));
                     resolve();
@@ -135,8 +131,7 @@
         name: "Runtime.getPreview.Function",
         description: "Runtime.getPreview should return an empty preview for non-objects.",
         test(resolve, reject) {
-            InspectorTest.evaluateInPage(`function f() {}; f`, (error, remoteObjectPayload) => {
-                let remoteObject = WI.RemoteObject.fromPayload(remoteObjectPayload);
+            InspectorTest.evaluateInPage(`function f() {}; f`, (error, remoteObject) => {
                 RuntimeAgent.getPreview(remoteObject.objectId, (error, result) => {
                     InspectorTest.log(JSON.stringify(result));
                     resolve();

Added: trunk/LayoutTests/inspector/unit-tests/test-harness-evaluate-in-page-expected.txt (0 => 225950)


--- trunk/LayoutTests/inspector/unit-tests/test-harness-evaluate-in-page-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/inspector/unit-tests/test-harness-evaluate-in-page-expected.txt	2017-12-15 01:39:11 UTC (rev 225950)
@@ -0,0 +1,150 @@
+Testing the InspectorTest.evaluateInPage function.
+
+
+== Running test suite: InspectorTest.evaluateInPage
+-- Running test case: evaluateInPage.Primitives
+Checking result of evaluating string: -42
+PASS: Expected and actual evaluation result should be equal.
+PASS: Should not be returned as a WI.RemoteObject.
+
+Checking result of evaluating string: 42
+PASS: Expected and actual evaluation result should be equal.
+PASS: Should not be returned as a WI.RemoteObject.
+
+Checking result of evaluating string: 0
+PASS: Expected and actual evaluation result should be equal.
+PASS: Should not be returned as a WI.RemoteObject.
+
+Checking result of evaluating string: "String"
+PASS: Expected and actual evaluation result should be equal.
+PASS: Should not be returned as a WI.RemoteObject.
+
+Checking result of evaluating string: false
+PASS: Expected and actual evaluation result should be equal.
+PASS: Should not be returned as a WI.RemoteObject.
+
+Checking result of evaluating string: true
+PASS: Expected and actual evaluation result should be equal.
+PASS: Should not be returned as a WI.RemoteObject.
+
+Checking result of evaluating string: null
+PASS: Expected and actual evaluation result should be equal.
+PASS: Should not be returned as a WI.RemoteObject.
+
+
+-- Running test case: evaluateInPage.PrimitivesWithoutUnwrapping
+Checking result of evaluating string without unwrapping: -42
+PASS: Should be returned as a WI.RemoteObject.
+PASS: Type of evaluation result should be 'number'.
+
+Checking result of evaluating string without unwrapping: 42
+PASS: Should be returned as a WI.RemoteObject.
+PASS: Type of evaluation result should be 'number'.
+
+Checking result of evaluating string without unwrapping: 0
+PASS: Should be returned as a WI.RemoteObject.
+PASS: Type of evaluation result should be 'number'.
+
+Checking result of evaluating string without unwrapping: "String"
+PASS: Should be returned as a WI.RemoteObject.
+PASS: Type of evaluation result should be 'string'.
+
+Checking result of evaluating string without unwrapping: false
+PASS: Should be returned as a WI.RemoteObject.
+PASS: Type of evaluation result should be 'boolean'.
+
+Checking result of evaluating string without unwrapping: true
+PASS: Should be returned as a WI.RemoteObject.
+PASS: Type of evaluation result should be 'boolean'.
+
+Checking result of evaluating string without unwrapping: null
+PASS: Should be returned as a WI.RemoteObject.
+PASS: Type of evaluation result should be 'object'.
+
+
+-- Running test case: evaluateInPage.RemoteObjects
+Checking result of evaluating string: ({a:42})
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'object'.
+PASS: Subtype of evaluation result should be 'undefined'.
+
+Checking result of evaluating string: [42, 43, 44]
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'object'.
+PASS: Subtype of evaluation result should be 'array'.
+
+Checking result of evaluating string: new Number(42)
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'object'.
+PASS: Subtype of evaluation result should be 'undefined'.
+
+Checking result of evaluating string: function foo() { return 42; }; foo
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'function'.
+PASS: Subtype of evaluation result should be 'undefined'.
+
+Checking result of evaluating string: Array.prototype.splice
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'function'.
+PASS: Subtype of evaluation result should be 'undefined'.
+
+Checking result of evaluating string: async function foo() { return 42; } foo
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'function'.
+PASS: Subtype of evaluation result should be 'undefined'.
+
+Checking result of evaluating string: () => { return 42; }
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'function'.
+PASS: Subtype of evaluation result should be 'undefined'.
+
+Checking result of evaluating string: async () => { return 42; }
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'function'.
+PASS: Subtype of evaluation result should be 'undefined'.
+
+Checking result of evaluating string: window.document
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'object'.
+PASS: Subtype of evaluation result should be 'node'.
+
+Checking result of evaluating string: Symbol.iterator
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'symbol'.
+PASS: Subtype of evaluation result should be 'undefined'.
+
+Checking result of evaluating string: [].entries()
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'object'.
+PASS: Subtype of evaluation result should be 'iterator'.
+
+Checking result of evaluating string: Promise.resolve()
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'object'.
+PASS: Subtype of evaluation result should be 'undefined'.
+
+Checking result of evaluating string: new Error(42)
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'object'.
+PASS: Subtype of evaluation result should be 'error'.
+
+Checking result of evaluating string: throw new Error(42)
+PASS: Returned result should be a WI.RemoteObject.
+PASS: Non-primitive evaluation results should not have a marshalled value.
+PASS: Type of evaluation result should be 'object'.
+PASS: Subtype of evaluation result should be 'error'.
+
+

Added: trunk/LayoutTests/inspector/unit-tests/test-harness-evaluate-in-page.html (0 => 225950)


--- trunk/LayoutTests/inspector/unit-tests/test-harness-evaluate-in-page.html	                        (rev 0)
+++ trunk/LayoutTests/inspector/unit-tests/test-harness-evaluate-in-page.html	2017-12-15 01:39:11 UTC (rev 225950)
@@ -0,0 +1,160 @@
+<!doctype html>
+<html>
+<head>
+<script src=""
+<script>
+function test()
+{
+    let primitiveCases = [
+        {
+            input: `-42`,
+            type: "number",
+            value: -42,
+        },
+        {
+            input: `42`,
+            type: "number",
+            value: 42,
+        },
+        {
+            input: `0`,
+            type: "number",
+            value: 0,
+        },
+        {
+            input: `"String"`,
+            type: "string",
+            value: "String",
+        },
+        {
+            input: `false`,
+            type: "boolean",
+            value: false,
+        },
+        {
+            input: `true`,
+            type: "boolean",
+            value: true,
+        },
+        {
+            input: `null`,
+            type: "object",
+            value: null,
+        },
+    ];
+
+    let complexCases = [
+        { // Plain object.
+            input: `({a:42})`,
+            type: "object",
+        },
+        { // Plain array.
+            input: `[42, 43, 44]`,
+            type: "object",
+            subtype: "array",
+        },
+        { // JS builtin object.
+            input: `new Number(42)`,
+            type: "object",
+        },
+        { // JS function.
+            input: `function foo() { return 42; }; foo`,
+            type: "function",
+        },
+        { // Native function.
+            input: `Array.prototype.splice`,
+            type: "function",
+        },
+        { // Async function.
+            input: `async function foo() { return 42; } foo`,
+            type: "function",
+        },
+        { // Arrow function.
+            input: `() => { return 42; }`,
+            type: "function",
+        },
+        { // Async arrow function.
+            input: `async () => { return 42; }`,
+            type: "function",
+        },
+        { // DOM object.
+            input: `window.document`,
+            type: "object",
+            subtype: "node",
+        },
+        { // Symbol.
+            input: `Symbol.iterator`,
+            type: "symbol",
+        },
+        { // Iterator.
+            input: `[].entries()`,
+            type: "object",
+            subtype: "iterator",
+        },
+        { // Promise.
+            input: `Promise.resolve()`,
+            type: "object",
+        },
+        { // Error (returned).
+            input: `new Error(42)`,
+            type: "object",
+            subtype: "error",
+        },
+        { // Error (thrown).
+            input: `throw new Error(42)`,
+            type: "object",
+            subtype: "error",
+        },
+    ];
+
+    let suite = InspectorTest.createAsyncSuite("InspectorTest.evaluateInPage");
+
+    suite.addTestCase({
+        name: "evaluateInPage.Primitives",
+        async test() {
+            for (let {input, value} of primitiveCases) {
+                let result = await InspectorTest.evaluateInPage(input);
+                InspectorTest.log(`Checking result of evaluating string: ${input}`);
+                InspectorTest.expectEqual(result, value, "Expected and actual evaluation result should be equal.");
+                InspectorTest.expectFalse(result instanceof WI.RemoteObject, "Should not be returned as a WI.RemoteObject.");
+                InspectorTest.log("");
+            }
+        }
+    });
+
+    suite.addTestCase({
+        name: "evaluateInPage.PrimitivesWithoutUnwrapping",
+        async test() {
+            for (let {input, type, value} of primitiveCases) {
+                let result = await InspectorTest.evaluateInPage(input, null, {remoteObjectOnly: true});
+                InspectorTest.log(`Checking result of evaluating string without unwrapping: ${input}`);
+                InspectorTest.expectThat(result instanceof WI.RemoteObject, "Should be returned as a WI.RemoteObject.");
+                InspectorTest.expectEqual(result.type, type, `Type of evaluation result should be '${type}'.`);
+                InspectorTest.log("");
+            }
+        }
+    });
+
+    suite.addTestCase({
+        name: "evaluateInPage.RemoteObjects",
+        async test() {
+            for (let {input, type, subtype, thrown} of complexCases) {
+                let result = await InspectorTest.evaluateInPage(input);
+                InspectorTest.log(`Checking result of evaluating string: ${input}`);
+                InspectorTest.expectThat(result instanceof WI.RemoteObject, "Returned result should be a WI.RemoteObject.");
+                InspectorTest.expectFalse(result.hasValue(), "Non-primitive evaluation results should not have a marshalled value.");
+                InspectorTest.expectEqual(result.type, type, `Type of evaluation result should be '${type}'.`);
+                InspectorTest.expectEqual(result.subtype, subtype, `Subtype of evaluation result should be '${subtype}'.`);
+                InspectorTest.log("");
+            }
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body _onload_="runTest()">
+    <p>Testing the InspectorTest.evaluateInPage function.</p>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebInspectorUI/ChangeLog (225949 => 225950)


--- trunk/Source/WebInspectorUI/ChangeLog	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/Source/WebInspectorUI/ChangeLog	2017-12-15 01:39:11 UTC (rev 225950)
@@ -1,3 +1,19 @@
+2017-12-14  Brian Burg  <bb...@apple.com>
+
+        Web Inspector: InspectorTest.evaluateInPage should unwrap primitive values by default
+        https://bugs.webkit.org/show_bug.cgi?id=180831
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Test/FrontendTestHarness.js:
+        (FrontendTestHarness.prototype.evaluateInPage.resultObjectToReturn):
+        (FrontendTestHarness.prototype.evaluateInPage):
+        Unwrap the resulting RemoteObject's value if it is a primitive.
+        Add an `options` dictionary so this behavior can be opted out.
+
+        * UserInterface/Test/TestHarness.js:
+        Add documentation of how evaluateInPage works.
+
 2017-12-14  Nikita Vasilyev  <nvasil...@apple.com>
 
         Web Inspector: Styles Redesign: selecting text should not add new properties

Modified: trunk/Source/WebInspectorUI/UserInterface/Test/FrontendTestHarness.js (225949 => 225950)


--- trunk/Source/WebInspectorUI/UserInterface/Test/FrontendTestHarness.js	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/Source/WebInspectorUI/UserInterface/Test/FrontendTestHarness.js	2017-12-15 01:39:11 UTC (rev 225950)
@@ -77,8 +77,10 @@
         this.evaluateInPage(`TestPage.debugLog(unescape("${escape(stringifiedMessage)}"));`);
     }
 
-    evaluateInPage(_expression_, callback)
+    evaluateInPage(_expression_, callback, options={})
     {
+        let remoteObjectOnly = !!options.remoteObjectOnly;
+
         // If we load this page outside of the inspector, or hit an early error when loading
         // the test frontend, then defer evaluating the commands (indefinitely in the former case).
         if (this._originalConsole && !window.RuntimeAgent) {
@@ -86,10 +88,18 @@
             return;
         }
 
-        if (typeof callback === "function")
-            RuntimeAgent.evaluate.invoke({_expression_, objectGroup: "test", includeCommandLineAPI: false}, callback);
-        else
-            return RuntimeAgent.evaluate.invoke({_expression_, objectGroup: "test", includeCommandLineAPI: false});
+        // Return primitive values directly, otherwise return a WI.RemoteObject instance.
+        function resultObjectToReturn(result) {
+            let remoteObject = WI.RemoteObject.fromPayload(result);
+            return (!remoteObjectOnly && remoteObject.hasValue()) ? remoteObject.value : remoteObject;
+        }
+
+        let response = RuntimeAgent.evaluate.invoke({_expression_, objectGroup: "test", includeCommandLineAPI: false})
+        if (callback && typeof callback === "function") {
+            response = response.then(({result, wasThrown}) => callback(null, resultObjectToReturn(result), wasThrown));
+            response = response.catch((error) => callback(error, null, false));
+        } else
+            return response.then(({result}) => resultObjectToReturn(result));
     }
 
     debug()

Modified: trunk/Source/WebInspectorUI/UserInterface/Test/TestHarness.js (225949 => 225950)


--- trunk/Source/WebInspectorUI/UserInterface/Test/TestHarness.js	2017-12-15 01:29:31 UTC (rev 225949)
+++ trunk/Source/WebInspectorUI/UserInterface/Test/TestHarness.js	2017-12-15 01:39:11 UTC (rev 225950)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015, 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -55,7 +55,14 @@
         throw new Error("Must be implemented by subclasses.");
     }
 
-    evaluateInPage(string, callback)
+    // If 'callback' is a function, it will be with the arguments
+    // callback(error, result, wasThrown). Otherwise, a promise is
+    // returned that resolves with 'result' or rejects with 'error'.
+
+    // The options object accepts the following keys and values:
+    // 'remoteObjectOnly': if true, do not unwrap the result payload to a
+    // primitive value even if possible. Useful if testing WI.RemoteObject directly.
+    evaluateInPage(string, callback, options={})
     {
         throw new Error("Must be implemented by subclasses.");
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to