Fix MessageReader not to invoke message callback from done callback.

Fix MessageReader not to invoke message callback from done callback.

In the current implementation MessageReader may read a message while processing
done callback for the previous message. The message handler may not expect that
which may lead to crashes. E.g. ChannelMultiplex was written such that
OnPacketReceived() is not reentrant.

BUG=148512
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=156490

[Wez, 2012-09-13 01:00:09]

http://codereview.chromium.org/10919259/diff/5/remoting/protocol/message_read...
File remoting/protocol/message_reader.cc (right):

http://codereview.chromium.org/10919259/diff/5/remoting/protocol/message_read...
remoting/protocol/message_reader.cc:129: if (!read_pending_)
nit: DoRead() already checks |read_pending_| so you don't need to check it here.

http://codereview.chromium.org/10919259/diff/5/remoting/protocol/message_read...
remoting/protocol/message_reader.cc:130: DoRead(true);
Wouldn't it be simpler to have a member that you set in RunCallback around the
|message_received_callback_.Run()|, and which you clear afterwards, that
prevents the DoRead() from within OnMessageDone()?

[Sergey Ulanov, 2012-09-13 01:40:21]

http://codereview.chromium.org/10919259/diff/5/remoting/protocol/message_read...
File remoting/protocol/message_reader.cc (right):

http://codereview.chromium.org/10919259/diff/5/remoting/protocol/message_read...
remoting/protocol/message_reader.cc:129: if (!read_pending_)
On 2012/09/13 01:00:09, Wez wrote:
> nit: DoRead() already checks |read_pending_| so you don't need to check it
here.

Done.

http://codereview.chromium.org/10919259/diff/5/remoting/protocol/message_read...
remoting/protocol/message_reader.cc:130: DoRead(true);
On 2012/09/13 01:00:09, Wez wrote:
> Wouldn't it be simpler to have a member that you set in RunCallback around the
> |message_received_callback_.Run()|, and which you clear afterwards, that
> prevents the DoRead() from within OnMessageDone()?

I actually noticed another bug in this code - it didn't handle properly the case
when MessageReader is deleted when running OnMessageCallback. So I changed it to
always post a task for new messages.
Simplified the code significantly.

[Wez, 2012-09-13 01:41:51]

lgtm

[commit-bot: I haz the power, 2012-09-13 01:57:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sergeyu@chromium.org/10919259/5002

[commit-bot: I haz the power, 2012-09-13 03:44:40]

Change committed as 156490