Fix even more crashes. To help identify remaining crashes now and in the future, I have made the ...

Fix even more crashes. To help identify remaining crashes now and in the future, I have made the GetWidget methods on WidgetDelegate pure virtual. This will cause classes that don't define them to fail compile instead of crashing at run time.

http://crbug.com/86119
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=89409

[sky, 2011-06-16 21:42:07]

LGTM

[Nico, 2011-06-17 03:48:15]

http://codereview.chromium.org/7189019/diff/21/chrome/browser/ui/views/defaul...
File chrome/browser/ui/views/default_search_view.h (right):

http://codereview.chromium.org/7189019/diff/21/chrome/browser/ui/views/defaul...
chrome/browser/ui/views/default_search_view.h:65: virtual const views::Widget*
GetWidget() const OVERRIDE;
Hi Ben,

you added a "TODO: Find out why clang doesn't like these OVERRIDEs in
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/defau...
".

The reason is that ConstrainedDialogDelegate is only a typef for
views::DialogDelegate – on CrOS, it's a typedef for ConstrainedWindowGtkDelegate
instead (see
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/content/browser/tab_c...
). That seems at best confusing and at worst bug-introducing, so it should maybe
be changed. I don't know this code well enough to know what the right thing is.

(ConstrainedWindowGtkDelegate is here:
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/ui/gtk...
I'm not sure why this compiles as is; I guess on ChromeOS nothing tries to
create an instance of DefaultSearchView.)

[Ben Goodger (Google), 2011-06-17 04:07:38]

I figured it was related to the typedef, thanks for confirming my suspicion.
It's on my list to clear all this up at some point.

-Ben

On Thu, Jun 16, 2011 at 8:48 PM, <thakis@chromium.org> wrote:

>
> http://codereview.chromium.**org/7189019/diff/21/chrome/**
>
browser/ui/views/default_**search_view.h<http://codereview.chromium.org/7189019/diff/21/chrome/browser/ui/views/default_search_view.h>
> File chrome/browser/ui/views/**default_search_view.h (right):
>
> http://codereview.chromium.**org/7189019/diff/21/chrome/**
>
browser/ui/views/default_**search_view.h#newcode65<http://codereview.chromium.org/7189019/diff/21/chrome/browser/ui/views/default_search_view.h#newcode65>
> chrome/browser/ui/views/**default_search_view.h:65: virtual const
> views::Widget* GetWidget() const OVERRIDE;
> Hi Ben,
>
> you added a "TODO: Find out why clang doesn't like these OVERRIDEs in
> http://src.chromium.org/**viewvc/chrome/trunk/src/**
> chrome/browser/ui/views/**default_search_view.h?r1=**
>
89411&r2=89410&pathrev=89411<http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/default_search_view.h?r1=89411&r2=89410&pathrev=89411>
> ".
>
> The reason is that ConstrainedDialogDelegate is only a typef for
> views::DialogDelegate – on CrOS, it's a typedef for
> ConstrainedWindowGtkDelegate instead (see
> http://codesearch.google.com/**codesearch#OAMlx_jo-ck/src/**
> content/browser/tab_contents/**constrained_window.h&exact_**
>
package=chromium&ct=rc&cd=1&q=**ConstrainedDialogDelegate<http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/content/browser/tab_contents/constrained_window.h&exact_package=chromium&ct=rc&cd=1&q=ConstrainedDialogDelegate>
> ). That seems at best confusing and at worst bug-introducing, so it
> should maybe be changed. I don't know this code well enough to know what
> the right thing is.
>
> (ConstrainedWindowGtkDelegate is here:
> http://codesearch.google.com/**codesearch#OAMlx_jo-ck/src/**
>
chrome/browser/ui/gtk/**constrained_window_gtk.h&l=27<http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/ui/gtk/constrained_window_gtk.h&l=27>
> I'm not sure why this compiles as is; I guess on ChromeOS nothing tries
> to create an instance of DefaultSearchView.)
>
>
>
http://codereview.chromium.**org/7189019/<http://codereview.chromium.org/7189...
>