Show information about Symbols in DevTools console.

Show information about Symbols in DevTools console.

BUG=376194
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174945

[yurys, 2014-05-22 15:25:49]

Can we add a test for this?

https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
Source/core/inspector/InjectedScriptSource.js:1012: return
Symbol.prototype.toString.call(obj);
toString(obj) as Symbol.prototype.toString can be overwritten.

[aandrey, 2014-05-22 15:29:50]

lgtm

https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
Source/core/inspector/InjectedScriptSource.js:1012: return
Symbol.prototype.toString.call(obj);
On 2014/05/22 15:25:49, yurys wrote:
> toString(obj) as Symbol.prototype.toString can be overwritten.

We already use Object.prototype.toString in the code. The toString(obj) will not
work:

$> "" + Symbol()
$> TypeError: Cannot convert a Symbol value to a string

[aandrey, 2014-05-22 15:31:11]

https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
Source/core/inspector/InjectedScriptSource.js:1012: return
Symbol.prototype.toString.call(obj);
wrap it in try-catch

[yurys, 2014-05-22 15:49:36]

https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
Source/core/inspector/InjectedScriptSource.js:1012: return
Symbol.prototype.toString.call(obj);
On 2014/05/22 15:29:50, aandrey wrote:
> On 2014/05/22 15:25:49, yurys wrote:
> > toString(obj) as Symbol.prototype.toString can be overwritten.
> 
> We already use Object.prototype.toString in the code. The toString(obj) will
not
> work:
> 
> $> "" + Symbol()
> $> TypeError: Cannot convert a Symbol value to a string

I'm wondering if the conversion is prohibited by the spec.

[aandrey, 2014-05-22 16:06:57]

maybe we should add a test, but disable it until es6 is supported?
I guess locally we it could be run as:
run_webkit_tests.sh --additional-drt-flag --enable-javascript-harmony

[Alexandra Mikhaylova, 2014-05-22 16:07:59]

https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
Source/core/inspector/InjectedScriptSource.js:1012: return
Symbol.prototype.toString.call(obj);
On 2014/05/22 15:31:12, aandrey wrote:
> wrap it in try-catch

Done, wrapped in a try-catch block.

[aandrey, 2014-05-22 16:10:16]

https://codereview.chromium.org/295633003/diff/20001/Source/core/inspector/In...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/295633003/diff/20001/Source/core/inspector/In...
Source/core/inspector/InjectedScriptSource.js:1011: if (typeof obj === "symbol")
{ ... }

https://codereview.chromium.org/295633003/diff/20001/Source/core/inspector/In...
Source/core/inspector/InjectedScriptSource.js:1013: return
Symbol.prototype.toString.call(obj);
|| "Symbol";

[aandrey, 2014-05-22 16:28:33]

On 2014/05/22 15:49:36, yurys wrote:
>
https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
> File Source/core/inspector/InjectedScriptSource.js (right):
> 
>
https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
> Source/core/inspector/InjectedScriptSource.js:1012: return
> Symbol.prototype.toString.call(obj);
> On 2014/05/22 15:29:50, aandrey wrote:
> > On 2014/05/22 15:25:49, yurys wrote:
> > > toString(obj) as Symbol.prototype.toString can be overwritten.
> > 
> > We already use Object.prototype.toString in the code. The toString(obj) will
> not
> > work:
> > 
> > $> "" + Symbol()
> > $> TypeError: Cannot convert a Symbol value to a string
> 
> I'm wondering if the conversion is prohibited by the spec.

https://people.mozilla.org/~jorendorff/es6-draft.html#sec-tostring

[lushnikov, 2014-05-22 16:34:28]

On 2014/05/22 16:28:33, aandrey wrote:
> On 2014/05/22 15:49:36, yurys wrote:
> >
>
https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
> > File Source/core/inspector/InjectedScriptSource.js (right):
> > 
> >
>
https://codereview.chromium.org/295633003/diff/1/Source/core/inspector/Inject...
> > Source/core/inspector/InjectedScriptSource.js:1012: return
> > Symbol.prototype.toString.call(obj);
> > On 2014/05/22 15:29:50, aandrey wrote:
> > > On 2014/05/22 15:25:49, yurys wrote:
> > > > toString(obj) as Symbol.prototype.toString can be overwritten.
> > > 
> > > We already use Object.prototype.toString in the code. The toString(obj)
will
> > not
> > > work:
> > > 
> > > $> "" + Symbol()
> > > $> TypeError: Cannot convert a Symbol value to a string
> > 
> > I'm wondering if the conversion is prohibited by the spec.
> 
> https://people.mozilla.org/~jorendorff/es6-draft.html#sec-tostring

FYI: the link states that "This is not the official ECMAScript Language
Specification" in the very beginning.

[yurys, 2014-05-26 05:30:54]

What is the status of this CL? Are we landing it?

