Closed Bug 581560 Opened 13 years ago Closed 7 years ago

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

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: Gavin, Assigned: Mardak)

References

Details

Attachments

(1 file, 4 obsolete files)

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
});
Attached patch WIP (obsolete) — Splinter Review
Will take a look at porting some of the tests from test_resource.js.
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attached patch updated WIP (obsolete) — Splinter Review
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
Assignee: gavin.sharp → nobody
Attached patch v1 (obsolete) — Splinter Review
Assignee: nobody → edilee
Attachment #460310 - Attachment is obsolete: true
Attachment #467503 - Flags: review?(cbiesinger)
Attached patch v1.1 (obsolete) — Splinter Review
Oops, thought I removed the ignore caching bits.
Attachment #467503 - Attachment is obsolete: true
Attachment #467504 - Flags: review?(cbiesinger)
Attachment #467503 - Flags: review?(cbiesinger)
Blocks: 571409
Attached patch v2Splinter Review
Dan added the ability to set load flags.
Attachment #467504 - Attachment is obsolete: true
Attachment #469307 - Flags: review?(cbiesinger)
Attachment #467504 - Flags: review?(cbiesinger)
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
Closed: 7 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.