Move Sad Tab implementation out of the TabContentsViews.

Move Sad Tab implementation out of the TabContentsViews.

BUG=103258
TEST=no change in sad tab behavior

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

[Avi (use Gerrit), 2011-11-13 23:30:55]

jochen-mac stuff
elliot-gtk
scott-views
john-general, and can you answer my question on TCVWin?

[jochen (gone - plz use gerrit), 2011-11-14 16:44:54]

http://codereview.chromium.org/8477042/diff/13001/chrome/browser/ui/cocoa/tab...
File chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.h (right):

http://codereview.chromium.org/8477042/diff/13001/chrome/browser/ui/cocoa/tab...
chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.h:47: gfx::NativeView
ViewOfSadTabController(SadTabController* sad_tab);
maybe verb+noun would be better? GetViewOfSadTabController?

http://codereview.chromium.org/8477042/diff/13001/chrome/browser/ui/cocoa/tab...
File chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.mm (right):

http://codereview.chromium.org/8477042/diff/13001/chrome/browser/ui/cocoa/tab...
chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.mm:13: }
empty line after this

http://codereview.chromium.org/8477042/diff/13001/chrome/browser/ui/cocoa/tab...
chrome/browser/ui/cocoa/tab_contents/sad_tab_controller.mm:17: }
// namespace sad_tab_controller_mac

http://codereview.chromium.org/8477042/diff/13001/chrome/browser/ui/sad_tab_o...
File chrome/browser/ui/sad_tab_observer.h (right):

http://codereview.chromium.org/8477042/diff/13001/chrome/browser/ui/sad_tab_o...
chrome/browser/ui/sad_tab_observer.h:59: class ScopedPtrRelease {
shouldn't that go into some base/ header?

http://codereview.chromium.org/8477042/diff/13001/content/browser/tab_content...
File content/browser/tab_contents/tab_contents_view.h (right):

http://codereview.chromium.org/8477042/diff/13001/content/browser/tab_content...
content/browser/tab_contents/tab_contents_view.h:124: // |RemoveOverlayView|.
Can you please add a comment that the view is owned by the caller, and either
needs to outlive the tab contents view, or needs to be removed

[Elliot Glaysher, 2011-11-14 18:29:33]

gtk lgtm

[jam, 2011-11-14 19:56:28]

very cool, lgtm

http://codereview.chromium.org/8477042/diff/15002/chrome/browser/ui/sad_tab_o...
File chrome/browser/ui/sad_tab_observer.cc (right):

http://codereview.chromium.org/8477042/diff/15002/chrome/browser/ui/sad_tab_o...
chrome/browser/ui/sad_tab_observer.cc:68: void SadTabObserver::ReleaseSadTab() {
this function and below are duplicated for all three platforms. why not just
list it once?

also, my personal style would be to list AcquireSadTab once and have the ifdef
inside of it, since it's the same function signature on all platforms. but up to
you.

http://codereview.chromium.org/8477042/diff/15002/content/browser/tab_content...
File content/browser/tab_contents/tab_contents_view_win.cc (right):

http://codereview.chromium.org/8477042/diff/15002/content/browser/tab_content...
content/browser/tab_contents/tab_contents_view_win.cc:78: // this be moved
elsewhere so that |OnTabCrashed| can be removed everywhere?
perhaps :) I had to add this to avoid dereferencing a null views_ after a tab
crashed. I can look into moving this elsewhere

[Avi (use Gerrit), 2011-11-14 19:59:47]

On 2011/11/14 19:56:28, John Abd-El-Malek wrote:
> perhaps :) I had to add this to avoid dereferencing a null views_ after a tab
> crashed. I can look into moving this elsewhere

I'll get this committed first; when you have a chance.

[Avi (use Gerrit), 2011-11-14 20:00:50]

http://codereview.chromium.org/8477042/diff/13001/chrome/browser/ui/sad_tab_o...
File chrome/browser/ui/sad_tab_observer.h (right):

http://codereview.chromium.org/8477042/diff/13001/chrome/browser/ui/sad_tab_o...
chrome/browser/ui/sad_tab_observer.h:59: class ScopedPtrRelease {
On 2011/11/14 16:44:54, jochen wrote:
> shouldn't that go into some base/ header?

This is a hack; this is building the equivalent of scoped_nsobject which is
already in a header but can't be used here.

[Avi (use Gerrit), 2011-11-14 20:01:11]

addressed; ptal

[jam, 2011-11-14 20:06:35]

On Mon, Nov 14, 2011 at 11:59 AM, <avi@chromium.org> wrote:

> On 2011/11/14 19:56:28, John Abd-El-Malek wrote:
>
>> perhaps :) I had to add this to avoid dereferencing a null views_ after a
>> tab
>> crashed. I can look into moving this elsewhere
>>
>
> I'll get this committed first; when you have a chance.
>

agreed, i meant later  :)

>
>
http://codereview.chromium.**org/8477042/<http://codereview.chromium.org/8477...
>

[jochen (gone - plz use gerrit), 2011-11-14 20:08:08]

mac lgtm

[sky, 2011-11-14 22:17:41]

LGTM