Fixes reloading after a background tab crashes....

Fixes reloading after a background tab crashes.

Needed to clear RenderWidgetHost::in_flight_size_ if the renderer crashed.

BUG=25097
TEST=RenderWidgetHostTest.ResizeThenCrash


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

[Nico, 2009-10-30 18:32:42]

LG.

I'm not super-familiar with this code (I only did the in_fligh_ change and else
never touched this file), but your change makes sense to me. Thanks for adding a
test.

[Nico, 2009-10-30 18:35:16]

Still LG, but I have a question :-)

(Also, why is `= gfx::Size()` better than `.SetSize(0, 0);`? The latter doesn't
rely on default arguments and needs potentially fewer data copying.)

http://codereview.chromium.org/348025/diff/1/3
File chrome/browser/renderer_host/render_widget_host.cc (right):

http://codereview.chromium.org/348025/diff/1/3#newcode239
Line 239: resize_ack_pending_ = false;
Might resetting the size be required here, too? Or should this be fine?

[Charlie Reis, 2009-10-30 19:17:14]

On 2009/10/30 18:35:16, Nico wrote:
> Still LG, but I have a question :-)
> 
> (Also, why is `= gfx::Size()` better than `.SetSize(0, 0);`? The latter
doesn't
> rely on default arguments and needs potentially fewer data copying.)

I was just trying to be consistent, since there were a few places using
gfx::Size() already (including the similar call for current_size_ in
RendererExited).  It sounds like it might be worth switching them over to
setSize, though.

I'll update a new changelist this afternoon when I get a minute.

Thanks!

[Charlie Reis, 2009-10-30 19:17:27]

http://codereview.chromium.org/348025/diff/1/3
File chrome/browser/renderer_host/render_widget_host.cc (right):

http://codereview.chromium.org/348025/diff/1/3#newcode239
Line 239: resize_ack_pending_ = false;
On 2009/10/30 18:35:17, Nico wrote:
> Might resetting the size be required here, too? Or should this be fine?

Good catch.  We should probably reset it here too.

[Charlie Reis, 2009-10-30 20:01:48]

New CL uploaded.

[Nico, 2009-10-30 20:03:28]

Still LG.

http://codereview.chromium.org/348025/diff/3001/4002
File chrome/browser/renderer_host/render_widget_host.cc (right):

http://codereview.chromium.org/348025/diff/3001/4002#newcode240
Line 240: in_flight_size_.SetSize(0, 0);
Is there a scenario where this line actually makes a difference? Would it be
possible to add a test for that scenario if it exists?

[Charlie Reis, 2009-10-30 22:34:13]

On Fri, Oct 30, 2009 at 1:03 PM, <thakis@chromium.org> wrote:

> Still LG.
>
>
> http://codereview.chromium.org/348025/diff/3001/4002
>
> File chrome/browser/renderer_host/render_widget_host.cc (right):
>
> http://codereview.chromium.org/348025/diff/3001/4002#newcode240
> Line 240: in_flight_size_.SetSize(0, 0);
> Is there a scenario where this line actually makes a difference? Would
> it be possible to add a test for that scenario if it exists?


That's a good question.  Brett, do you know what could make
RenderWidgetHost->Send fail, if the view is valid?  (I suppose the view
could go away between the start of the function and here, but that's harder
to test.)

Charlie


>
> http://codereview.chromium.org/348025
>

[brettw, 2009-11-02 15:51:17]

On Fri, Oct 30, 2009 at 2:33 PM, Charles Reis <creis@chromium.org> wrote:
>
>
> On Fri, Oct 30, 2009 at 1:03 PM, <thakis@chromium.org> wrote:
>>
>> Still LG.
>>
>>
>> http://codereview.chromium.org/348025/diff/3001/4002
>> File chrome/browser/renderer_host/render_widget_host.cc (right):
>>
>> http://codereview.chromium.org/348025/diff/3001/4002#newcode240
>> Line 240: in_flight_size_.SetSize(0, 0);
>> Is there a scenario where this line actually makes a difference? Would
>> it be possible to add a test for that scenario if it exists?
>
> That's a good question. =A0Brett, do you know what could make
> RenderWidgetHost->Send fail, if the view is valid? =A0(I suppose the view
> could go away between the start of the function and here, but that's hard=
er
> to test.)

The only way I can think that Send can fail is if the channel is
deleted (i.e. the process goes away).

Brett

[Charlie Reis, 2009-11-02 17:27:46]

Thanks Brett.  In that case, it looks like the line was redundant with the one
in RendererExited and doesn't make sense there (especially since we won't end up
setting in_flight_size_ if the send fails).

I took it out and cleaned up some unnecessary code in the new test.  Should be
ready to go.  Sound good, Nico?

[Nico, 2009-11-02 17:41:20]

Ship it.

[Nico, 2009-11-02 17:42:38]

…assuming the trybots pass. Linux looks unhappy, seems to crash in
RenderWidgetHostTest.ResizeThenCrash :-/

[Charlie Reis, 2009-11-02 17:52:14]

On 2009/11/02 17:42:38, Nico wrote:
> …assuming the trybots pass. Linux looks unhappy, seems to crash in
> RenderWidgetHostTest.ResizeThenCrash :-/

Yeah, I saw that...  I'd be surprised if the lines I removed from the tests
caused it to fail on Linux, but it might be possible.  I'm running it again to
see what happens.

[Charlie Reis, 2009-11-02 18:32:01]

On 2009/11/02 17:52:14, creis wrote:
> On 2009/11/02 17:42:38, Nico wrote:
> > …assuming the trybots pass. Linux looks unhappy, seems to crash in
> > RenderWidgetHostTest.ResizeThenCrash :-/
> 
> Yeah, I saw that...  I'd be surprised if the lines I removed from the tests
> caused it to fail on Linux, but it might be possible.  I'm running it again to
> see what happens.

Looks like a fluke on the try bot-- it passed this time.  I'll commit it and
keep an eye on the tree.