Attachment 248712 Details for Bug 142322 – [PATCH] Proposed Fix

[patch] [PATCH] Proposed Fix

bug-142322-20150315232057.patch (text/plain), 18.79 KB, created by Joseph Pecoraro on 2015-03-15 23:21:26 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Joseph Pecoraro

Created: 2015-03-15 23:21:26 PDT

Size: 18.79 KB

patch

obsolete

>Subversion Revision: 181502
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 746d6e408394156f4809b032aba69d740c660f39..9bb17f1b2ddff5d8b0d46beb928c163534c7fe9b 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,5 +1,17 @@
> 2015-03-15  Joseph Pecoraro  <pecoraro@apple.com>
> 
>+        Web Inspector: Better Console Previews for Arrays / Small Objects
>+        https://bugs.webkit.org/show_bug.cgi?id=142322
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * inspector/InjectedScriptSource.js:
>+        Create deep valuePreviews for simple previewable objects,
>+        such as arrays with 5 values, or basic objects with
>+        3 properties.
>+
>+2015-03-15  Joseph Pecoraro  <pecoraro@apple.com>
>+
>         Update Map/Set to treat -0 and 0 as the same value
>         https://bugs.webkit.org/show_bug.cgi?id=142709
> 
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog
>index fc615c6c66b9c9d18feada4af664a4daedafcabd..1149b43de497fb6b7b873e4a9eca0a32c5d7c326 100644
>--- a/Source/WebInspectorUI/ChangeLog
>+++ b/Source/WebInspectorUI/ChangeLog
>@@ -1,3 +1,19 @@
>+2015-03-15  Joseph Pecoraro  <pecoraro@apple.com>
>+
>+        Web Inspector: Better Console Previews for Arrays / Small Objects
>+        https://bugs.webkit.org/show_bug.cgi?id=142322
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UserInterface/Views/ObjectPreviewView.js:
>+        If there is a sub-preview, show the sub-preview.
>+
>+        * UserInterface/Views/ObjectTreeView.js:
>+        (WebInspector.ObjectTreeView):
>+        For an ObjectTree that is not a root (e.g. one inside of
>+        an array/set/map property tree element) allow it to be
>+        expanded even if the preview is lossless.
>+
> 2015-03-12  Jono Wells  <jonowells@apple.com>
> 
>         Web Inspector: Debugger sidebar should group global breakpoints together
>diff --git a/Source/JavaScriptCore/inspector/InjectedScriptSource.js b/Source/JavaScriptCore/inspector/InjectedScriptSource.js
>index 9127eff304802a5205d061ad8edff1071f7528fd..5da0a9a81700548695b56a15b4cb31dd73f0aafd 100644
>--- a/Source/JavaScriptCore/inspector/InjectedScriptSource.js
>+++ b/Source/JavaScriptCore/inspector/InjectedScriptSource.js
>@@ -990,13 +990,14 @@ InjectedScript.RemoteObject.prototype = {
>                     return preview;
>             }
> 
>+            if (preview.entries)
>+                return preview;
>+
>             // Properties.
>             var descriptors = injectedScript._propertyDescriptors(object, InjectedScript.CollectionMode.AllProperties);
>             this._appendPropertyPreviews(preview, descriptors, false, propertiesThreshold, firstLevelKeys, secondLevelKeys);
>             if (propertiesThreshold.indexes < 0 || propertiesThreshold.properties < 0)
>                 return preview;
>-
>-            // FIXME: Iterator entries.
>         } catch (e) {
>             preview.lossless = false;
>         }
>@@ -1093,6 +1094,13 @@ InjectedScript.RemoteObject.prototype = {
>                     preview.lossless = false;
>                 if (subPreview.overflow)
>                     preview.overflow = true;
>+            } else if (this._isPreviewableObject(value)) {
>+                var subPreview = this._createObjectPreviewForValue(value);
>+                property.valuePreview = subPreview;
>+                if (!subPreview.lossless)
>+                    preview.lossless = false;
>+                if (subPreview.overflow)
>+                    preview.overflow = true;
>             } else {
>                 var description = "";
>                 if (type !== "function")
>@@ -1145,6 +1153,57 @@ InjectedScript.RemoteObject.prototype = {
>         }, this);
>     },
> 
>+    _isPreviewableObject: function(object)
>+    {
>+        return this._isPreviewableObjectInternal(object, new Set, 1);
>+    },
>+
>+    _isPreviewableObjectInternal: function(object, seen, depth)
>+    {
>+        // Deep object.
>+        if (depth > 3)
>+            return false;
>+
>+        // Primitive.
>+        if (injectedScript.isPrimitiveValue(object) || isSymbol(object))
>+            return true;
>+
>+        // Cyclic objects.
>+        if (seen.has(object))
>+            return false;
>+
>+        ++depth;
>+        seen.add(object);
>+
>+        // Arrays are simple if they have 5 or less simple objects.
>+        var subtype = injectedScript._subtype(object);
>+        if (subtype === "array") {
>+            var length = object.length;
>+            if (length > 5)
>+                return false;
>+            for (var i = 0; i < length; ++i) {
>+                if (!this._isPreviewableObjectInternal(object[i], seen, depth))
>+                    return false;
>+            }
>+            return true;
>+        }
>+
>+        // Not a basic object.
>+        if (object.__proto__ && object.__proto__.__proto__)
>+            return false;
>+
>+        // Objects are simple if they have 3 or less simple properties.
>+        var ownPropertyNames = Object.getOwnPropertyNames(object);
>+        if (ownPropertyNames.length > 3)
>+            return false;
>+        for (var propertyName of ownPropertyNames) {
>+            if (!this._isPreviewableObjectInternal(object[propertyName], seen, depth))
>+                return false;
>+        }
>+
>+        return true;
>+    },
>+
>     _abbreviateString: function(string, maxLength, middle)
>     {
>         if (string.length <= maxLength)
>diff --git a/Source/WebInspectorUI/UserInterface/Views/ObjectPreviewView.js b/Source/WebInspectorUI/UserInterface/Views/ObjectPreviewView.js
>index 9d87a45e3042bbdd3ba55412734f3ae58decac19..c7ed96e06fbdee8020dc21a16e8927441026fa43 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/ObjectPreviewView.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/ObjectPreviewView.js
>@@ -211,7 +211,10 @@ WebInspector.ObjectPreviewView.prototype = {
>                 element.appendChild(document.createTextNode(": "));
>             }
> 
>-            element.appendChild(WebInspector.FormattedValue.createElementForPropertyPreview(property));
>+            if (property.valuePreview)
>+                this._appendPreview(element, property.valuePreview);
>+            else
>+                element.appendChild(WebInspector.FormattedValue.createElementForPropertyPreview(property));
>         }
> 
>         if (preview.overflow)
>diff --git a/Source/WebInspectorUI/UserInterface/Views/ObjectTreeView.js b/Source/WebInspectorUI/UserInterface/Views/ObjectTreeView.js
>index 6818befe2d45241d04314da4ede6c09e5062cadc..8d70501b5318794e2bce4f43d8eb12a9686aa863 100644
>--- a/Source/WebInspectorUI/UserInterface/Views/ObjectTreeView.js
>+++ b/Source/WebInspectorUI/UserInterface/Views/ObjectTreeView.js
>@@ -49,7 +49,7 @@ WebInspector.ObjectTreeView = function(object, mode, propertyPath, forceExpandin
>         this._previewView.element.addEventListener("click", this._handlePreviewOrTitleElementClick.bind(this));
>         this._element.appendChild(this._previewView.element);
> 
>-        if (this._previewView.lossless && !forceExpanding) {
>+        if (this._previewView.lossless && !this._propertyPath.parent && !forceExpanding) {
>             this._hasLosslessPreview = true;
>             this.element.classList.add("lossless-preview");
>         }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ee26d99bd95ff0caad6c61fcc12726db78545db1..08ebcdaed55d472965b43ffa20284b0ac3189b77 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,5 +1,17 @@
> 2015-03-15  Joseph Pecoraro  <pecoraro@apple.com>
> 
>+        Web Inspector: Better Console Previews for Arrays / Small Objects
>+        https://bugs.webkit.org/show_bug.cgi?id=142322
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * inspector/model/remote-object-expected.txt:
>+        * inspector/model/remote-object.html:
>+        Include a test with cyclic values, and update
>+        results which now have sub-previews.
>+
>+2015-03-15  Joseph Pecoraro  <pecoraro@apple.com>
>+
>         Update Map/Set to treat -0 and 0 as the same value
>         https://bugs.webkit.org/show_bug.cgi?id=142709
> 
>diff --git a/LayoutTests/inspector/model/remote-object-expected.txt b/LayoutTests/inspector/model/remote-object-expected.txt
>index b15f60ba6ab27f7422e81e7007af7f98f2923f11..d2f0549db17d425beba7b96b4ef613a07b92a118 100644
>--- a/LayoutTests/inspector/model/remote-object-expected.txt
>+++ b/LayoutTests/inspector/model/remote-object-expected.txt
>@@ -592,7 +592,7 @@ EXPRESSION: [[1],[2],[3]]
>     "_type": "object",
>     "_subtype": "array",
>     "_description": "Array",
>-    "_lossless": false,
>+    "_lossless": true,
>     "_overflow": false,
>     "_size": 3,
>     "_properties": [
>@@ -600,19 +600,64 @@ EXPRESSION: [[1],[2],[3]]
>         "_name": "0",
>         "_type": "object",
>         "_subtype": "array",
>-        "_value": "Array"
>+        "_valuePreview": {
>+          "_type": "object",
>+          "_subtype": "array",
>+          "_description": "Array",
>+          "_lossless": true,
>+          "_overflow": false,
>+          "_size": 1,
>+          "_properties": [
>+            {
>+              "_name": "0",
>+              "_type": "number",
>+              "_value": "1"
>+            }
>+          ],
>+          "_entries": null
>+        }
>       },
>       {
>         "_name": "1",
>         "_type": "object",
>         "_subtype": "array",
>-        "_value": "Array"
>+        "_valuePreview": {
>+          "_type": "object",
>+          "_subtype": "array",
>+          "_description": "Array",
>+          "_lossless": true,
>+          "_overflow": false,
>+          "_size": 1,
>+          "_properties": [
>+            {
>+              "_name": "0",
>+              "_type": "number",
>+              "_value": "2"
>+            }
>+          ],
>+          "_entries": null
>+        }
>       },
>       {
>         "_name": "2",
>         "_type": "object",
>         "_subtype": "array",
>-        "_value": "Array"
>+        "_valuePreview": {
>+          "_type": "object",
>+          "_subtype": "array",
>+          "_description": "Array",
>+          "_lossless": true,
>+          "_overflow": false,
>+          "_size": 1,
>+          "_properties": [
>+            {
>+              "_name": "0",
>+              "_type": "number",
>+              "_value": "3"
>+            }
>+          ],
>+          "_entries": null
>+        }
>       }
>     ],
>     "_entries": null
>@@ -678,24 +723,63 @@ EXPRESSION: [{a:1}, {b:2}, {c:2}]
>     "_type": "object",
>     "_subtype": "array",
>     "_description": "Array",
>-    "_lossless": false,
>+    "_lossless": true,
>     "_overflow": false,
>     "_size": 3,
>     "_properties": [
>       {
>         "_name": "0",
>         "_type": "object",
>-        "_value": "Object"
>+        "_valuePreview": {
>+          "_type": "object",
>+          "_description": "Object",
>+          "_lossless": true,
>+          "_overflow": false,
>+          "_properties": [
>+            {
>+              "_name": "a",
>+              "_type": "number",
>+              "_value": "1"
>+            }
>+          ],
>+          "_entries": null
>+        }
>       },
>       {
>         "_name": "1",
>         "_type": "object",
>-        "_value": "Object"
>+        "_valuePreview": {
>+          "_type": "object",
>+          "_description": "Object",
>+          "_lossless": true,
>+          "_overflow": false,
>+          "_properties": [
>+            {
>+              "_name": "b",
>+              "_type": "number",
>+              "_value": "2"
>+            }
>+          ],
>+          "_entries": null
>+        }
>       },
>       {
>         "_name": "2",
>         "_type": "object",
>-        "_value": "Object"
>+        "_valuePreview": {
>+          "_type": "object",
>+          "_description": "Object",
>+          "_lossless": true,
>+          "_overflow": false,
>+          "_properties": [
>+            {
>+              "_name": "c",
>+              "_type": "number",
>+              "_value": "2"
>+            }
>+          ],
>+          "_entries": null
>+        }
>       }
>     ],
>     "_entries": null
>@@ -714,7 +798,7 @@ EXPRESSION: [[{a:1}, {b:2}, {c:2}]]
>     "_type": "object",
>     "_subtype": "array",
>     "_description": "Array",
>-    "_lossless": false,
>+    "_lossless": true,
>     "_overflow": false,
>     "_size": 1,
>     "_properties": [
>@@ -722,7 +806,71 @@ EXPRESSION: [[{a:1}, {b:2}, {c:2}]]
>         "_name": "0",
>         "_type": "object",
>         "_subtype": "array",
>-        "_value": "Array"
>+        "_valuePreview": {
>+          "_type": "object",
>+          "_subtype": "array",
>+          "_description": "Array",
>+          "_lossless": true,
>+          "_overflow": false,
>+          "_size": 3,
>+          "_properties": [
>+            {
>+              "_name": "0",
>+              "_type": "object",
>+              "_valuePreview": {
>+                "_type": "object",
>+                "_description": "Object",
>+                "_lossless": true,
>+                "_overflow": false,
>+                "_properties": [
>+                  {
>+                    "_name": "a",
>+                    "_type": "number",
>+                    "_value": "1"
>+                  }
>+                ],
>+                "_entries": null
>+              }
>+            },
>+            {
>+              "_name": "1",
>+              "_type": "object",
>+              "_valuePreview": {
>+                "_type": "object",
>+                "_description": "Object",
>+                "_lossless": true,
>+                "_overflow": false,
>+                "_properties": [
>+                  {
>+                    "_name": "b",
>+                    "_type": "number",
>+                    "_value": "2"
>+                  }
>+                ],
>+                "_entries": null
>+              }
>+            },
>+            {
>+              "_name": "2",
>+              "_type": "object",
>+              "_valuePreview": {
>+                "_type": "object",
>+                "_description": "Object",
>+                "_lossless": true,
>+                "_overflow": false,
>+                "_properties": [
>+                  {
>+                    "_name": "c",
>+                    "_type": "number",
>+                    "_value": "2"
>+                  }
>+                ],
>+                "_entries": null
>+              }
>+            }
>+          ],
>+          "_entries": null
>+        }
>       }
>     ],
>     "_entries": null
>@@ -1554,6 +1702,33 @@ EXPRESSION: ({a: 1, b: "string", c: /regex/, d: Symbol('sym')})
> }
> 
> -----------------------------------------------------
>+EXPRESSION: o = {a:1}; o.b = o; o
>+{
>+  "_type": "object",
>+  "_objectId": "<filtered>",
>+  "_description": "Object",
>+  "_preview": {
>+    "_type": "object",
>+    "_description": "Object",
>+    "_lossless": false,
>+    "_overflow": false,
>+    "_properties": [
>+      {
>+        "_name": "a",
>+        "_type": "number",
>+        "_value": "1"
>+      },
>+      {
>+        "_name": "b",
>+        "_type": "object",
>+        "_value": "Object"
>+      }
>+    ],
>+    "_entries": null
>+  }
>+}
>+
>+-----------------------------------------------------
> EXPRESSION: ({a:function a(){}, b:function b(){}, get getter(){}, set setter(v){}})
> {
>   "_type": "object",
>@@ -2948,14 +3123,23 @@ EXPRESSION: [][Symbol.iterator]()
>     "_type": "object",
>     "_subtype": "iterator",
>     "_description": "ArrayIterator",
>-    "_lossless": false,
>+    "_lossless": true,
>     "_overflow": false,
>     "_properties": [
>       {
>         "_name": "array",
>         "_type": "object",
>         "_subtype": "array",
>-        "_value": "Array",
>+        "_valuePreview": {
>+          "_type": "object",
>+          "_subtype": "array",
>+          "_description": "Array",
>+          "_lossless": true,
>+          "_overflow": false,
>+          "_size": 0,
>+          "_properties": [],
>+          "_entries": null
>+        },
>         "_internal": true
>       },
>       {
>@@ -2980,14 +3164,29 @@ EXPRESSION: [1][Symbol.iterator]()
>     "_type": "object",
>     "_subtype": "iterator",
>     "_description": "ArrayIterator",
>-    "_lossless": false,
>+    "_lossless": true,
>     "_overflow": false,
>     "_properties": [
>       {
>         "_name": "array",
>         "_type": "object",
>         "_subtype": "array",
>-        "_value": "Array",
>+        "_valuePreview": {
>+          "_type": "object",
>+          "_subtype": "array",
>+          "_description": "Array",
>+          "_lossless": true,
>+          "_overflow": false,
>+          "_size": 1,
>+          "_properties": [
>+            {
>+              "_name": "0",
>+              "_type": "number",
>+              "_value": "1"
>+            }
>+          ],
>+          "_entries": null
>+        },
>         "_internal": true
>       },
>       {
>@@ -3684,14 +3883,34 @@ EXPRESSION: x = undefined; (function() { x = arguments; })(1, 'two'); x[Symbol.i
>     "_type": "object",
>     "_subtype": "iterator",
>     "_description": "ArgumentsIterator",
>-    "_lossless": false,
>+    "_lossless": true,
>     "_overflow": false,
>     "_properties": [
>       {
>         "_name": "arguments",
>         "_type": "object",
>         "_subtype": "array",
>-        "_value": "Arguments",
>+        "_valuePreview": {
>+          "_type": "object",
>+          "_subtype": "array",
>+          "_description": "Arguments",
>+          "_lossless": true,
>+          "_overflow": false,
>+          "_size": 2,
>+          "_properties": [
>+            {
>+              "_name": "0",
>+              "_type": "number",
>+              "_value": "1"
>+            },
>+            {
>+              "_name": "1",
>+              "_type": "string",
>+              "_value": "two"
>+            }
>+          ],
>+          "_entries": null
>+        },
>         "_internal": true
>       }
>     ],
>@@ -3839,7 +4058,7 @@ EXPRESSION: Promise.resolve({result:1})
>   "_preview": {
>     "_type": "object",
>     "_description": "Promise",
>-    "_lossless": false,
>+    "_lossless": true,
>     "_overflow": false,
>     "_properties": [
>       {
>@@ -3851,7 +4070,20 @@ EXPRESSION: Promise.resolve({result:1})
>       {
>         "_name": "result",
>         "_type": "object",
>-        "_value": "Object",
>+        "_valuePreview": {
>+          "_type": "object",
>+          "_description": "Object",
>+          "_lossless": true,
>+          "_overflow": false,
>+          "_properties": [
>+            {
>+              "_name": "result",
>+              "_type": "number",
>+              "_value": "1"
>+            }
>+          ],
>+          "_entries": null
>+        },
>         "_internal": true
>       }
>     ],
>diff --git a/LayoutTests/inspector/model/remote-object.html b/LayoutTests/inspector/model/remote-object.html
>index 2134948fbeb5d0a5ed0b92af7dd0947b90c02a32..6ecac87c3b7d4712a37abdb9c102d76b01afdba3 100644
>--- a/LayoutTests/inspector/model/remote-object.html
>+++ b/LayoutTests/inspector/model/remote-object.html
>@@ -91,6 +91,7 @@ function test()
>         {expression: "({a: 1})"},
>         {expression: "({a: 1, b: 0, c: -0})"},
>         {expression: "({a: 1, b: \"string\", c: /regex/, d: Symbol('sym')})"},
>+        {expression: "o = {a:1}; o.b = o; o"}, // Cyclic.
>         {expression: "({a:function a(){}, b:function b(){}, get getter(){}, set setter(v){}})"},
>         {expression: "function Foo() {}; new Foo"},
>         {expression: "var Foo2 = function() {}; new Foo2"},

Flags:

timothy: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 142322: 248712