Save/load profiles to/from disk

RESOLVED FIXED in Firefox 25

Status

()

Firefox
Developer Tools: Performance Tools (Profiler/Timeline)
P3
normal
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: anton, Assigned: anton)

Tracking

Trunk
Firefox 25
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

5 years ago
It's nice to be able to save your profiles for later. For example, you can profile your app, save the data, optimize the code, then load your data and compare with the new state.
(Assignee)

Updated

5 years ago
Depends on: 828038
(Assignee)

Comment 1

5 years ago
Created attachment 768719 [details] [diff] [review]
UI work for Save/Import functionality
Assignee: nobody → anton
Status: NEW → ASSIGNED
(Assignee)

Comment 2

5 years ago
Created attachment 768732 [details] [diff] [review]
UI work for Save/Import functionality
Attachment #768719 - Attachment is obsolete: true
(Assignee)

Comment 3

5 years ago
Created attachment 773729 [details] [diff] [review]
UI to save/load profiles

I really tried to use sdk/io/fs module but addon SDK has two blocker bugs in the Buffer implementation (will file a bug for that) so I decided not to wait.

Try: https://tbpl.mozilla.org/?tree=Try&rev=858616fd15b5
Attachment #768732 - Attachment is obsolete: true
Attachment #773729 - Flags: review?(rcampbell)
(Assignee)

Comment 4

5 years ago
OK, try is green.
looks good. This is next on my review list.
(Assignee)

Comment 6

5 years ago
Created attachment 775919 [details] [diff] [review]
UI to save/load profiles

Rebased the patch.
Attachment #773729 - Attachment is obsolete: true
Attachment #773729 - Flags: review?(rcampbell)
Attachment #775919 - Flags: review?(rcampbell)
Comment on attachment 775919 [details] [diff] [review]
UI to save/load profiles

Review of attachment 775919 [details] [diff] [review]:
-----------------------------------------------------------------

ok, just a few nits about comments, otherwise it's good.

Looks good and plays nice. I didn't test with importing more than one saved profile though.

::: browser/devtools/profiler/panel.js
@@ +260,5 @@
>  
>      return profile;
>    },
>  
> +  importProfile: function (name, data, opts={}) {

wat? no doc comment??

@@ +480,5 @@
>        panelWin.DebuggerView.Sources.preferredSource = data.uri;
>      }.bind(this));
>    },
>  
> +  openFileDialog: function (opts={}) {

no doc comment?

@@ +503,5 @@
> +
> +    return deferred.promise;
> +  },
> +
> +  saveProfile: function (file, data) {

no doc comment. :(

@@ +516,5 @@
> +    let deferred = promise.defer();
> +    let ch = NetUtil.newChannel(file);
> +    ch.contentType = "application/json";
> +
> +    NetUtil.asyncFetch(ch, (input, status) => {

surprised to see NetUtil here and not OS.File.read(). Not a bad thing though.

::: browser/devtools/profiler/sidebar.js
@@ +36,5 @@
>  }
>  
>  Sidebar.prototype = Heritage.extend(WidgetMethods, {
> +  /**
> +   *

nit: extra empty line.
Attachment #775919 - Flags: review?(rcampbell) → review+
(Assignee)

Comment 8

5 years ago
Created attachment 776758 [details] [diff] [review]
Ability to save/load profiles

Fixed nits. Carrying over r+.
Attachment #775919 - Attachment is obsolete: true
Attachment #776758 - Flags: review+
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/integration/fx-team/rev/cca017679bfa
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/cca017679bfa
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 25
(Assignee)

Updated

4 years ago
Duplicate of this bug: 899713
You need to log in before you can comment on or make changes to this bug.