[Alexandra Mikhaylova, 2014-05-26 10:38:33]

https://codereview.chromium.org/295633003/diff/20001/Source/core/inspector/In...
File Source/core/inspector/InjectedScriptSource.js (right):

https://codereview.chromium.org/295633003/diff/20001/Source/core/inspector/In...
Source/core/inspector/InjectedScriptSource.js:1011: if (typeof obj === "symbol")
On 2014/05/22 16:10:16, aandrey wrote:
> { ... }

Done.

https://codereview.chromium.org/295633003/diff/20001/Source/core/inspector/In...
Source/core/inspector/InjectedScriptSource.js:1013: return
Symbol.prototype.toString.call(obj);
On 2014/05/22 16:10:16, aandrey wrote:
> || "Symbol";

Done.

[aandrey, 2014-05-26 11:01:42]

https://codereview.chromium.org/295633003/diff/40001/LayoutTests/inspector/co...
File LayoutTests/inspector/console/console-format-es6.html (right):

https://codereview.chromium.org/295633003/diff/40001/LayoutTests/inspector/co...
LayoutTests/inspector/console/console-format-es6.html:22: function test()
can you take inspector/console/console-format.html as a base for this one?
you will have smth like this
  globals = [ ...symbols... ];

[Alexandra Mikhaylova, 2014-05-27 12:41:37]

https://codereview.chromium.org/295633003/diff/40001/LayoutTests/inspector/co...
File LayoutTests/inspector/console/console-format-es6.html (right):

https://codereview.chromium.org/295633003/diff/40001/LayoutTests/inspector/co...
LayoutTests/inspector/console/console-format-es6.html:22: function test()
On 2014/05/26 11:01:42, aandrey wrote:
> can you take inspector/console/console-format.html as a base for this one?
> you will have smth like this
>   globals = [ ...symbols... ];

Done.

[aandrey, 2014-05-27 13:19:43]

lgtm

[commit-bot: I haz the power, 2014-05-27 13:19:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/amikhaylova@google.com/295633003/60001

[commit-bot: I haz the power, 2014-05-27 13:20:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-27 13:20:06]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 993.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
41a62f71f76fb48555e4726292b1088b59b5bec9..0266637f6765b664bba153dcc5625ab4b6ca424a
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -993,3 +993,6 @@ crbug.com/375567
web-animations-api/w3c/simple-keyframes.html [ Pass Failure ]
 
 # Temporarily disabled so crrev.com/277583002 can land
 crbug.com/370056 storage/indexeddb/createIndex-after-failure.html [ Skip ]
+
+crbug.com/376194 inspector/console/console-format-es6.html [ Skip ]
+crbug.com/376194 inspector/console/console-format-es6-symbols-error.html [ Skip
]
\ No newline at end of file

[aandrey, 2014-05-27 13:20:58]

please rebase the patch and cq+

[Alexandra Mikhaylova, 2014-05-27 15:25:34]

The CQ bit was checked by amikhaylova@google.com

[commit-bot: I haz the power, 2014-05-27 15:25:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/amikhaylova@google.com/295633003/80001

[commit-bot: I haz the power, 2014-05-27 17:06:06]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6256)

[commit-bot: I haz the power, 2014-05-27 17:21:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-27 17:21:40]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/6260)

[pfeldman, 2014-05-28 07:51:09]

lgtm

[aandrey, 2014-05-28 08:01:24]

The CQ bit was checked by aandrey@chromium.org

[commit-bot: I haz the power, 2014-05-28 08:02:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/amikhaylova@google.com/295633003/80001

[commit-bot: I haz the power, 2014-05-28 08:16:31]

Change committed as 174945

[yurys, 2014-05-28 11:00:09]

This change causes compiler error. How did you pass pre-upload checks?


Total Closure errors: 1

InjectedScriptSource.js and InjectedScriptCanvasModuleSource.js compilation
output:
/sources/chromium/src/third_party/WebKit/Source/core/inspector/InjectedScriptSourceTmp.js:1013:
ERROR - variable Symbol is undeclared
                return Symbol.prototype.toString.call(obj) || "Symbol";
                       ^

1 error(s), 0 warning(s)

[aandrey, 2014-05-28 12:02:53]

On 2014/05/28 11:00:09, yurys wrote:
> This change causes compiler error. How did you pass pre-upload checks?
> 
> 
> Total Closure errors: 1
> 
> InjectedScriptSource.js and InjectedScriptCanvasModuleSource.js compilation
> output:
>
/sources/chromium/src/third_party/WebKit/Source/core/inspector/InjectedScriptSourceTmp.js:1013:
> ERROR - variable Symbol is undeclared
>                 return Symbol.prototype.toString.call(obj) || "Symbol";
>                        ^
> 
> 1 error(s), 0 warning(s)

Fixed in https://codereview.chromium.org/307533006/