Attachment 346936 Details for Bug 187542 – [PATCH] Proposed Fix

[patch] [PATCH] Proposed Fix

invoke-getter-1.patch (text/plain), 4.52 KB, created by Joseph Pecoraro on 2018-08-10 15:45:38 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Joseph Pecoraro

Created: 2018-08-10 15:45:38 PDT

Size: 4.52 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index bf935a02ae5..523b17f7c6c 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-08-10  Joseph Pecoraro  <pecoraro@apple.com>
>+
>+        Web Inspector: console.log fires getters for deep properties
>+        https://bugs.webkit.org/show_bug.cgi?id=187542
>+        <rdar://problem/42873158>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * inspector/injected-script/avoid-getter-invocation-expected.txt: Added.
>+        * inspector/injected-script/avoid-getter-invocation.html: Added.
>+
> 2018-08-09  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Eliminate remaining instances of "ElCapitan" from LayoutTests / Tools
>diff --git a/LayoutTests/inspector/injected-script/avoid-getter-invocation-expected.txt b/LayoutTests/inspector/injected-script/avoid-getter-invocation-expected.txt
>new file mode 100644
>index 00000000000..1998b7e03c0
>--- /dev/null
>+++ b/LayoutTests/inspector/injected-script/avoid-getter-invocation-expected.txt
>@@ -0,0 +1,9 @@
>+CONSOLE MESSAGE: line 11: [object Object]
>+CONSOLE MESSAGE: line 12: [object Object]
>+CONSOLE MESSAGE: line 13: [object Object]
>+Test that certain InjectedScriptSource operations do not invoke user getters.
>+
>+
>+== Running test suite: InjectedScript.GetterInvocation.Preview
>+-- Running test case: InjectedScript.GetterInvocation.Preview
>+
>diff --git a/LayoutTests/inspector/injected-script/avoid-getter-invocation.html b/LayoutTests/inspector/injected-script/avoid-getter-invocation.html
>new file mode 100644
>index 00000000000..35b8b428cab
>--- /dev/null
>+++ b/LayoutTests/inspector/injected-script/avoid-getter-invocation.html
>@@ -0,0 +1,35 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
>+<script>
>+function triggerLogs() {
>+    let test = {};
>+    Object.defineProperty(test, "foo", {
>+        get() { console.log("FAILURE: Should not fire getter") }
>+    });
>+    console.log(test);
>+    console.log({test});
>+    console.log([test]);
>+}
>+
>+function test()
>+{
>+    let suite = InspectorTest.createAsyncSuite("InjectedScript.GetterInvocation.Preview");
>+
>+    suite.addTestCase({
>+        name: "InjectedScript.GetterInvocation.Preview",
>+        description: "Getters should not be invoked during object previews.",
>+        async test() {
>+            await InspectorTest.evaluateInPage(`triggerLogs()`);
>+        }
>+    });
>+
>+    suite.runTestCasesAndFinish();
>+}
>+</script>
>+</head>
>+<body onload="runTest()">
>+<p>Test that certain InjectedScriptSource operations do not invoke user getters.</p>
>+</body>
>+</html>
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index b4c8aad7a0b..1e530667347 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-08-10  Joseph Pecoraro  <pecoraro@apple.com>
>+
>+        Web Inspector: console.log fires getters for deep properties
>+        https://bugs.webkit.org/show_bug.cgi?id=187542
>+        <rdar://problem/42873158>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * inspector/InjectedScriptSource.js:
>+        (RemoteObject.prototype._isPreviewableObject):
>+        Avoid getters/setters when checking for simple properties to preview.
>+        Here we avoid invoking `object[property]` if it could be a user getter.
>+
> 2018-08-05  Darin Adler  <darin@apple.com>
> 
>         [Cocoa] More tweaks and refactoring to prepare for ARC
>diff --git a/Source/JavaScriptCore/inspector/InjectedScriptSource.js b/Source/JavaScriptCore/inspector/InjectedScriptSource.js
>index 45118a53e64..d6b75f1fecd 100644
>--- a/Source/JavaScriptCore/inspector/InjectedScriptSource.js
>+++ b/Source/JavaScriptCore/inspector/InjectedScriptSource.js
>@@ -1286,12 +1286,15 @@ let RemoteObject = class RemoteObject
>         if (object.__proto__ && object.__proto__.__proto__)
>             return false;
> 
>-        // Objects are simple if they have 3 or less simple properties.
>+        // Objects are simple if they have 3 or less simple value properties.
>         let ownPropertyNames = Object.getOwnPropertyNames(object);
>         if (ownPropertyNames.length > 3)
>             return false;
>         for (let i = 0; i < ownPropertyNames.length; ++i) {
>             let propertyName = ownPropertyNames[i];
>+            let descriptor = Object.getOwnPropertyDescriptor(object, propertyName);
>+            if (descriptor && !("value" in descriptor))
>+                return false;
>             if (!this._isPreviewableObjectInternal(object[propertyName], knownObjects, depth))
>                 return false;
>         }

Actions: View | Formatted Diff | Diff

Attachments on bug 187542: 346936