Support save/restore in about:profiler

Support save/restore in about:profiler

BUG=107265
TEST=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=118179

[rlarocque, 2011-12-12 22:25:45]

I'm still working on this, though progress has been slow due to a lack of time
and my inexperience with JavaScript.  

TL;DR: This is a preview of my attempt to implement about:profiler save/restore.
 I'm not looking to commit this yet.  Feel free to review now or wait until the
feature is more fully baked.  

=======

This is the Javascript portion of the change.  Saving and restoring from the
about:profiler UI is supported.  Snapshots are supported, too.  

I intend to make some minor additions to the log file format once I implement
support for writing it from C++.  

I tried to implement restore by emulating the process by which snapshots are
taken from the live browser.  Hopefully this approach will lead to less code,
since we only need to implement one set of functions for loading snapshot data
into the UI.  The save function and file format are also built around this
concept; they make it easy to "replay" the sequence of events that lead to a
specific about:profiler state.

[eroman, 2011-12-12 23:14:28]

overall approach looks good, let me know when you are ready for a full review.

http://codereview.chromium.org/8918018/diff/1/chrome/browser/resources/profil...
File chrome/browser/resources/profiler.js (right):

http://codereview.chromium.org/8918018/diff/1/chrome/browser/resources/profil...
chrome/browser/resources/profiler.js:1761: // 'browser_version': '' // FIXME
fetch this somehow
you could include the user agent (navigator.userAgent) to give some version
information.

http://codereview.chromium.org/8918018/diff/1/chrome/browser/resources/profil...
chrome/browser/resources/profiler.js:1762: 'snapshots': snapshots
I suggest putting a version field in the format too, since things invariably
change, and having a required version number specifier can make it easier to
tell.

http://codereview.chromium.org/8918018/diff/1/chrome/browser/resources/profil...
chrome/browser/resources/profiler.js:1803: while (tbody.hasChildNodes()) {
A more typical way to empty a node is:

tbody.innerHTML = '';

[eroman, 2011-12-13 03:51:02]

Also note that when you sync your client, those files have moved into the
"profiler/*" subdirectory.

[rlarocque, 2012-01-06 23:27:00]

Sorry about the delay.  I haven't had much time to work on this lately.  

I wanted to wait until I had implemented the JSON-output code in C++ before
opening this up to a more thorough review.  I figured that I might run into
issues on the C++ side that would force me to modify the output format.  

I did have to change the timestamp to be in seconds instead of milliseconds,
since our C++ JSON routines don't support a "long" type.  Other than that, there
were no changes required so this latest diff should be very similar to the old
one.  

Review for C++ profiler output is here: http://codereview.chromium.org/9125015/

PTAL.

[eroman, 2012-01-07 03:22:54]

Not sure if I will get around to reviewing this today (so may not respond until
monday).

Cheers.

[eroman, 2012-01-10 02:40:41]

LGTM.

> I did have to change the timestamp to be in seconds instead 
> of milliseconds, since our C++ JSON routines don't support a 
> "long" type. 

I have worked around this in the past by outputting int64 from C++ as strings.
(Then in java call parseInt() to convert it back to a number).

http://codereview.chromium.org/8918018/diff/5001/chrome/browser/resources/pro...
File chrome/browser/resources/profiler/profiler.js (right):

http://codereview.chromium.org/8918018/diff/5001/chrome/browser/resources/pro...
chrome/browser/resources/profiler/profiler.js:1799: }
nit: terminate statements with a semicolon

http://codereview.chromium.org/8918018/diff/5001/chrome/browser/resources/pro...
chrome/browser/resources/profiler/profiler.js:1834: throw new Error ('File
contains no data');
nit: remove space after Error

[rlarocque, 2012-01-13 23:08:50]

Thanks for the review.

I'll try to land this later today or early next week.

http://codereview.chromium.org/8918018/diff/5001/chrome/browser/resources/pro...
File chrome/browser/resources/profiler/profiler.js (right):

http://codereview.chromium.org/8918018/diff/5001/chrome/browser/resources/pro...
chrome/browser/resources/profiler/profiler.js:1799: }
On 2012/01/10 02:40:41, eroman wrote:
> nit: terminate statements with a semicolon

Done.

http://codereview.chromium.org/8918018/diff/5001/chrome/browser/resources/pro...
chrome/browser/resources/profiler/profiler.js:1834: throw new Error ('File
contains no data');
On 2012/01/10 02:40:41, eroman wrote:
> nit: remove space after Error

Done.

[commit-bot: I haz the power, 2012-01-18 21:13:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rlarocque@chromium.org/8918018/12001

[commit-bot: I haz the power, 2012-01-18 23:01:42]

Change committed as 118179