Fix a long standing painting problem seen in the tabstrip on Windows Desktop Chrome.

Fix a long standing painting problem seen in the tabstrip on Windows Desktop Chrome.

The problem appears when we switch away from an installed theme to glass while in fullscreen mode. Debugging this revealed the following:-

1. Windows sends the WM_DWMNCRENDERINGCHANGED message when DWM
   NC rendering is enabled or disabled for a window. The wParam
   in the message indicates if NC rendering is enabled.

2. In fullscreen mode if we attempt to transition a window to
   DWM we get the above message with wParam as TRUE which
   indicates that DWM is enabled. However we eventually receive
   this message again with wParam as FALSE indicating that DWM
   is turned off for the window. Some windbg debugging revealed
   that there is a function in the themes dll which turns off
   DWM for fullscreen windows.

The painting problem occurs because we think we are in DWM and Windows does not.

Attempted fix is to delay the DWM portion of the themes change until the window comes out of fullscreen mode.

BUG=404564
R=sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290941

[sky, 2014-08-19 13:19:36]

https://codereview.chromium.org/484963002/diff/1/chrome/browser/ui/views/fram...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/484963002/diff/1/chrome/browser/ui/views/fram...
chrome/browser/ui/views/frame/browser_view.cc:1253: if (IsFullscreen())
This makes no sense to me. The user wants to toggle the theme, not the
fullscreen mode. Something tells me you're working around bugs and not really
fixing the issue.

[ananta, 2014-08-20 02:28:06]

https://codereview.chromium.org/484963002/diff/1/chrome/browser/ui/views/fram...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/484963002/diff/1/chrome/browser/ui/views/fram...
chrome/browser/ui/views/frame/browser_view.cc:1253: if (IsFullscreen())
On 2014/08/19 13:19:36, sky wrote:
> This makes no sense to me. The user wants to toggle the theme, not the
> fullscreen mode. Something tells me you're working around bugs and not really
> fixing the issue.

Please take a look at the updated patch. This has the delayed DWM related stuff
which we talked about.

[ananta, 2014-08-20 03:51:21]

On 2014/08/20 02:28:06, ananta wrote:
>
https://codereview.chromium.org/484963002/diff/1/chrome/browser/ui/views/fram...
> File chrome/browser/ui/views/frame/browser_view.cc (right):
> 
>
https://codereview.chromium.org/484963002/diff/1/chrome/browser/ui/views/fram...
> chrome/browser/ui/views/frame/browser_view.cc:1253: if (IsFullscreen())
> On 2014/08/19 13:19:36, sky wrote:
> > This makes no sense to me. The user wants to toggle the theme, not the
> > fullscreen mode. Something tells me you're working around bugs and not
really
> > fixing the issue.
> 
> Please take a look at the updated patch. This has the delayed DWM related
stuff
> which we talked about.

Tested this with different window size combinations including fullscreen. Works
well.

I dont think that the version check code on HWNDMessageHandler::FrameTypeChanged
is needed. Will look into that

[sky, 2014-08-20 15:25:56]

https://codereview.chromium.org/484963002/diff/20001/ui/views/win/hwnd_messag...
File ui/views/win/hwnd_message_handler.cc (right):

https://codereview.chromium.org/484963002/diff/20001/ui/views/win/hwnd_messag...
ui/views/win/hwnd_message_handler.cc:805: dwm_transition_desired_ = true;
Shouldn't this be conditional on whether you actually need a dwm frame?

https://codereview.chromium.org/484963002/diff/20001/ui/views/win/hwnd_messag...
ui/views/win/hwnd_message_handler.cc:806: // On Windows Vista and beyond, if we
are transitioning from glass to
I think this comment would be better in the header for dwm_transition_desired_.
That way there is a central place describing how all this works.

[ananta, 2014-08-20 18:52:55]

https://codereview.chromium.org/484963002/diff/20001/ui/views/win/hwnd_messag...
File ui/views/win/hwnd_message_handler.cc (right):

https://codereview.chromium.org/484963002/diff/20001/ui/views/win/hwnd_messag...
ui/views/win/hwnd_message_handler.cc:805: dwm_transition_desired_ = true;
On 2014/08/20 15:25:56, sky wrote:
> Shouldn't this be conditional on whether you actually need a dwm frame?

Done.

https://codereview.chromium.org/484963002/diff/20001/ui/views/win/hwnd_messag...
ui/views/win/hwnd_message_handler.cc:806: // On Windows Vista and beyond, if we
are transitioning from glass to
On 2014/08/20 15:25:56, sky wrote:
> I think this comment would be better in the header for
dwm_transition_desired_.
> That way there is a central place describing how all this works.

Done.

[sky, 2014-08-20 19:20:53]

LGTM

[ananta, 2014-08-20 19:55:11]

The CQ bit was checked by ananta@chromium.org

[commit-bot: I haz the power, 2014-08-20 19:56:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ananta@chromium.org/484963002/100001

[commit-bot: I haz the power, 2014-08-20 21:31:58]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[commit-bot: I haz the power, 2014-08-20 21:36:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-20 21:36:51]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[ananta, 2014-08-20 21:37:20]

The CQ bit was checked by ananta@chromium.org

[commit-bot: I haz the power, 2014-08-20 21:38:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ananta@chromium.org/484963002/100001

[commit-bot: I haz the power, 2014-08-20 22:30:19]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[commit-bot: I haz the power, 2014-08-20 22:34:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-20 22:34:34]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[ananta, 2014-08-20 22:59:57]

Committed patchset #3 manually as 290941 (presubmit successful).