Don't check autohide taskbar for WS_EX_TOPMOST when we are querying the autohide state.

chrome/browser/ui/views/chrome_views_delegate.cc

Index: chrome/browser/ui/views/chrome_views_delegate.cc
diff --git a/chrome/browser/ui/views/chrome_views_delegate.cc b/chrome/browser/ui/views/chrome_views_delegate.cc
index 3d66274647c07cd4dde40679454665a7c93361e0..7c09713b40a7b34348af42c59441eb7c83587bc2 100644
--- a/chrome/browser/ui/views/chrome_views_delegate.cc
+++ b/chrome/browser/ui/views/chrome_views_delegate.cc
@@ -99,14 +99,14 @@ PrefService* GetPrefsForWindow(const views::Widget* window) {
 }
 
 #if defined(OS_WIN)
-bool MonitorHasTopmostAutohideTaskbarForEdge(UINT edge, HMONITOR monitor) {
+bool MonitorHasAutohideTaskbarForEdge(UINT edge, HMONITOR monitor) {
   APPBARDATA taskbar_data = { sizeof(APPBARDATA), NULL, 0, edge };
   taskbar_data.hWnd = ::GetForegroundWindow();
 
   // TODO(robliao): Remove ScopedTracker below once crbug.com/462368 is fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
-          "462368 MonitorHasTopmostAutohideTaskbarForEdge"));
+          "462368 MonitorHasAutohideTaskbarForEdge"));
 
   // MSDN documents an ABM_GETAUTOHIDEBAREX, which supposedly takes a monitor
   // rect and returns autohide bars on that monitor.  This sounds like a good
@@ -139,8 +139,7 @@ bool MonitorHasTopmostAutohideTaskbarForEdge(UINT edge, HMONITOR monitor) {
       taskbar = taskbar_data.hWnd;
   }
 
-  if (::IsWindow(taskbar) &&
-      (GetWindowLong(taskbar, GWL_EXSTYLE) & WS_EX_TOPMOST)) {
+  if (::IsWindow(taskbar)) {
     if (MonitorFromWindow(taskbar, MONITOR_DEFAULTTONEAREST) == monitor)
       return true;
     // In some cases like when the autohide taskbar is on the left of the
@@ -159,13 +158,13 @@ int GetAppbarAutohideEdgesOnWorkerThread(HMONITOR monitor) {
   DCHECK(monitor);
 
   int edges = 0;
-  if (MonitorHasTopmostAutohideTaskbarForEdge(ABE_LEFT, monitor))
+  if (MonitorHasAutohideTaskbarForEdge(ABE_LEFT, monitor))
     edges |= views::ViewsDelegate::EDGE_LEFT;
-  if (MonitorHasTopmostAutohideTaskbarForEdge(ABE_TOP, monitor))
+  if (MonitorHasAutohideTaskbarForEdge(ABE_TOP, monitor))
     edges |= views::ViewsDelegate::EDGE_TOP;
-  if (MonitorHasTopmostAutohideTaskbarForEdge(ABE_RIGHT, monitor))
+  if (MonitorHasAutohideTaskbarForEdge(ABE_RIGHT, monitor))
     edges |= views::ViewsDelegate::EDGE_RIGHT;
-  if (MonitorHasTopmostAutohideTaskbarForEdge(ABE_BOTTOM, monitor))
+  if (MonitorHasAutohideTaskbarForEdge(ABE_BOTTOM, monitor))
     edges |= views::ViewsDelegate::EDGE_BOTTOM;
   return edges;
 }
@@ -467,6 +466,29 @@ std::string ChromeViewsDelegate::GetApplicationName() {
 }
 
 #if defined(OS_WIN)
+// We use the SHAppBarMessage API to get the taskbar autohide state. This API

[Peter Kasting, 2016/12/02 01:56:41]

Perhaps this comment should be moved up to where we had the TOPMOST check, and
start with something like "On Windows versions earlier than Windows 7, taskbars
could easily be made always-on-top or not, in which case we'd only want to
return a taskbar with WS_EX_TOPMOST set.  However, this causes problems."  Then
you could say most of the rest of the comment, but modify point 4 and the
closing paragraph a bit.

[ananta, 2016/12/02 06:09:14]

Done.

+// spins a modal loop which could cause callers to be reentered. This may not
+// be desirable and to avoid that we retrieve the taskbar state in a worker
+// thread. This fixes the reentrancy problems but introduces subtle timing
+// issues like the one below:
+// 1. A maximized chrome window is fullscreened.
+// 2. It is switched back to maximized.
+// 3. In the process the window gets a WM_NCCACLSIZE message which calls us to
+//    get the autohide state.
+// 4. The worker thread is invoked and the main thread is suspended. The worker

[Peter Kasting, 2016/12/02 01:56:41]

Is the main thread always suspended, or is it just that the worrisome case is
the one when the main thread is for some reason suspended?

[ananta, 2016/12/02 06:09:14]

The latter.

+//    thread calls the API to get the autohide state. The code also checked
+//    whether the taskbar was a topmost window. That fails in this case because
+//    the main window on the main thread is still in the process of switching
+//    away from fullscreen.
+// 5. The main thread resumes and does not leave space for the taskbar and
+//    hence it does not popup up when hovered on.
+//
+// To fix point 4 above we removed the check for the WS_EX_TOPMOST style as
+// that is the default on Windows 7+. Additionally we also want to ensure
+// that we query the taskbar state at the point when a window switches fully
+// away from fullscreen. The best time appears to be when we receive the
+// ABN_FULLSCREENAPP notification from the shell. Please refer to the
+// HWNDMessageHandler::OnAppBarMessage() for the code.
 int ChromeViewsDelegate::GetAppbarAutohideEdges(HMONITOR monitor,
                                                 const base::Closure& callback) {
   // Initialize the map with EDGE_BOTTOM. This is important, as if we return an