Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(458)

Issue 7330027: Add an example for using the URLLoader in streaming mode. This also avoids the (Closed)

Created:
9 years, 5 months ago by brettw
Modified:
9 years, 5 months ago
CC:
chromium-reviews
Visibility:
Public.

Description

Add an example for using the URLLoader in streaming mode. This also avoids the infinite recursion bug of the old example (and actually compiles, the old example had a lot of typos). BUG=69203 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=92196

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+262 lines, -58 lines) Patch
M ppapi/cpp/url_loader.h View 1 chunk +1 line, -58 lines 0 comments Download
A ppapi/examples/url_loader/README_chrome_developer.txt View 1 chunk +22 lines, -0 lines 0 comments Download
A ppapi/examples/url_loader/fetched_content.html View 1 chunk +4 lines, -0 lines 0 comments Download
A ppapi/examples/url_loader/streaming.cc View 1 chunk +172 lines, -0 lines 4 comments Download
A ppapi/examples/url_loader/url_loader.html View 1 chunk +53 lines, -0 lines 1 comment Download
M ppapi/ppapi_tests.gypi View 1 chunk +10 lines, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
brettw
9 years, 5 months ago (2011-07-08 23:13:18 UTC) #1
darin (slow to review)
9 years, 5 months ago (2011-07-09 04:01:27 UTC) #2
LGTM w/ nits:

http://codereview.chromium.org/7330027/diff/1/ppapi/examples/url_loader/strea...
File ppapi/examples/url_loader/streaming.cc (right):

http://codereview.chromium.org/7330027/diff/1/ppapi/examples/url_loader/strea...
ppapi/examples/url_loader/streaming.cc:13: // Stream-to-file is simpler, but
won't allow you to get partial data as it
This is not exactly true.  You can open the file for reading, and read from it,
while the download is still underway.  At least that was what I had in mind when
I created the API.

http://codereview.chromium.org/7330027/diff/1/ppapi/examples/url_loader/strea...
ppapi/examples/url_loader/streaming.cc:24: const int kBufSize = 1024;
nit: 'static'

http://codereview.chromium.org/7330027/diff/1/ppapi/examples/url_loader/strea...
ppapi/examples/url_loader/streaming.cc:110: // case of a synchronous return, we
need to be sure to delete our callback
you say "be sure to delete our callback", but you don't delete it down below. 
instead, you Run it, which is what causes memory to be released.

http://codereview.chromium.org/7330027/diff/1/ppapi/examples/url_loader/strea...
ppapi/examples/url_loader/streaming.cc:125: // Either we reached the end of the
stream or there was an error. We want
It might help to say:  "we reached the end of the stream (result == 0) or..."

http://codereview.chromium.org/7330027/diff/1/ppapi/examples/url_loader/url_l...
File ppapi/examples/url_loader/url_loader.html (right):

http://codereview.chromium.org/7330027/diff/1/ppapi/examples/url_loader/url_l...
ppapi/examples/url_loader/url_loader.html:24:
document.addEventListener("DOMContentLoaded", AddListener, false);
note:  you could also just put this <script> block at the end of the document
(or at least after the <object> tag).  then you wouldn't need to register for
the DOMContentLoaded event.

Powered by Google App Engine
This is Rietveld 408576698