On Mon, Dec 15, 2008 at 8:17 AM, Andrei Popescu <[email protected]> wrote: > > ======================================================================== > http://mondrian.corp.google.com/file/9387431///depot/googleclient/gears/opensource/gears/localserver/android/http_request_android.cc?a=1 > File > //depot/googleclient/gears/opensource/gears/localserver/android/http_request_android.cc > (snapshot 1) > ------------------------------------ > Line 523: static_cast<int64_t>(0)); > But surely a negative length is illegal. Should we not assert here, instead? > ------------------------------------ > Line 989: bytes_sent_ = pos; > On 12 Dec, michaeln wrote: >> maybe set bytes_send_ outside prior to the if test? > > I think we can just do away with bytes_sent_ altogether and just pass pos to > the > functor. > > Also instead of pumping messages like this, could we just look at how big the > data is and send an update say every 10%? It would do away with all the need > for > locking and keeping state about whether we've sent functors, etc.
May be ok for the upload side, but on the download side this could interfere with some BrowserChannel'ish use cases (hanging XHR GETs that recieve may small structured messages via a single request). Also would make this impl's behavior different from other Gears ports. > ======================================================================== > http://mondrian.corp.google.com/file/9387431///depot/googleclient/gears/opensource/gears/localserver/android/http_request_android.h?a=1 > File > //depot/googleclient/gears/opensource/gears/localserver/android/http_request_android.h > (snapshot 1) > ------------------------------------ > Line 238: class ProgressFunctor : public AsyncFunctor { > All other platforms use a ProgressEvent. Why are we not using that? It seems > we > are replicating existing functionality. > ------------------------------------ > Line 357: void OnProgress(); > All other platforms have an OnUploadProgress() that is called by the > ProgressEvent. > ------------------------------------ > Line 431: // The total number of bytes that will be by the child thread, or 0 > Are the two member variables above needed at all? Can't the functor itself > store > this info? > ======================================================================== > > -- > To respond, reply to this email or visit > http://mondrian.corp.google.com/9387431 >
