On Windows 8 when a metro application is snapped running desktop application windows are resized as

On Windows 8 when a metro application is snapped running desktop application windows are resized as
the workarea is reduced. Desktop chrome fails to paint the tabstrip when we receive this resize notification.

Reason: The WM_GETTITLEBARINFOEX message we send to calculate the minimize button offset in the caption, fails
        if we are not active.

Fix is to calculate and cache the delta of the minimize button which is applied to the left/right edge of the
client rect if we fail to get the minimize button offset.

BUG=116745
R=sky
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=143447

[sky, 2012-06-21 16:38:40]

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
File chrome/browser/ui/views/frame/browser_frame_win.cc (right):

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_frame_win.cc:257: if (action !=
WA_INACTIVE)
Can you move this before the call to NativeWidgetWin::OnActivate.

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_frame_win.cc:289: BOOL ret =
SendMessage(GetNativeView(), WM_GETTITLEBARINFOEX, 0,
If this does fail, won't titlebar_info contain garbage since we never clear it?
And shouldn't we check the return value? And remain 'ret' to 'succeeded' or
something clearer.

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_frame_win.cc:304: if (base::i18n::IsRTL())
{
no {}

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_frame_win.cc:542: if (base::i18n::IsRTL())
{
no {}

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
File chrome/browser/ui/views/frame/browser_frame_win.h (right):

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_frame_win.h:122: // Enables us to
calculate the offset of the minimize button given the client
Newline before comment, and have the comment describe what it is. Or perhaps:
See CacheMinimizeButtonDelta() for a description of this.

[ananta, 2012-06-21 17:39:40]

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
File chrome/browser/ui/views/frame/browser_frame_win.cc (right):

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_frame_win.cc:257: if (action !=
WA_INACTIVE)
On 2012/06/21 16:38:40, sky wrote:
> Can you move this before the call to NativeWidgetWin::OnActivate.

Done.

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_frame_win.cc:289: BOOL ret =
SendMessage(GetNativeView(), WM_GETTITLEBARINFOEX, 0,
On 2012/06/21 16:38:40, sky wrote:
> If this does fail, won't titlebar_info contain garbage since we never clear
it?
> And shouldn't we check the return value? And remain 'ret' to 'succeeded' or
> something clearer.

Unfortunately SendMessage always returns success. Added code to initialize the
titlebar_info to 0.

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_frame_win.cc:304: if (base::i18n::IsRTL())
{
On 2012/06/21 16:38:40, sky wrote:
> no {}

Done.

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_frame_win.cc:542: if (base::i18n::IsRTL())
{
On 2012/06/21 16:38:40, sky wrote:
> no {}

Done.

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
File chrome/browser/ui/views/frame/browser_frame_win.h (right):

https://chromiumcodereview.appspot.com/10596005/diff/5/chrome/browser/ui/view...
chrome/browser/ui/views/frame/browser_frame_win.h:122: // Enables us to
calculate the offset of the minimize button given the client
On 2012/06/21 16:38:40, sky wrote:
> Newline before comment, and have the comment describe what it is. Or perhaps:
> See CacheMinimizeButtonDelta() for a description of this.

Done.

[sky, 2012-06-21 18:13:40]

https://chromiumcodereview.appspot.com/10596005/diff/15001/chrome/browser/ui/...
File chrome/browser/ui/views/frame/browser_frame_win.cc (right):

https://chromiumcodereview.appspot.com/10596005/diff/15001/chrome/browser/ui/...
chrome/browser/ui/views/frame/browser_frame_win.cc:288: BOOL ret =
SendMessage(GetNativeView(), WM_GETTITLEBARINFOEX, 0,
Remove 'ret' since as you said its always true ad you don't use it.

https://chromiumcodereview.appspot.com/10596005/diff/15001/chrome/browser/ui/...
chrome/browser/ui/views/frame/browser_frame_win.cc:529: int minimize_offset =
GetMinimizeButtonOffset();
The way you have the code now you're only going to cache the first time through,
subsequently this never returns 0 (since it uses the cached value).

[ananta, 2012-06-21 18:27:46]

https://chromiumcodereview.appspot.com/10596005/diff/15001/chrome/browser/ui/...
File chrome/browser/ui/views/frame/browser_frame_win.cc (right):

https://chromiumcodereview.appspot.com/10596005/diff/15001/chrome/browser/ui/...
chrome/browser/ui/views/frame/browser_frame_win.cc:288: BOOL ret =
SendMessage(GetNativeView(), WM_GETTITLEBARINFOEX, 0,
On 2012/06/21 18:13:40, sky wrote:
> Remove 'ret' since as you said its always true ad you don't use it.

Done.

https://chromiumcodereview.appspot.com/10596005/diff/15001/chrome/browser/ui/...
chrome/browser/ui/views/frame/browser_frame_win.cc:529: int minimize_offset =
GetMinimizeButtonOffset();
On 2012/06/21 18:13:40, sky wrote:
> The way you have the code now you're only going to cache the first time
through,
> subsequently this never returns 0 (since it uses the cached value).

Thanks for catching this. Isolated the WM_GETTITLEBARINFO stuff into a helper
functionGetMinimizeButtonOffsetFromWindows which is called from here and
GetMinimizeButtonOffset.

[sky, 2012-06-21 19:37:10]

LGTM

https://chromiumcodereview.appspot.com/10596005/diff/18002/chrome/browser/ui/...
File chrome/browser/ui/views/frame/browser_frame_win.cc (right):

https://chromiumcodereview.appspot.com/10596005/diff/18002/chrome/browser/ui/...
chrome/browser/ui/views/frame/browser_frame_win.cc:89: int
GetMinimizeButtonOffsetFromWindows(gfx::NativeView window) {
static

https://chromiumcodereview.appspot.com/10596005/diff/18002/chrome/browser/ui/...
chrome/browser/ui/views/frame/browser_frame_win.cc:89: int
GetMinimizeButtonOffsetFromWindows(gfx::NativeView window) {
GetMinimizeButtonOffsetFromWindows - >GetMinimizeButtonOffsetForWindow

[ananta, 2012-06-21 20:27:27]

https://chromiumcodereview.appspot.com/10596005/diff/18002/chrome/browser/ui/...
File chrome/browser/ui/views/frame/browser_frame_win.cc (right):

https://chromiumcodereview.appspot.com/10596005/diff/18002/chrome/browser/ui/...
chrome/browser/ui/views/frame/browser_frame_win.cc:89: int
GetMinimizeButtonOffsetFromWindows(gfx::NativeView window) {
On 2012/06/21 19:37:10, sky wrote:
> GetMinimizeButtonOffsetFromWindows - >GetMinimizeButtonOffsetForWindow

Done.