Layout in the ShellWindow becomes chaos

Layout in the ShellWindow becomes chaos 
after the window restored from minimize operation.


BUG=138074
TEST=manually


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

[Junmin Zhu, 2012-07-19 14:53:18]

In this issue, because after window minimized, there would still trigger
"OnWindowPosChanged" to repaint view hierarchy, what's more, the changed size
for repaint is wrong, take this issue as example, "GetWindowRect" always return
[160,27] for the minimized window, which is incorrect.

So here add a "!IsMinimized()" check, before call "ClientAreaSizeChanged".


Please review the patch, thanks.

[Ben Goodger (Google), 2012-07-23 17:18:41]

Did you ensure this doesn't cause strange visual regressions for the
various other frame modes?

(Glass browser window, glass popups, themed browser window, themed popups)
etc.

-Ben


On Thu, Jul 19, 2012 at 7:53 AM, <junmin.zhu@intel.com> wrote:

> Reviewers: Ben Goodger (Google),
>
> Message:
> In this issue, because after window minimized, there would still trigger
> "OnWindowPosChanged" to repaint view hierarchy, what's more, the changed
> size
> for repaint is wrong, take this issue as example, "GetWindowRect" always
> return
> [160,27] for the minimized window, which is incorrect.
>
> So here add a "!IsMinimized()" check, before call "ClientAreaSizeChanged".
>
>
> Please review the patch, thanks.
>
> Description:
> Layout in the ShellWindow becomes chaos
> after the window restored from minimize operation.
>
>
> BUG=138074
> TEST=manually
>
>
> Please review this at
http://codereview.chromium.**org/10815006/<http://codereview.chromium.org/108...
>
> SVN Base:
http://git.chromium.org/**chromium/src.git@master<http://git.chromium.org/chr...
>
> Affected files:
>   M ui/views/widget/native_widget_**win.cc
>
>
> Index: ui/views/widget/native_widget_**win.cc
> diff --git a/ui/views/widget/native_**widget_win.cc
> b/ui/views/widget/native_**widget_win.cc
> index 6bb6f406d94e0ffd3d0032cfd3a646**d1ac8c431b..**
> 369b43093e4ac692183457f53b2286**fb2f805a6d 100644
> --- a/ui/views/widget/native_**widget_win.cc
> +++ b/ui/views/widget/native_**widget_win.cc
> @@ -2210,7 +2210,7 @@ void NativeWidgetWin::**OnWindowPosChanging(WINDOWPOS*
> window_pos) {
>  }
>
>  void NativeWidgetWin::**OnWindowPosChanged(WINDOWPOS* window_pos) {
> -  if (DidClientAreaSizeChange(**window_pos))
> +  if (DidClientAreaSizeChange(**window_pos) && !IsMinimized())
>      ClientAreaSizeChanged();
>    if (remove_standard_frame_ && window_pos->flags & SWP_FRAMECHANGED &&
>        IsAeroGlassEnabled())
>
>
>

[Junmin Zhu, 2012-08-02 11:46:19]

Test with the various other frame modes, no regression happened.

"The (-32000,-32000) coordinates are a rather nasty hack of the Windows window
manager to hide "minimized" windows; these are actually just normal visible
windows, and placed at (-32000,-32000) to get them off the visible desktop. and
Height & width of windows are very small too (160 & 27)". 

Reference: http://blogs.msdn.com/b/oldnewthing/archive/2004/10/28/249044.aspx


Add jeremya as reviewer, he is this issue's owner.
thanks.

[Junmin Zhu, 2012-08-03 01:33:17]

update the patch, not to cruelly cut relayout path when minimized the window. 

Please review the patch. Thanks.

[jeremya, 2012-08-03 05:01:08]

On 2012/08/03 01:33:17, Junmin Zhu wrote:
> update the patch, not to cruelly cut relayout path when minimized the window. 
> 
> Please review the patch. Thanks.

I'm confused why resizing the window to 0x0 pixels is better than resizing it to
160x27.

[Junmin Zhu, 2012-08-03 05:12:41]

On 2012/08/03 05:01:08, jeremya wrote:
> On 2012/08/03 01:33:17, Junmin Zhu wrote:
> > update the patch, not to cruelly cut relayout path when minimized the
window. 
> > 
> > Please review the patch. Thanks.
> 
> I'm confused why resizing the window to 0x0 pixels is better than resizing it
to
> 160x27.

Because the caption height for ShellWindowFrameView is 25(kCaptionHeight = 25),
so if resizing to 160x27, ShellWindowView would relayout to the size 160x2,
which is't empty, and cause web content relayout too. After that, the scrollbar
showed, and the UI of app would become chao after restore.

[Junmin Zhu, 2012-08-03 05:12:45]

On 2012/08/03 05:01:08, jeremya wrote:
> On 2012/08/03 01:33:17, Junmin Zhu wrote:
> > update the patch, not to cruelly cut relayout path when minimized the
window. 
> > 
> > Please review the patch. Thanks.
> 
> I'm confused why resizing the window to 0x0 pixels is better than resizing it
to
> 160x27.

Because the caption height for ShellWindowFrameView is 25(kCaptionHeight = 25),
so if resizing to 160x27, ShellWindowView would relayout to the size 160x2,
which is't empty, and cause web content relayout too. After that, the scrollbar
showed, and the UI of app would become chao after restore.

[jeremya, 2012-08-05 22:53:46]

On 2012/08/03 05:12:45, Junmin Zhu wrote:
> On 2012/08/03 05:01:08, jeremya wrote:
> > On 2012/08/03 01:33:17, Junmin Zhu wrote:
> > > update the patch, not to cruelly cut relayout path when minimized the
> window. 
> > > 
> > > Please review the patch. Thanks.
> > 
> > I'm confused why resizing the window to 0x0 pixels is better than resizing
it
> to
> > 160x27.
> 
> Because the caption height for ShellWindowFrameView is 25(kCaptionHeight =
25),
> so if resizing to 160x27, ShellWindowView would relayout to the size 160x2,
> which is't empty, and cause web content relayout too. After that, the
scrollbar
> showed, and the UI of app would become chao after restore.

Ah, right :)

LGTM, but you'll need Ben's approval to land this.

[Ben Goodger (Google), 2012-08-06 19:32:35]

I'm a little curious as to if this could be done for the other branch of the
if-else as well. Can you add a TODO(beng): for me to investigate that?

LGTM.

http://codereview.chromium.org/10815006/diff/3001/ui/views/widget/native_widg...
File ui/views/widget/native_widget_win.cc (right):

http://codereview.chromium.org/10815006/diff/3001/ui/views/widget/native_widg...
ui/views/widget/native_widget_win.cc:2503: GetWindowRect(&r);
braces around else too.

[commit-bot: I haz the power, 2012-08-07 00:55:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/junmin.zhu@intel.com/10815006/3002

[commit-bot: I haz the power, 2012-08-07 03:55:52]

Change committed as 150256