Add async version of Labs' resource.js next to NetUtil.jsm

RESOLVED INCOMPLETE

Status

()

Core
Networking
RESOLVED INCOMPLETE
8 years ago
2 years ago

People

(Reporter: Gavin, Assigned: Mardak)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

It exposes a nice API for simple HTTP (only, for the moment) requests:

var resource = new Resource("http://example.com");
resource.get(function (result) {
  print(result); // string containing HTML source
  print(result.status); // 0 (nsresult)
  print(result.success); // true (boolean)
);

resource.setHeader("Content-Type", "application/x-www-form-urlencoded");
resource.post("foo=bar", function (result) {
  // Same as above
});
Created attachment 459914 [details] [diff] [review]
WIP

Will take a look at porting some of the tests from test_resource.js.
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Created attachment 460310 [details] [diff] [review]
updated WIP

Some minor changes. Still needs tests based on http://mxr.mozilla.org/mozilla-central/source/services/sync/tests/unit/test_resource.js .
Attachment #459914 - Attachment is obsolete: true
Blocks: 571413
Assignee: gavin.sharp → nobody
(Assignee)

Comment 3

8 years ago
Created attachment 467503 [details] [diff] [review]
v1
Assignee: nobody → edilee
Attachment #460310 - Attachment is obsolete: true
Attachment #467503 - Flags: review?(cbiesinger)
(Assignee)

Comment 4

8 years ago
Created attachment 467504 [details] [diff] [review]
v1.1

Oops, thought I removed the ignore caching bits.
Attachment #467503 - Attachment is obsolete: true
Attachment #467504 - Flags: review?(cbiesinger)
Attachment #467503 - Flags: review?(cbiesinger)
(Assignee)

Updated

8 years ago
Blocks: 571409
(Assignee)

Comment 5

8 years ago
Created attachment 469307 [details] [diff] [review]
v2

Dan added the ability to set load flags.
Attachment #467504 - Attachment is obsolete: true
Attachment #469307 - Flags: review?(cbiesinger)
Attachment #467504 - Flags: review?(cbiesinger)
Blocks: 600059
I can probably do this review if bz or biesi are OK with it.  I tend to do the reviews of NetUtil.jsm and bz or biesi do the sr.
FWIW, I've filed (and implemented) bug 603301 to provide an async version of Resource in services/sync.

It differs from the implementation in this bug in that it still contains the Firefox Sync specific additions. It also has a different signature for the callback, adopting the node.js convention of passing any errors/exceptions into the callback as the first argument and results as second and further arguments. I personally think this makes for a clearer API than sticking attributes on a String object. I would welcome if the netwerk version would adopt the same signature, but it's cool if not. For now we Sync needs to stick with its own implementation anyway, at least as long as we still continue to support Firefox 3.5/3.6.
No longer blocks: 600059
We've found that the Resource implementation, even the async one as described in comment 7, is lacking some crucial features which is mostly due to poor API design. Bug 669547 explains the details and also has a new alternative implementation called RESTRequest (which is inspired by Resource a lot.)

I hereby propose RESTRequest to be uplifted to either network or toolkit (e.g. into mozapps/shared), rather than Labs' Resource. Sync is definitely going to abandon Resource in favour of RESTRequest.
Comment on attachment 469307 [details] [diff] [review]
v2

r- given comment 8
Attachment #469307 - Flags: review?(cbiesinger) → review-
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.