Closed
Bug 795979
Opened 12 years ago
Closed 11 years ago
RDP, new packet type: getObjectsProperties
Categories
(DevTools :: Debugger, defect, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 26
People
(Reporter: Honza, Assigned: mhordecki)
References
Details
Attachments
(1 file, 4 obsolete files)
6.89 KB,
patch
|
past
:
review+
|
Details | Diff | Splinter Review |
In order to reduce number of client server round-trips, it would be useful to get properties for more objects at once - getObjectsProperties. (there is currently only getObjectPropeties packet type). Honza
Updated•12 years ago
|
Priority: -- → P3
Assignee | ||
Comment 1•11 years ago
|
||
It looks like this packet type doesn't exist anymore. I'm closing the issue for now.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID
Comment 2•11 years ago
|
||
This packet type has never existed, this bug is a request to implement it.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Assignee | ||
Comment 3•11 years ago
|
||
I was talking about the getObjectProperties one - there is no mention of it in the code, at least I can't find any.
Comment 4•11 years ago
|
||
That is true, Honza was referring to the DebuggerClient's getPrototypeAndProperties method that sends a prototypeAndProperties protocol request.
Reporter | ||
Comment 5•11 years ago
|
||
(In reply to Panos Astithas [:past] from comment #4) > That is true, Honza was referring to the DebuggerClient's > getPrototypeAndProperties method that sends a prototypeAndProperties > protocol request. That's correct Honza
Assignee | ||
Comment 6•11 years ago
|
||
As far as implementation goes, I thinking about adding a method to ThreadActor that takes a list of actor names. What do you guys think? I'm just getting my feet wet in devtools.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → mhordecki
Comment 7•11 years ago
|
||
Things we need: * Proposed spec (pull request to https://github.com/jimblandy/DebuggerDocs would be nice) * Handler on the server (ThreadActor) * Requester on the client (ThreadClient, toolkit/devtools/client/dbg-client.js)
Assignee | ||
Comment 8•11 years ago
|
||
Assignee | ||
Comment 9•11 years ago
|
||
Also, the corresponding pull request can be found at https://github.com/jimblandy/DebuggerDocs/pull/12 .
Assignee | ||
Updated•11 years ago
|
Attachment #776774 -
Flags: review?(past)
Comment 10•11 years ago
|
||
Comment on attachment 776774 [details] [diff] [review] New packet type implementation. Review of attachment 776774 [details] [diff] [review]: ----------------------------------------------------------------- Did a quick pass, but I'll play with it more tomorrow. ::: toolkit/devtools/client/dbg-client.jsm @@ +1543,5 @@ > + getPrototypesAndProperties: DebuggerClient.requester({ > + type: "prototypesAndProperties", > + actors: args(0) > + }, { > + telemetry: "PROTOTYPEANDPROPERTIES" Typo: PROTOTYPESANDPROPERTIES (you are missing an 'S'). You also need to add this to Histograms.json (2 entries like the rest of the requests).
Comment 11•11 years ago
|
||
Jim should also sign off on the protocol spec change.
Status: REOPENED → ASSIGNED
Flags: needinfo?(jimb)
Assignee | ||
Comment 12•11 years ago
|
||
Attachment #776774 -
Attachment is obsolete: true
Attachment #776774 -
Flags: review?(past)
Attachment #777488 -
Flags: review?(past)
Comment 13•11 years ago
|
||
Comment on attachment 777488 [details] [diff] [review] Patch v2 Review of attachment 777488 [details] [diff] [review]: ----------------------------------------------------------------- Just a few nits, but this looks good, if Jim approves the protocol spec change. Try push: https://tbpl.mozilla.org/?tree=Try&rev=956fd0f47891 ::: toolkit/devtools/client/dbg-client.jsm @@ +1537,5 @@ > + /** > + * Request the prototype and own properties of mutlipleObjects. > + * > + * @param aOnResponse function Called with the request's response. > + * @param actors [string] List of actor ID of the queried objects. Nit: move the description beneath the type info, as we do in other comments in this file. ::: toolkit/devtools/server/actors/script.js @@ +1351,5 @@ > + if (!actor) { > + return { from: this.actorID, > + error: "noSuchActor" }; > + } > + let handler = actor.requestTypes["prototypeAndProperties"]; actor.onPrototypeAndProperties would be clearer. ::: toolkit/devtools/server/tests/unit/test_objectgrips-08.js @@ +4,5 @@ > +var gDebuggee; > +var gClient; > +var gThreadClient; > + > +function run_test() Add a brief comment explaining the purpose of the test please.
Attachment #777488 -
Flags: review?(past) → review+
Comment 14•11 years ago
|
||
(In reply to Panos Astithas [:past] from comment #11) > Jim should also sign off on the protocol spec change. Looks good; merged the pull request.
Flags: needinfo?(jimb)
Assignee | ||
Comment 15•11 years ago
|
||
Attached a new patch with changes proposed by Panos.
Attachment #777488 -
Attachment is obsolete: true
Attachment #786522 -
Flags: review?(past)
Assignee | ||
Comment 16•11 years ago
|
||
Submitted a wrong thing.
Attachment #786522 -
Attachment is obsolete: true
Attachment #786522 -
Flags: review?(past)
Attachment #786527 -
Flags: review?(past)
Comment 17•11 years ago
|
||
Try push: https://tbpl.mozilla.org/?tree=Try&rev=6f63082fdd42
Comment 18•11 years ago
|
||
Comment on attachment 786527 [details] [diff] [review] Patch v3.1 Review of attachment 786527 [details] [diff] [review]: ----------------------------------------------------------------- Cancelling review until test failures are taken care of.
Attachment #786527 -
Flags: review?(past)
Assignee | ||
Comment 19•11 years ago
|
||
Test failures should be corrected now. tbpl: https://tbpl.mozilla.org/?tree=Try&rev=5d67a721cf67
Attachment #786527 -
Attachment is obsolete: true
Attachment #791052 -
Flags: review?(past)
Updated•11 years ago
|
Attachment #791052 -
Flags: review?(past) → review+
Updated•11 years ago
|
Whiteboard: [land-in-fx-team]
Comment 20•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/e039f5fc18d0
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Comment 21•11 years ago
|
||
Was backed out (https://hg.mozilla.org/integration/fx-team/rev/60610dbd2cf0) on suspicion of causing orange, but this was actually due to another landing, so have relanded: remote: https://hg.mozilla.org/integration/fx-team/rev/092dd07390a1
Comment 22•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/092dd07390a1
Status: ASSIGNED → RESOLVED
Closed: 11 years ago → 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 26
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•