Move notifications used only in chrome/ out of content/.

Move notifications used only in chrome/ out of content/.

Moved NOTIFICATION_TAB_ADDED, NOTIFICATION_TAB_PARENTED, and NOTIFICATION_TAB_CLOSING. Turned NOTIFICATION_TAB_CLOSED into NOTIFICATION_WEB_CONTENTS_DESTROYED.

BUG=76698
TEST=none


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

[Avi (use Gerrit), 2012-04-14 03:03:56]

ben:
 chrome/browser/ui/*
 chrome/browser/tabs
marja:
 chrome/browser/sessions
thestig:
 chrome/browser/printing
atwilson:
 chrome/browser/sync
jam:
 FYI

Justification:
NOTIFICATION_TAB_ADDED, NOTIFICATION_TAB_PARENTED, and NOTIFICATION_TAB_CLOSING
are only used inside of chrome, so they don't belong in content.

NOTIFICATION_TAB_CLOSED was fired by the NavigationControllerImpl in its
destructor. Because the NavigationControllerImpl is an inline member of
WebContentsImpl (and has been for about three years now), and WebContentsImpl
already fires NOTIFICATION_WEB_CONTENTS_DESTROYED in its destructor,
NOTIFICATION_TAB_CLOSED is redundant.

And WindowedTabAddedNotificationObserver is a gloriously cheesy hack to get
around deps issues.

[marja, 2012-04-14 07:20:58]

chrome/browser/sessions lgtm

[Andrew T Wilson (Slow), 2012-04-14 23:29:29]

browser/sync LGTM

[Avi (use Gerrit), 2012-04-15 00:03:38]

On 2012/04/14 03:03:56, Avi wrote:
> jam:
>  FYI

My mistake, sorry. John, please review everything else that remains.

[jam, 2012-04-15 22:10:17]

lgtm, nice

i had it as a todo to look through all the notifications in
content/public/common/notification_types and audit that they're all raised by
content (i.e. i had caught some before which chrome raised and content listened
to, which breaks the content separation)

http://codereview.chromium.org/10079023/diff/3042/chrome/browser/ui/omnibox/o...
File chrome/browser/ui/omnibox/omnibox_view_browsertest.cc (right):

http://codereview.chromium.org/10079023/diff/3042/chrome/browser/ui/omnibox/o...
chrome/browser/ui/omnibox/omnibox_view_browsertest.cc:231: ?
static_cast<int>(chrome::NOTIFICATION_TAB_PARENTED)
nit: you shouldn't need a cast here?

[jam, 2012-04-15 22:10:36]

On 2012/04/15 22:10:17, John Abd-El-Malek wrote:
> lgtm, nice
> 
> i had it as a todo to look through all the notifications in
> content/public/common/notification_types and audit that they're all raised by
> content (i.e. i had caught some before which chrome raised and content
listened
> to, which breaks the content separation)

(hit send too early). do you know if there are any more like this?

[Avi (use Gerrit), 2012-04-16 00:09:14]

On 2012/04/15 22:10:36, John Abd-El-Malek wrote:
> do you know if there are any more like this?

I'm on a hunt for the word "tab" in the content module, which led me to this. I
don't know of any others. I'd imagine it would be easy to search for
"content::NotificationService::current()->Notify(content::" in chrome/ code.

[Avi (use Gerrit), 2012-04-16 00:09:38]

http://codereview.chromium.org/10079023/diff/3042/chrome/browser/ui/omnibox/o...
File chrome/browser/ui/omnibox/omnibox_view_browsertest.cc (right):

http://codereview.chromium.org/10079023/diff/3042/chrome/browser/ui/omnibox/o...
chrome/browser/ui/omnibox/omnibox_view_browsertest.cc:231: ?
static_cast<int>(chrome::NOTIFICATION_TAB_PARENTED)
On 2012/04/15 22:10:18, John Abd-El-Malek wrote:
> nit: you shouldn't need a cast here?

It wouldn't be there if I didn't need it :) chrome::XXX and content:XXX are two
different types; you can't ?: them together without a cast because the compiler
won't know what type to make the result. (See the C++98 standard §5.16¶5; "if
the overload resolution fails, the program is ill-formed".)

[Ben Goodger (Google), 2012-04-16 16:25:56]

LGTM for browser/ui and browser/tabs

[Scott Byer, 2012-04-16 17:53:49]

LGTM for printing.

[commit-bot: I haz the power, 2012-04-16 17:57:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10079023/3042

[commit-bot: I haz the power, 2012-04-16 19:38:46]

Change committed as 132449

[jam, 2012-04-17 03:19:56]

On 2012/04/16 00:09:38, Avi wrote:
>
http://codereview.chromium.org/10079023/diff/3042/chrome/browser/ui/omnibox/o...
> File chrome/browser/ui/omnibox/omnibox_view_browsertest.cc (right):
> 
>
http://codereview.chromium.org/10079023/diff/3042/chrome/browser/ui/omnibox/o...
> chrome/browser/ui/omnibox/omnibox_view_browsertest.cc:231: ?
> static_cast<int>(chrome::NOTIFICATION_TAB_PARENTED)
> On 2012/04/15 22:10:18, John Abd-El-Malek wrote:
> > nit: you shouldn't need a cast here?
> 
> It wouldn't be there if I didn't need it :) chrome::XXX and content:XXX are
two
> different types; you can't ?: them together without a cast because the
compiler
> won't know what type to make the result. (See the C++98 standard §5.16¶5; "if
> the overload resolution fails, the program is ill-formed".)

got it, thanks :)