DevTools: include traces when exporting console log via 'Save as'

DevTools: include traces when exporting console log via 'Save as'

This CL modifies the export behavior when clicking 'Save as...' from the context menu in the
console. Exported logs now include the entire formatted message, including the anchor location
(source file : line number) and contents of stack traces.

Screenshot: http://imgur.com/a/DVKkW

BUG=505190, 505177
Committed: https://crrev.com/78ecc7501f1856b791cf5ff65811da03871f72f1
Cr-Commit-Position: refs/heads/master@{#419218}

[luoe, 2016-08-24 00:44:22]

Description was changed from

==========
reset result other tests


export console should format stack traces

BUG=505190, 505177
==========

to

==========
DevTools: use formattedMessage when exporting console log via 'Save as'

This CL modifies the export behavior when clicking 'Save as...' from the context
menu in the
console. Exported logs now include the entire formatted message, including the
anchor location
(source file : line number) and contents of stack traces.

BUG=505190, 505177
==========

[luoe, 2016-08-24 00:46:46]

luoe@chromium.org changed reviewers:
+ lushnikov@chromium.org

[luoe, 2016-08-24 00:46:47]

Note:
- this properly formats async stack traces
- doesn't automatically expand objects (or anything with an arrow, besides
traces)
- long urls that were truncated still contain an ellipsis in the log file.  This
will be fixed in another CL :(

The only test that was adjusted is
inspector/tracing/timeline-misc/timeline-event-causes.html

The rest were checked and reset-result'd.

[dgozman, 2016-08-24 18:57:08]

dgozman@chromium.org changed reviewers:
+ dgozman@chromium.org

[dgozman, 2016-08-24 18:57:09]

https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/components/DOMPresentationUtils.js
(right):

https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/DOMPresentationUtils.js:238:
row.createChild("td").textContent = "\n";
Does this change things visually? Screenshot?

[dgozman, 2016-08-24 18:58:00]

https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js (left):

https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:660:
lines.push(message.searchableElement().deepTextContent());
Do we need searchableElement anymore? Shouldn't search work on the same data as
export?

[luoe, 2016-08-24 20:31:01]

https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/components/DOMPresentationUtils.js
(right):

https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/components/DOMPresentationUtils.js:238:
row.createChild("td").textContent = "\n";
On 2016/08/24 18:57:09, dgozman wrote:
> Does this change things visually? Screenshot?

No visual change.  Screenshot (normal trace on top, async trace on bottom):
http://imgur.com/a/DVKkW

1 Test expectations changed:
inspector/sources/debugger-async/async-callstack-in-console-expected.txt

https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js (left):

https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:660:
lines.push(message.searchableElement().deepTextContent());
On 2016/08/24 18:58:00, dgozman wrote:
> Do we need searchableElement anymore? Shouldn't search work on the same data
as
> export?

The only reason I can think might be users who don't want to search the anchor
location (source file:line).

[luoe, 2016-08-24 20:31:12]

Description was changed from

==========
DevTools: use formattedMessage when exporting console log via 'Save as'

This CL modifies the export behavior when clicking 'Save as...' from the context
menu in the
console. Exported logs now include the entire formatted message, including the
anchor location
(source file : line number) and contents of stack traces.

BUG=505190, 505177
==========

to

==========
DevTools: use formattedMessage when exporting console log via 'Save as'

This CL modifies the export behavior when clicking 'Save as...' from the context
menu in the
console. Exported logs now include the entire formatted message, including the
anchor location
(source file : line number) and contents of stack traces.

Screenshot: http://imgur.com/a/DVKkW

BUG=505190, 505177
==========

[luoe, 2016-08-30 17:44:57]

On 2016/08/24 20:31:01, luoe wrote:
>
https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
> File
>
third_party/WebKit/Source/devtools/front_end/components/DOMPresentationUtils.js
> (right):
> 
>
https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
>
third_party/WebKit/Source/devtools/front_end/components/DOMPresentationUtils.js:238:
> row.createChild("td").textContent = "\n";
> On 2016/08/24 18:57:09, dgozman wrote:
> > Does this change things visually? Screenshot?
> 
> No visual change.  Screenshot (normal trace on top, async trace on bottom):
> http://imgur.com/a/DVKkW
> 
> 1 Test expectations changed:
> inspector/sources/debugger-async/async-callstack-in-console-expected.txt
> 
>
https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
> File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(left):
> 
>
https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
> third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:660:
> lines.push(message.searchableElement().deepTextContent());
> On 2016/08/24 18:58:00, dgozman wrote:
> > Do we need searchableElement anymore? Shouldn't search work on the same data
> as
> > export?
> 
> The only reason I can think might be users who don't want to search the anchor
> location (source file:line).

Gentle ping

[luoe, 2016-08-30 18:52:15]

On 2016/08/30 17:44:57, luoe wrote:
> On 2016/08/24 20:31:01, luoe wrote:
> >
>
https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
> > File
> >
>
third_party/WebKit/Source/devtools/front_end/components/DOMPresentationUtils.js
> > (right):
> > 
> >
>
https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
> >
>
third_party/WebKit/Source/devtools/front_end/components/DOMPresentationUtils.js:238:
> > row.createChild("td").textContent = "\n";
> > On 2016/08/24 18:57:09, dgozman wrote:
> > > Does this change things visually? Screenshot?
> > 
> > No visual change.  Screenshot (normal trace on top, async trace on bottom):
> > http://imgur.com/a/DVKkW
> > 
> > 1 Test expectations changed:
> > inspector/sources/debugger-async/async-callstack-in-console-expected.txt
> > 
> >
>
https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
> > File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
> (left):
> > 
> >
>
https://codereview.chromium.org/2270033003/diff/1/third_party/WebKit/Source/d...
> > third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:660:
> > lines.push(message.searchableElement().deepTextContent());
> > On 2016/08/24 18:58:00, dgozman wrote:
> > > Do we need searchableElement anymore? Shouldn't search work on the same
data
> > as
> > > export?
> > 
> > The only reason I can think might be users who don't want to search the
anchor
> > location (source file:line).
> 
> Gentle ping

On reconsideration, I think you're right Dmitry.  We already 'search' the
anchors when we apply filters, so searching via Ctrl-F should do the same.

[luoe, 2016-08-30 19:31:47]

Replaced searchableElement with formattedMessage.  Previously, searchableElement
existed to prevent stack traces and anchors from being searchable.  Since we
already 'search' those two when filtering, we can remove searchableElement and
make behavior more consistent.

Ptal

[dgozman, 2016-08-30 20:26:31]

Thanks, I defer to lushnikov@ here.

[lushnikov, 2016-08-31 00:08:12]

https://codereview.chromium.org/2270033003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2270033003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:1253:
contentElement: function()
can we switch to contentElement altogether? looks like there's no reason why it
shouldn't be possible

[luoe, 2016-08-31 17:52:02]

Notes:
- Filtering and searching now includes timestamps
- I changed CVM:toString() so that it uses contentElement, but I can't find
where this is called.  No tests broke, so it might not be used, but I'm not
confident enough to remove it.
- Remaining uses of formattedMessage will be removed completely from CVM.js in a
future CL (the one that breaks out formatting logic)

https://codereview.chromium.org/2270033003/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2270033003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:1253:
contentElement: function()
On 2016/08/31 00:08:11, lushnikov wrote:
> can we switch to contentElement altogether? looks like there's no reason why
it
> shouldn't be possible

Done.

[lushnikov, 2016-09-06 19:30:29]

https://codereview.chromium.org/2270033003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2270033003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:660:
lines.push(message.formattedMessage().deepTextContent());
why do we still use formatMessage?

[luoe, 2016-09-06 21:50:29]

Removed formattedMessage().  ContentElement() still uses the helper
_formatMessage().  this._anchorElement and this._messageElement no longer need
to be stored on the instance.

https://codereview.chromium.org/2270033003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2270033003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:660:
lines.push(message.formattedMessage().deepTextContent());
On 2016/09/06 19:30:29, lushnikov wrote:
> why do we still use formatMessage?

My bad, I didn't remove formattedMessage() uses in ConsoleView.  This should be
fixed in patch 6

[luoe, 2016-09-09 00:44:56]

Gentle ping

[luoe, 2016-09-12 20:58:36]

Description was changed from

==========
DevTools: use formattedMessage when exporting console log via 'Save as'

This CL modifies the export behavior when clicking 'Save as...' from the context
menu in the
console. Exported logs now include the entire formatted message, including the
anchor location
(source file : line number) and contents of stack traces.

Screenshot: http://imgur.com/a/DVKkW

BUG=505190, 505177
==========

to

==========
DevTools: include traces when exporting console log via 'Save as'

This CL modifies the export behavior when clicking 'Save as...' from the context
menu in the
console. Exported logs now include the entire formatted message, including the
anchor location
(source file : line number) and contents of stack traces.

Screenshot: http://imgur.com/a/DVKkW

BUG=505190, 505177
==========

[luoe, 2016-09-12 22:20:59]

After an offline discussion, it seems that we should put refactoring into future
CLs.  Please review this CL as CL1

CL1 (this CL):
- export now uses formattedMessage (gets traces, anchors)
- line breaks in traces are correct when exporting/copy-pasting

Tentative future CL plan:
CL2 - replace searchableElement with formattedMessage
CL3 - replace formattedMessage with contentElement


We have a lot of information that isn't captured in 'Save as': the repeat count
(x10), message source (console, network, worker), type (warn, dirxml), level
(log, warning, debug, error).  If 'Save as' called toString(), we could include
these extras at the cost of cluttering exported logs.  WDYT?

[lushnikov, 2016-09-14 01:25:00]

lgtm

lgtm

[luoe, 2016-09-14 23:01:12]

Post-lgtm, I've added a line and verified that repeated messages are now shown
in the saved log file.

[luoe, 2016-09-14 23:01:24]

The CQ bit was checked by luoe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-14 23:02:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[luoe, 2016-09-14 23:08:15]

The CQ bit was checked by luoe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-14 23:08:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-15 01:35:21]

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

