Web Inspector: add RemoteObject.fetchProperties and some basic tests for RemoteObject API
authorbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Jan 2018 21:29:13 +0000 (21:29 +0000)
committerbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Jan 2018 21:29:13 +0000 (21:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180945

Reviewed by Joseph Pecoraro.

Source/WebInspectorUI:

Add a new method, fetchProperties, which async fetches an arbitrary list of properties
from a RemoteObject. This is intended for writing tests and other quick evaluations,
so it has some behaviors that are suitable in these situations:
- If the evaluation throws an exception, the result will reject with that exception.
- If there is a protocol error for some reason, the result will reject with an exception.
- Non-string and non-number keys cause an exception, as this is probably not intended.
- Does not accept a callback, returns a promise only. New code should use async.

For full fidelity introspection of property descriptors, clients should use the existing
getOwnPropertyDescriptor[s] class of methods.

* UserInterface/Protocol/RemoteObject.js:
(WI.RemoteObject.prototype.async.fetchProperties): Added.
- Validate specified keys and remove duplicates.
- Request properties one-by-one to avoid fetching all descriptors and dealing with previews.
- Unwrap returned primitive values to avoid unnecessary munging in tests.

(WI.RemoteObject.prototype.getProperty):
- Rework this to return a promise if no callback was supplied.
- Introduce stricter property type checking to avoid unintended mistakes.

(WI.RemoteObject.prototype.callFunction):
- Rework this to return a promise if no callback was supplied.
- Turn thrown exceptions and protocol errors into rejected promises.

LayoutTests:

Add new test coverage for existing RemoteObject.prototype.getProperty.
Add new test coverage for new method RemoteObject.prototype.fetchProperties.
I didn't add test coverage for callFunction, as I had to stop somewhere.

* inspector/model/remote-object-api-expected.txt: Added.
* inspector/model/remote-object-api.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@226417 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/inspector/model/remote-object-api-expected.txt [new file with mode: 0644]
LayoutTests/inspector/model/remote-object-api.html [new file with mode: 0644]
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Protocol/RemoteObject.js

index 3cd4c44..ab8c53a 100644 (file)
@@ -1,5 +1,19 @@
 2018-01-04  Brian Burg  <bburg@apple.com>
 
+        Web Inspector: add RemoteObject.fetchProperties and some basic tests for RemoteObject API
+        https://bugs.webkit.org/show_bug.cgi?id=180945
+
+        Reviewed by Joseph Pecoraro.
+
+        Add new test coverage for existing RemoteObject.prototype.getProperty.
+        Add new test coverage for new method RemoteObject.prototype.fetchProperties.
+        I didn't add test coverage for callFunction, as I had to stop somewhere.
+
+        * inspector/model/remote-object-api-expected.txt: Added.
+        * inspector/model/remote-object-api.html: Added.
+
+2018-01-04  Brian Burg  <bburg@apple.com>
+
         Web Inspector: add TestPage.debug() to inspect evaluations being sent to Inspector page
         https://bugs.webkit.org/show_bug.cgi?id=181005
 
diff --git a/LayoutTests/inspector/model/remote-object-api-expected.txt b/LayoutTests/inspector/model/remote-object-api-expected.txt
new file mode 100644 (file)
index 0000000..3cc7933
--- /dev/null
@@ -0,0 +1,62 @@
+Testing basic API and convenience methods of RemoteObject.
+
+
+== Running test suite: RemoteObject.API
+-- Running test case: RemoteObject.getProperty.SuccessWithCallback.String
+PASS: Should not have a thrown exception.
+PASS: Fetched property should have a primitive value.
+PASS: Fetched property value should be as expected.
+
+-- Running test case: RemoteObject.getProperty.SuccessWithCallback.Number
+PASS: Should not have a thrown exception.
+PASS: Fetched property should have a primitive value.
+PASS: Fetched property value should be as expected.
+
+-- Running test case: RemoteObject.getProperty.NotFoundWithCallback
+PASS: Should not have a thrown exception.
+PASS: Fetched property should have a primitive value.
+PASS: Fetched property value should be as expected.
+
+-- Running test case: RemoteObject.getProperty.FailureWithCallback
+PASS: Should have a thrown exception.
+PASS: Fetched property should not have a primitive value.
+PASS: Fetched property value should have type `object`.
+PASS: Fetched property value should have subtype `error`.
+
+-- Running test case: RemoteObject.getProperty.SuccessWithPromise.String
+PASS: Fetched property should have a primitive value.
+PASS: Fetched property value should be as expected.
+
+-- Running test case: RemoteObject.getProperty.SuccessWithPromise.Number
+PASS: Fetched property should have a primitive value.
+PASS: Fetched property value should be as expected.
+
+-- Running test case: RemoteObject.getProperty.FailureWithPromise
+PASS: Should produce an exception.
+[object Object]
+
+-- Running test case: RemoteObject.fetchProperties.Success
+PASS: Result object should contain three keys.
+PASS: Result object should contain fetched property 'name'.
+PASS: Result object should contain fetched property 'size'.
+PASS: Result object should contain fetched property 'data'.
+
+-- Running test case: RemoteObject.fetchProperties.SuccessWithDuplicateKey
+PASS: Result object should contain three keys.
+PASS: Result object should contain fetched property 'name'.
+PASS: Result object should contain fetched property 'size'.
+PASS: Result object should contain fetched property 'data'.
+
+-- Running test case: RemoteObject.fetchProperties.SuccessWithOutputObject
+PASS: Resolved value should be the passed-in result object.
+PASS: Result object should contain fetched property 'name'.
+PASS: Result object should contain fetched property 'size'.
+PASS: Result object should contain fetched property 'data'.
+PASS: Fetched property 'name' should equal 'Favorites'.
+PASS: Fetched property 'size' should equal '456'.
+PASS: Fetched property 'data' should be a WI.RemoteObject.
+
+-- Running test case: RemoteObject.fetchProperties.FailureWithInvalidKey
+PASS: Should produce an exception.
+Error: Tried to get property using key is not a string or number: [object Object]
+
diff --git a/LayoutTests/inspector/model/remote-object-api.html b/LayoutTests/inspector/model/remote-object-api.html
new file mode 100644 (file)
index 0000000..915968d
--- /dev/null
@@ -0,0 +1,178 @@
+<!doctype html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script>
+window.testObject = {
+    1: true,
+    "name": "Favorites",
+    "size": 456,
+    "data": {x: 42, y: 50},
+};
+
+function test()
+{
+    let object;
+    let suite = InspectorTest.createAsyncSuite("RemoteObject.API");
+
+    suite.addTestCase({
+        name: "RemoteObject.getProperty.SuccessWithCallback.String",
+        async test() {
+            let finished = new WI.WrappedPromise;
+            object.getProperty("name", (error, result, wasThrown) => {
+                InspectorTest.assert(!error, "Should not have a protocol error.");
+
+                InspectorTest.expectFalse(wasThrown, "Should not have a thrown exception.");
+                InspectorTest.expectThat(result.hasValue(), "Fetched property should have a primitive value.");
+                InspectorTest.expectEqual(result.value, "Favorites", "Fetched property value should be as expected.");
+                error ? finished.reject(error) : finished.resolve();
+            });
+
+            await finished.promise;
+        }
+    });
+
+    suite.addTestCase({
+        name: "RemoteObject.getProperty.SuccessWithCallback.Number",
+        async test() {
+            let finished = new WI.WrappedPromise;
+            object.getProperty(1, (error, result, wasThrown) => {
+                InspectorTest.assert(!error, "Should not have a protocol error.");
+
+                InspectorTest.expectFalse(wasThrown, "Should not have a thrown exception.");
+                InspectorTest.expectThat(result.hasValue(), "Fetched property should have a primitive value.");
+                InspectorTest.expectEqual(result.value, true, "Fetched property value should be as expected.");
+                error ? finished.reject(error) : finished.resolve();
+            });
+
+            await finished.promise;
+        }
+    });
+
+    suite.addTestCase({
+        name: "RemoteObject.getProperty.NotFoundWithCallback",
+        async test() {
+            let finished = new WI.WrappedPromise;
+            object.getProperty("doesNotExist", (error, result, wasThrown) => {
+                InspectorTest.assert(!error, "Should not have a protocol error.");
+
+                InspectorTest.expectFalse(wasThrown, "Should not have a thrown exception.");
+                InspectorTest.expectThat(result.hasValue(), "Fetched property should have a primitive value.");
+                InspectorTest.expectEqual(result.value, undefined, "Fetched property value should be as expected.");
+                error ? finished.reject(error) : finished.resolve();
+            });
+
+            await finished.promise;
+        }
+    });
+
+    suite.addTestCase({
+        name: "RemoteObject.getProperty.FailureWithCallback",
+        async test() {
+            let finished = new WI.WrappedPromise;
+            object.getProperty({}, (error, result, wasThrown) => {
+                InspectorTest.assert(!error, "Should not have a protocol error.");
+
+                InspectorTest.expectThat(wasThrown, "Should have a thrown exception.");
+                InspectorTest.expectFalse(result.hasValue(), "Fetched property should not have a primitive value.");
+                InspectorTest.expectEqual(result.type, "object", "Fetched property value should have type `object`.");
+                InspectorTest.expectEqual(result.subtype, "error", "Fetched property value should have subtype `error`.");
+
+                error ? finished.reject(error) : finished.resolve();
+            });
+
+            await finished.promise;
+        }
+    });
+
+    suite.addTestCase({
+        name: "RemoteObject.getProperty.SuccessWithPromise.String",
+        async test() {
+            let result = await object.getProperty("name");
+            InspectorTest.expectThat(result.hasValue(), "Fetched property should have a primitive value.");
+            InspectorTest.expectEqual(result.value, "Favorites", "Fetched property value should be as expected.");
+        }
+    });
+
+    suite.addTestCase({
+        name: "RemoteObject.getProperty.SuccessWithPromise.Number",
+        async test() {
+            let result = await object.getProperty(1);
+            InspectorTest.expectThat(result.hasValue(), "Fetched property should have a primitive value.");
+            InspectorTest.expectEqual(result.value, true, "Fetched property value should be as expected.");
+        }
+    });
+
+    suite.addTestCase({
+        name: "RemoteObject.getProperty.FailureWithPromise",
+        async test() {
+            await InspectorTest.expectException(async () => {
+                await object.getProperty({});
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "RemoteObject.fetchProperties.Success",
+        async test() {
+            let keys = ["name", "size", "data"];
+            let result = await object.fetchProperties(keys);
+
+            InspectorTest.expectEqual(Object.keys(result).length, 3, "Result object should contain three keys.");
+            for (let key of keys)
+                InspectorTest.expectThat(key in result, `Result object should contain fetched property '${key}'.`);
+        }
+    });
+
+    suite.addTestCase({
+        name: "RemoteObject.fetchProperties.SuccessWithDuplicateKey",
+        async test() {
+            let keys = ["name", "size", "data", "data"];
+            let result = await object.fetchProperties(keys);
+
+            InspectorTest.expectEqual(Object.keys(result).length, 3, "Result object should contain three keys.");
+            for (let key of new Set(keys))
+                InspectorTest.expectThat(key in result, `Result object should contain fetched property '${key}'.`);
+        }
+    });
+
+    suite.addTestCase({
+        name: "RemoteObject.fetchProperties.SuccessWithOutputObject",
+        async test() {
+            let resultObject = {"existingKey": true};
+            let keys = ["name", "size", "data"];
+            let returnValue = await object.fetchProperties(keys, resultObject);
+            InspectorTest.expectEqual(returnValue, resultObject, "Resolved value should be the passed-in result object.");
+            for (let key of keys)
+                InspectorTest.expectThat(key in resultObject, `Result object should contain fetched property '${key}'.`);
+
+            let {name, size, data} = returnValue;
+            InspectorTest.expectEqual(name, "Favorites", `Fetched property 'name' should equal 'Favorites'.`);
+            InspectorTest.expectEqual(size, 456, `Fetched property 'size' should equal '456'.`);
+            InspectorTest.expectThat(data instanceof WI.RemoteObject, `Fetched property 'data' should be a WI.RemoteObject.`);
+        }
+    });
+
+    suite.addTestCase({
+        name: "RemoteObject.fetchProperties.FailureWithInvalidKey",
+        async test() {
+            let keys = ["name", "size", {}];
+            await InspectorTest.expectException(async () => {
+                await object.fetchProperties(keys);
+            });
+        }
+    });
+
+    InspectorTest.evaluateInPage(`window.testObject`).then((result) => {
+        object = result;
+        InspectorTest.assert(object instanceof WI.RemoteObject, "Should get a remote object.");
+
+        suite.runTestCasesAndFinish();
+    }).catch(reportUnhandledRejection);
+}
+</script>
+</head>
+<body onload="runTest()">
+    <p>Testing basic API and convenience methods of RemoteObject.</p>
+</body>
+</html>
\ No newline at end of file
index a7cb874..858ab15 100644 (file)
@@ -1,3 +1,35 @@
+2018-01-04  Brian Burg  <bburg@apple.com>
+
+        Web Inspector: add RemoteObject.fetchProperties and some basic tests for RemoteObject API
+        https://bugs.webkit.org/show_bug.cgi?id=180945
+
+        Reviewed by Joseph Pecoraro.
+
+        Add a new method, fetchProperties, which async fetches an arbitrary list of properties
+        from a RemoteObject. This is intended for writing tests and other quick evaluations,
+        so it has some behaviors that are suitable in these situations:
+        - If the evaluation throws an exception, the result will reject with that exception.
+        - If there is a protocol error for some reason, the result will reject with an exception.
+        - Non-string and non-number keys cause an exception, as this is probably not intended.
+        - Does not accept a callback, returns a promise only. New code should use async.
+
+        For full fidelity introspection of property descriptors, clients should use the existing
+        getOwnPropertyDescriptor[s] class of methods.
+
+        * UserInterface/Protocol/RemoteObject.js:
+        (WI.RemoteObject.prototype.async.fetchProperties): Added.
+        - Validate specified keys and remove duplicates.
+        - Request properties one-by-one to avoid fetching all descriptors and dealing with previews.
+        - Unwrap returned primitive values to avoid unnecessary munging in tests.
+
+        (WI.RemoteObject.prototype.getProperty):
+        - Rework this to return a promise if no callback was supplied.
+        - Introduce stricter property type checking to avoid unintended mistakes.
+
+        (WI.RemoteObject.prototype.callFunction):
+        - Rework this to return a promise if no callback was supplied.
+        - Turn thrown exceptions and protocol errors into rejected promises.
+
 2018-01-04  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: ⌘G / ⇧⌘G text search does not working after closing find banner
index c5abd05..809777e 100644 (file)
@@ -462,29 +462,73 @@ WI.RemoteObject = class RemoteObject
             callback(0);
     }
 
-    getProperty(propertyName, callback)
+    async fetchProperties(propertyNames, resultObject={})
     {
-        function inspectedPage_object_getProperty(property) {
-            return this[property];
+        let seenPropertyNames = new Set;
+        let requestedValues = [];
+        for (let propertyName of propertyNames) {
+            // Check this here, otherwise things like '{}' would be valid Set keys.
+            if (typeof propertyName !== "string" && typeof propertyName !== "number")
+                throw new Error(`Tried to get property using key is not a string or number: ${propertyName}`);
+
+            if (seenPropertyNames.has(propertyName))
+                continue;
+
+            seenPropertyNames.add(propertyName);
+            requestedValues.push(this.getProperty(propertyName));
         }
 
-        this.callFunction(inspectedPage_object_getProperty, [propertyName], true, callback);
+        // Return primitive values directly, otherwise return a WI.RemoteObject instance.
+        function maybeUnwrapValue(remoteObject) {
+            return remoteObject.hasValue() ? remoteObject.value : remoteObject;
+        }
+
+        // Request property values one by one, since returning an array of property
+        // values would then be subject to arbitrary object preview size limits.
+        let fetchedKeys = Array.from(seenPropertyNames);
+        let fetchedValues = await Promise.all(requestedValues);
+        for (let i = 0; i < fetchedKeys.length; ++i)
+            resultObject[fetchedKeys[i]] = maybeUnwrapValue(fetchedValues[i]);
+
+        return resultObject;
     }
 
-    callFunction(functionDeclaration, args, generatePreview, callback)
+    getProperty(propertyName, callback = null)
     {
-        function mycallback(error, result, wasThrown)
-        {
-            result = result ? WI.RemoteObject.fromPayload(result, this._target) : null;
+        function inspectedPage_object_getProperty(property) {
+            if (typeof property !== "string" && typeof property !== "number")
+                throw new Error(`Tried to get property using key is not a string or number: ${property}`);
 
-            if (callback && typeof callback === "function")
-                callback(error, result, wasThrown);
+            return this[property];
         }
 
+        if (callback && typeof callback === "function")
+            this.callFunction(inspectedPage_object_getProperty, [propertyName], true, callback);
+        else
+            return this.callFunction(inspectedPage_object_getProperty, [propertyName], true);
+    }
+
+    callFunction(functionDeclaration, args, generatePreview, callback = null)
+    {
+        let translateResult = (result) => result ? WI.RemoteObject.fromPayload(result, this._target) : null;
+
         if (args)
             args = args.map(WI.RemoteObject.createCallArgument);
 
-        this._target.RuntimeAgent.callFunctionOn(this._objectId, appendWebInspectorSourceURL(functionDeclaration.toString()), args, true, undefined, !!generatePreview, mycallback.bind(this));
+        if (callback && typeof callback === "function") {
+            this._target.RuntimeAgent.callFunctionOn(this._objectId, appendWebInspectorSourceURL(functionDeclaration.toString()), args, true, undefined, !!generatePreview, (error, result, wasThrown) => {
+                callback(error, translateResult(result), wasThrown);
+            });
+        } else {
+            // Protocol errors and results that were thrown should cause promise rejection with the same.
+            return this._target.RuntimeAgent.callFunctionOn(this._objectId, appendWebInspectorSourceURL(functionDeclaration.toString()), args, true, undefined, !!generatePreview)
+                .then(({result, wasThrown}) => {
+                    result = translateResult(result);
+                    if (result && wasThrown)
+                        return Promise.reject(result);
+                    return Promise.resolve(result);
+                });
+        }
     }
 
     callFunctionJSON(functionDeclaration, args, callback)