========================================================================
http://mondrian.corp.google.com/file/10462049///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 612: void HttpRequestAndroid::SetState(State state) {
Right now the functors change the state by setting instance_->state_ directly.
But we also have the SetState() method, so it's not clear when one should use
SetState and when one should just set state_ directly.
How about making SetState() return a bool instead? Return true if the state was
changed successfully and false otherwise. In the functors we can then look at
the return value and decide whether to invoke HandleStateMachine() or not. In
SetState() we check which thread we are on and then check the conditions that
apply to each thread: was_aborted_ must be false for the state to be changed on
the main thread and !(was_aborted_ && (state != STATE_CHILD_ABORT)) for the
state to be changed on the child thread.
------------------------------------
Line 643: // Note that every Child states can also transition to CHILD_ABORT
Child state
------------------------------------
Line 644: // in case of error, and that CHILD_ABORT transition to MAIN_COMPLETE:
CHILD_ABORT can only transition to MAIN_COMPLETE.
------------------------------------
Line 659: // call interrupt -- we are already past the normal request execution.
I don't think it matters if was_aborted_ is true or false. At that point there
are no blocking operations on the Java side. If there were, then the child
thread would be blocked so it could not enter the ABORT state and call
interrupt. Perhaps we want leave the call to interrupt() in the Abort() method,
as before?
------------------------------------
Line 665: for (;;) {
I wonder if we can eliminate this loop altogether. If the child thread sends
functors to itself, the state machine will be automatically handled for us by
the thread's message loop.
------------------------------------
Line 799: GetMethod(JAVA_METHOD_INTERRUPT));
Move this to Abort()?
------------------------------------
Line 1420: // We simply switch the child to the STATE_CHILD_ABORT state.
Note that this won't abort the child immediately. Rather it will wait until the
child finishes processing its current state (this can be receiving 1GB of data
so it'll take a while) and then it will proceed directly to the ABORT state and
terminate. Leave the interrupt call after SwitchToChildThreadState?
========================================================================
http://mondrian.corp.google.com/file/10462049///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 145: instance_->Ref();
Say that we need to Ref() since we need to keep the instance alive while the
thread is running.
------------------------------------
Line 150: instance_->Unref();
assert(instance_->GetRef() == 1) ?
------------------------------------
Line 226: // STATE_MAIN_COMPLETE.
This last sentence doesn't really make sense to me..?
------------------------------------
Line 233: // invoked final processing.
This comment is out of date? The can never delete the instance. Assert that the
refcount is > 1 (i.e. instance->GetRef() > 1).
========================================================================
--
To respond, reply to this email or visit
http://mondrian.corp.google.com/10462049