[commit-bot: I haz the power, 2016-09-15 01:35:21]

Dry run: This issue passed the CQ dry run.

[luoe, 2016-09-15 23:21:52]

The CQ bit was checked by luoe@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-15 23:22:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 02:14:45]

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

[commit-bot: I haz the power, 2016-09-16 02:14:46]

Dry run: This issue passed the CQ dry run.

[luoe, 2016-09-16 18:08:17]

Removed post-lgtm changes, moved them to a future cl.

[luoe, 2016-09-16 18:08:27]

The CQ bit was checked by luoe@chromium.org

[luoe, 2016-09-16 18:08:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
lushnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2270033003/#ps200001
(title: "take out post-lgtm repeat logic")

[commit-bot: I haz the power, 2016-09-16 18:08:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 18:15:20]

Description was changed from

==========
DevTools: include traces when exporting console log via 'Save as'

This CL modifies the export behavior when clicking 'Save as...' from the context
menu in the
console. Exported logs now include the entire formatted message, including the
anchor location
(source file : line number) and contents of stack traces.

Screenshot: http://imgur.com/a/DVKkW

BUG=505190, 505177
==========

to

==========
DevTools: include traces when exporting console log via 'Save as'

This CL modifies the export behavior when clicking 'Save as...' from the context
menu in the
console. Exported logs now include the entire formatted message, including the
anchor location
(source file : line number) and contents of stack traces.

Screenshot: http://imgur.com/a/DVKkW

BUG=505190, 505177
==========

[commit-bot: I haz the power, 2016-09-16 18:15:21]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-09-16 18:18:32]

Description was changed from

==========
DevTools: include traces when exporting console log via 'Save as'

This CL modifies the export behavior when clicking 'Save as...' from the context
menu in the
console. Exported logs now include the entire formatted message, including the
anchor location
(source file : line number) and contents of stack traces.

Screenshot: http://imgur.com/a/DVKkW

BUG=505190, 505177
==========

to

==========
DevTools: include traces when exporting console log via 'Save as'

This CL modifies the export behavior when clicking 'Save as...' from the context
menu in the
console. Exported logs now include the entire formatted message, including the
anchor location
(source file : line number) and contents of stack traces.

Screenshot: http://imgur.com/a/DVKkW

BUG=505190, 505177

Committed: https://crrev.com/78ecc7501f1856b791cf5ff65811da03871f72f1
Cr-Commit-Position: refs/heads/master@{#419218}
==========

[commit-bot: I haz the power, 2016-09-16 18:18:34]

Patchset 11 (id:??) landed as
https://crrev.com/78ecc7501f1856b791cf5ff65811da03871f72f1
Cr-Commit-Position: refs/heads/master@{#419218}