content: Move constrained window code from TabContents to TabContentsWrapper

content: Move constrained window code from TabContents to TabContentsWrapper

BUG=95257
TEST=compiles,ConstrainedWindowTabHelperUnit.ConstrainedWindows is green


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

[Elliot Glaysher, 2011-09-27 23:06:42]

Even with a week of preparation patches, this final move all the code out of
content/ and into its own TabHelper in chrome/ is HUGE.

Most files remove "->tab_contents()" calls from TabContentsWrapper objects,
moving the joints between chrome and content. The most important code
modifications are the ones in tab_contents.* and
constrained_window_tab_helper.*.

Avi: Is the change I did in tab_strip_controller.mm OK? Because the timing of
destruction has also moved slightly (based on the TabContentsDestroyed()
observer method), I'm having problems satisfying views requirements and not
having that little hack there at the same time.

Followup patches will move the constrained_window.h header out of content/.
Patch is too long as is.

[jam, 2011-09-28 00:08:35]

lgtm

http://codereview.chromium.org/7880003/diff/33001/chrome/browser/ui/constrain...
File chrome/browser/ui/constrained_window_tab_helper.h (right):

http://codereview.chromium.org/7880003/diff/33001/chrome/browser/ui/constrain...
chrome/browser/ui/constrained_window_tab_helper.h:42: { return
child_windows_.begin(); }
nit: the brace bracket be on the previous line. also below

http://codereview.chromium.org/7880003/diff/33001/chrome/browser/ui/constrain...
chrome/browser/ui/constrained_window_tab_helper.h:60: ConstrainedWindowList
child_windows_;
nit: DISALLOW_COPY_AND_ASSIGN?

[Avi (use Gerrit), 2011-09-28 00:16:50]

http://codereview.chromium.org/7880003/diff/33001/chrome/browser/ui/tab_conte...
File chrome/browser/ui/tab_contents/tab_contents_wrapper.cc (right):

http://codereview.chromium.org/7880003/diff/33001/chrome/browser/ui/tab_conte...
chrome/browser/ui/tab_contents/tab_contents_wrapper.cc:259:
constrained_window_tab_helper_.reset(new ConstrainedWindowTabHelper(this));
in same order as alphabetized variables.

http://codereview.chromium.org/7880003/diff/33001/chrome/browser/ui/tab_conte...
File chrome/browser/ui/tab_contents/tab_contents_wrapper.h (right):

http://codereview.chromium.org/7880003/diff/33001/chrome/browser/ui/tab_conte...
chrome/browser/ui/tab_contents/tab_contents_wrapper.h:177: }
Alphabetical, plz? Between BookmarkTabHelper and ExtensionTabHelper.

http://codereview.chromium.org/7880003/diff/33001/chrome/browser/ui/tab_conte...
chrome/browser/ui/tab_contents/tab_contents_wrapper.h:286:
scoped_ptr<ConstrainedWindowTabHelper> constrained_window_tab_helper_;
Alphabetical here too, plz.

http://codereview.chromium.org/7880003/diff/33001/chrome/browser/ui/tab_conte...
File chrome/browser/ui/tab_contents/tab_contents_wrapper_delegate.h (right):

http://codereview.chromium.org/7880003/diff/33001/chrome/browser/ui/tab_conte...
chrome/browser/ui/tab_contents/tab_contents_wrapper_delegate.h:39: virtual void
SetTabContentBlocked(TabContentsWrapper* wrapper, bool blocked);
Are these for the constrained window stuff? (Looks like it!) Then make a
ConstrainedWindowTabHelperDelegate. Tab helpers get their own delegates; don't
make them all share one big messy file.

[Elliot Glaysher, 2011-09-29 17:59:33]

ptal

[Avi (use Gerrit), 2011-09-29 18:30:36]

lgtm with fixes noted below

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/automation/t...
File chrome/browser/automation/testing_automation_provider.cc (right):

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/automation/t...
chrome/browser/automation/testing_automation_provider.cc:1363: if (wrapper)
braces around content of if when it spans more than one line

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/cocoa/con...
File chrome/browser/ui/cocoa/constrained_window_mac.h (right):

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/cocoa/con...
chrome/browser/ui/cocoa/constrained_window_mac.h:21: class TabContents;
forward decl still needed?

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/cocoa/con...
chrome/browser/ui/cocoa/constrained_window_mac.h:121: // Returns the TabContents
that constrains this Constrained Window.
update comment?

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/cocoa/con...
chrome/browser/ui/cocoa/constrained_window_mac.h:133: // The TabContents that
owns and constrains this ConstrainedWindow.
update comment?

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/constrain...
File chrome/browser/ui/constrained_window_tab_helper.h (right):

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/constrain...
chrome/browser/ui/constrained_window_tab_helper.h:45:
ConstrainedWindowList::iterator constrained_window_begin()
As previously noted, opening brace goes on this line and closing brace goes on
its own line.

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/constrain...
chrome/browser/ui/constrained_window_tab_helper.h:50: { return
child_windows_.end(); }
ditto

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/gtk/const...
File chrome/browser/ui/gtk/constrained_window_gtk.h (right):

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/gtk/const...
chrome/browser/ui/gtk/constrained_window_gtk.h:70: // Returns the TabContents
that constrains this Constrained Window.
comment?

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/gtk/const...
chrome/browser/ui/gtk/constrained_window_gtk.h:88: // The TabContents that owns
and constrains this ConstrainedWindow.
comment?

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/views/con...
File chrome/browser/ui/views/constrained_window_views.h (right):

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/views/con...
chrome/browser/ui/views/constrained_window_views.h:15: class TabContents;
forward decl still needed?

http://codereview.chromium.org/7880003/diff/40050/chrome/browser/ui/views/con...
chrome/browser/ui/views/constrained_window_views.h:66: // Returns the
TabContents that constrains this Constrained Window.
comment?

[commit-bot: I haz the power, 2011-09-29 22:52:19]

CQ is trying tha patch. Follow status at
https://chromium-status.appspot.com/cq/erg@chromium.org/7880003/48001

[commit-bot: I haz the power, 2011-09-30 01:49:48]

Change committed as 103403