Change grow box computation back to a method on BrowserWindow

Change grow box computation back to a method on BrowserWindow

This is a revert of http://codereview.chromium.org/3547008, which was
added to implement sidebars. aa has reverted most of the sidebar code
( http://codereview.chromium.org/9006027/ ), so this is no longer needed.

This not only makes the code simpler (look at all the red), but also fixes a
long-standing regression on Mac that was caused by the CL this reverts.

Parts of the windows part with help form sky@.

This does not add back the code in rwhv_win that was added by mad@ for
bug 458, since it has since then bitrotted, and it's not used anyway. If someone
wants to attack 458 again,
http://codereview.chromium.org/16488/diff/1101/chrome/browser/renderer_host/render_widget_host_view_win.cc
should be fairly easy to add back.

BUG=458, 70482, 107646
TEST=Not too much stuff breaks; resize box looks good again on 10.5/10.6
TBR=avi

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

[Nico, 2012-01-20 18:42:08]

Hi Scott,

thanks a lot for your help with this!

It looks like the resize corner is drawn by browser_view these days, so I guess
the right thing to do is to revert the changes to
render_widget_host_view_win.h/cc in this CL, right? (I haven't done so yet since
it's easier for you to see what I'll be reverting this way.)

The rest of the CL is ready for review, I think.

[jennb, 2012-01-20 20:17:04]

LGTM 

(Owners LGTM for chrome/browser/ui/panels.)

http://codereview.chromium.org/9265018/diff/14001/chrome/browser/ui/panels/pa...
File chrome/browser/ui/panels/panel.cc (right):

http://codereview.chromium.org/9265018/diff/14001/chrome/browser/ui/panels/pa...
chrome/browser/ui/panels/panel.cc:396: NOTIMPLEMENTED();
No need for NOTIMPLEMENTED. Panels don't need to draw a resizer.

[sky, 2012-01-20 21:55:02]

On Fri, Jan 20, 2012 at 10:42 AM,  <thakis@chromium.org> wrote:
> Reviewers: sky,
>
> Message:
> Hi Scott,
>
> thanks a lot for your help with this!
>
> It looks like the resize corner is drawn by browser_view these days, so I
> guess
> the right thing to do is to revert the changes to
> render_widget_host_view_win.h/cc in this CL, right? (I haven't done so yet
> since
> it's easier for you to see what I'll be reverting this way.)

I believe so, but the resize corner isn't enabled on windows. I
believe there were performance issues with it. mad was the last one to
work on it.

  -Scott

>
> The rest of the CL is ready for review, I think.
>
> Description:
> Change grow box computation back to a method on BrowserWindow
>
> This is a revert of http://codereview.chromium.org/3547008, which was
> added to implement sidebars. aa has reverted most of the sidebar code
> ( http://codereview.chromium.org/9006027/ ), so this is no longer needed.
>
> This not only makes the code simpler (look at all the red), but also fixes a
> long-standing regression on Mac that was caused by the CL this reverts.
>
> Parts of the windows part with help form sky@.
>
> BUG=70482,107646
> TEST=Not too much stuff breaks; resize box looks good again on 10.5/10.6
>
> Please review this at http://codereview.chromium.org/9265018/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>  M chrome/browser/ui/browser.h
>  M chrome/browser/ui/browser.cc
>  M chrome/browser/ui/browser_window.h
>  M chrome/browser/ui/cocoa/browser_window_cocoa.h
>  M chrome/browser/ui/cocoa/browser_window_cocoa.mm
>  M chrome/browser/ui/cocoa/browser_window_controller.h
>  M chrome/browser/ui/cocoa/browser_window_controller.mm
>  M chrome/browser/ui/cocoa/dev_tools_controller.h
>  M chrome/browser/ui/cocoa/dev_tools_controller.mm
>  M chrome/browser/ui/cocoa/tab_contents/tab_contents_controller.h
>  M chrome/browser/ui/cocoa/tab_contents/tab_contents_controller.mm
>  M chrome/browser/ui/cocoa/tabs/tab_strip_controller.h
>  M chrome/browser/ui/cocoa/tabs/tab_strip_controller.mm
>  M chrome/browser/ui/gtk/browser_window_gtk.h
>  M chrome/browser/ui/gtk/browser_window_gtk.cc
>  M chrome/browser/ui/panels/panel.h
>  M chrome/browser/ui/panels/panel.cc
>  M chrome/browser/ui/panels/panel_window_controller_cocoa.mm
>  M chrome/browser/ui/views/frame/browser_view.h
>  M chrome/browser/ui/views/frame/browser_view.cc
>  M chrome/browser/ui/views/frame/browser_view_layout.h
>  M chrome/browser/ui/views/frame/browser_view_layout.cc
>  M chrome/browser/ui/views/tab_contents/tab_contents_container.h
>  M chrome/browser/ui/views/tab_contents/tab_contents_container.cc
>  M chrome/test/base/test_browser_window.h
>  M chrome/test/base/test_browser_window.cc
>  M content/browser/renderer_host/render_view_host.h
>  M content/browser/renderer_host/render_view_host.cc
>  M content/browser/renderer_host/render_widget_host.h
>  M content/browser/renderer_host/render_widget_host.cc
>  M content/browser/renderer_host/render_widget_host_unittest.cc
>  M content/browser/renderer_host/render_widget_host_view.h
>  M content/browser/renderer_host/render_widget_host_view_aura.h
>  M content/browser/renderer_host/render_widget_host_view_win.h
>  M content/browser/renderer_host/render_widget_host_view_win.cc
>  M content/browser/tab_contents/interstitial_page.h
>  M content/browser/tab_contents/interstitial_page.cc
>  M content/browser/tab_contents/tab_contents.h
>  M content/browser/tab_contents/tab_contents.cc
>  M content/common/view_messages.h
>  M content/public/browser/render_view_host_delegate.h
>  M content/public/browser/web_contents_delegate.h
>  M content/public/browser/web_contents_delegate.cc
>  M content/renderer/render_widget.cc
>
>

[Nico, 2012-01-21 02:29:40]

Ok, I reverted the rwhv_win change – sounds like that's a safe thing to do.
PTAL.

[sky, 2012-01-23 15:45:34]

http://codereview.chromium.org/9265018/diff/13047/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/browser_view.cc (right):

http://codereview.chromium.org/9265018/diff/13047/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_view.cc:1040: if
(IsDownloadShelfVisible()) {
nit: no {}

http://codereview.chromium.org/9265018/diff/13047/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host.cc (left):

http://codereview.chromium.org/9265018/diff/13047/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host.cc:379: if
(resize_ack_pending_) {
How come we don't need this if/else anymore?

http://codereview.chromium.org/9265018/diff/13047/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host.cc (right):

http://codereview.chromium.org/9265018/diff/13047/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host.cc:358: gfx::Size
new_size(view_bounds.width(), view_bounds.height());
nit: view_bounds.size()

http://codereview.chromium.org/9265018/diff/13047/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host.cc:1123: if
(current_size_.width() != view_bounds.width() ||
nit: current_size_ != view_bounds.size()

[Nico, 2012-01-23 15:54:14]

Thanks!

http://codereview.chromium.org/9265018/diff/13047/chrome/browser/ui/views/fra...
File chrome/browser/ui/views/frame/browser_view.cc (right):

http://codereview.chromium.org/9265018/diff/13047/chrome/browser/ui/views/fra...
chrome/browser/ui/views/frame/browser_view.cc:1040: if
(IsDownloadShelfVisible()) {
On 2012/01/23 15:45:34, sky wrote:
> nit: no {}

Done.

http://codereview.chromium.org/9265018/diff/13047/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host.cc (left):

http://codereview.chromium.org/9265018/diff/13047/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host.cc:379: if
(resize_ack_pending_) {
On 2012/01/23 15:45:34, sky wrote:
> How come we don't need this if/else anymore?

The changelist I'm reverting here added it:
http://codereview.chromium.org/3547008/diff/264001/chrome/browser/renderer_ho...

The changelist I'm reverting made it so that WasResized() is called not only
when the size changes, but also when the size stays the same and the
reserved_rect (the resize handle cutout) changes. In that case,
resize_ack_pending_ was false as well, which required the else branch in here.

The else branch still kind of makes sense just for updating current_size_, but
that case is currently handled elsewhere: See the 1. 2. 3. 4. comment at the end
of WasRestored() in this file.

http://codereview.chromium.org/9265018/diff/13047/content/browser/renderer_ho...
File content/browser/renderer_host/render_widget_host.cc (right):

http://codereview.chromium.org/9265018/diff/13047/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host.cc:358: gfx::Size
new_size(view_bounds.width(), view_bounds.height());
On 2012/01/23 15:45:34, sky wrote:
> nit: view_bounds.size()

Done.

http://codereview.chromium.org/9265018/diff/13047/content/browser/renderer_ho...
content/browser/renderer_host/render_widget_host.cc:1123: if
(current_size_.width() != view_bounds.width() ||
On 2012/01/23 15:45:34, sky wrote:
> nit: current_size_ != view_bounds.size()

Done.

[sky, 2012-01-23 16:24:39]

LGTM

[commit-bot: I haz the power, 2012-01-23 16:25:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/9265018/18002

[commit-bot: I haz the power, 2012-01-23 16:25:54]

Presubmit check for 9265018-18002 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for:
content/renderer/render_widget.cc,chrome/browser/ui/gtk/browser_window_gtk.h,content/public/browser/render_view_host_delegate.h,content/public/browser/web_contents_delegate.cc,content/public/browser/web_contents_delegate.h,chrome/browser/ui/gtk/browser_window_gtk.cc,content/common/view_messages.h

Presubmit checks took 3.4s to calculate.

[Nico, 2012-01-23 16:47:19]

avi for OWNERS

[commit-bot: I haz the power, 2012-01-23 16:47:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/9265018/18002

[commit-bot: I haz the power, 2012-01-23 18:08:22]

Change committed as 118700