Refactor connections into app-actor-front

RESOLVED FIXED in Firefox 28

Status

RESOLVED FIXED
5 years ago
6 months ago

People

(Reporter: jryans, Assigned: jryans)

Tracking

Trunk
Firefox 28

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
DRY up the code by moving client.request calls into app-actor-front.
(Assignee)

Comment 1

5 years ago
Created attachment 825041 [details] [diff] [review]
Refactor app requests into app-actor-front

This centralizes some repeated code into app-actor-front.  Also, calls that rely on the device are guarded with connection checks.
Attachment #825041 - Flags: review?(poirot.alex)
Comment on attachment 825041 [details] [diff] [review]
Refactor app requests into app-actor-front

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

r+ but if, and only if you also add:
 get connected() { return !!this.listTabsResponse; },
to projects.js ;)

::: browser/devtools/app-manager/content/projects.js
@@ +302,4 @@
>  
>    debug: function(button, location) {
> +    if (!this.connected) {
> +      return;

You should also return a rejected promise here.
Attachment #825041 - Flags: review?(poirot.alex) → review+
(Assignee)

Comment 3

5 years ago
(In reply to Alexandre Poirot (:ochameau) from comment #2)
> r+ but if, and only if you also add:
>  get connected() { return !!this.listTabsResponse; },
> to projects.js ;)

Sorry for the confusion, that was in the previous patch from bug 933025, but that hasn't landed, so I understand your concern. :)
Depends on: 933025
(Assignee)

Comment 4

5 years ago
Created attachment 826126 [details] [diff] [review]
Refactor app requests into app-actor-front v2

Carrying over r+ from attachment 825041 [details] [diff] [review].

(In reply to Alexandre Poirot (:ochameau) from comment #2) 
> ::: browser/devtools/app-manager/content/projects.js
> @@ +302,4 @@
> >  
> >    debug: function(button, location) {
> > +    if (!this.connected) {
> > +      return;
> 
> You should also return a rejected promise here.

Added.

Try: https://tbpl.mozilla.org/?tree=Try&rev=6e067dc84e10
Attachment #825041 - Attachment is obsolete: true
Attachment #826126 - Flags: review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/81e276c73ede
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/81e276c73ede
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28

Updated

5 years ago
Whiteboard: [qa-]

Updated

6 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.