Try to add more sanity checking to help track down a crash....

Try to add more sanity checking to help track down a crash.

BUG=20511
TEST=none

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

[sky, 2009-11-04 00:27:47]

http://codereview.chromium.org/342112/diff/1/3
File chrome/browser/autocomplete/autocomplete_popup_model.cc (right):

http://codereview.chromium.org/342112/diff/1/3#newcode182
Line 182: CHECK(FALSE);
nit: FALSE -> false

http://codereview.chromium.org/342112/diff/1/4
File chrome/browser/views/autocomplete/autocomplete_popup_contents_view.cc
(right):

http://codereview.chromium.org/342112/diff/1/4#newcode745
Line 745: popup_->Show();
Do you need to set is_open_ here?

[Peter Kasting, 2009-11-04 00:59:58]

http://codereview.chromium.org/342112/diff/1/4
File chrome/browser/views/autocomplete/autocomplete_popup_contents_view.cc
(right):

http://codereview.chromium.org/342112/diff/1/4#newcode745
Line 745: popup_->Show();
On 2009/11/04 00:27:47, sky wrote:
> Do you need to set is_open_ here?

No; this should only be called when the popup is already open.  I added a
CHECK() to that effect.

[sky, 2009-11-04 01:03:07]

LGTM

On Tue, Nov 3, 2009 at 4:59 PM,  <pkasting@chromium.org> wrote:
>
> http://codereview.chromium.org/342112/diff/1/4
> File
> chrome/browser/views/autocomplete/autocomplete_popup_contents_view.cc
> (right):
>
> http://codereview.chromium.org/342112/diff/1/4#newcode745
> Line 745: popup_->Show();
> On 2009/11/04 00:27:47, sky wrote:
>>
>> Do you need to set is_open_ here?
>
> No; this should only be called when the popup is already open. =A0I added
> a CHECK() to that effect.
>
> http://codereview.chromium.org/342112
>

[oshima, 2009-11-05 00:32:47]

Hi, looks like this broke linux_view. I'm getting

Check failed: is_open == is_open_. 

I'm trying to fix this now, but let me know if you already
started working on it.

- oshima

On 2009/11/04 01:03:07, sky wrote:
> LGTM
> 
> On Tue, Nov 3, 2009 at 4:59 PM,  <mailto:pkasting@chromium.org> wrote:
> >
> > http://codereview.chromium.org/342112/diff/1/4
> > File
> > chrome/browser/views/autocomplete/autocomplete_popup_contents_view.cc
> > (right):
> >
> > http://codereview.chromium.org/342112/diff/1/4#newcode745
> > Line 745: popup_->Show();
> > On 2009/11/04 00:27:47, sky wrote:
> >>
> >> Do you need to set is_open_ here?
> >
> > No; this should only be called when the popup is already open. =A0I added
> > a CHECK() to that effect.
> >
> > http://codereview.chromium.org/342112
> >

[Peter Kasting, 2009-11-05 00:34:00]

On 2009/11/05 00:32:47, oshima wrote:
> Hi, looks like this broke linux_view. I'm getting
> 
> Check failed: is_open == is_open_. 
> 
> I'm trying to fix this now, but let me know if you already
> started working on it.

I haven't, but I'm very interested in why that would be failing.

[oshima, 2009-11-05 00:37:49]

On Wed, Nov 4, 2009 at 4:34 PM, <pkasting@chromium.org> wrote:

> On 2009/11/05 00:32:47, oshima wrote:
>
>> Hi, looks like this broke linux_view. I'm getting
>>
>
>  Check failed: is_open == is_open_.
>>
>
>  I'm trying to fix this now, but let me know if you already
>> started working on it.
>>
>
> I haven't, but I'm very interested in why that would be failing.
>
>
May be the word "broke" was too strong. Could be that the condition has
already been broken,
and your change just might have revealed it. I just started looking at, so
I'll let you know.

- oshima

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

[oshima, 2009-11-05 00:55:47]

Here is the change that fixed the failure.

http://codereview.chromium.org/363018

Does this make sense to you? I'm going to test it on windows now.

- oshima

On Wed, Nov 4, 2009 at 4:37 PM, oshima <oshima@chromium.org> wrote:

>
>
> On Wed, Nov 4, 2009 at 4:34 PM, <pkasting@chromium.org> wrote:
>
>> On 2009/11/05 00:32:47, oshima wrote:
>>
>>> Hi, looks like this broke linux_view. I'm getting
>>>
>>
>>  Check failed: is_open == is_open_.
>>>
>>
>>  I'm trying to fix this now, but let me know if you already
>>> started working on it.
>>>
>>
>> I haven't, but I'm very interested in why that would be failing.
>>
>>
> May be the word "broke" was too strong. Could be that the condition has
> already been broken,
> and your change just might have revealed it. I just started looking at, so
> I'll let you know.
>
> - oshima
>
>>
>> http://codereview.chromium.org/342112
>>
>
>