ChromeOS MD: Fix cannot drag tab onto immersive fullscreen window

chrome/browser/ui/views/tabs/tab_drag_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/tabs/tab_drag_controller.h"
 
 #include <math.h>
 #include <set>
 
 #include "base/auto_reset.h"
@@ skipping 805 matching lines @@
   gfx::NativeWindow local_window;
   const Liveness state = GetLocalProcessWindow(
       point_in_screen, is_dragging_window_, &local_window);
   if (state == Liveness::DELETED)
     return Liveness::DELETED;
 
   // Do not allow dragging into a window with a modal dialog, it causes a weird
   // behavior.  See crbug.com/336691
   if (!GetModalTransient(local_window)) {
     TabStrip* result = GetTabStripForWindow(local_window);
-    if (result && DoesTabStripContain(result, point_in_screen)) {
+    if (result && DoesTabStripContain(result, point_in_screen, local_window)) {
       *tab_strip = result;
       return Liveness::ALIVE;
     }
   }
 
   *tab_strip = is_dragging_window_ ? attached_tabstrip_ : nullptr;
   return Liveness::ALIVE;
 }
 
 TabStrip* TabDragController::GetTabStripForWindow(gfx::NativeWindow window) {
   if (!window)
     return NULL;
   BrowserView* browser_view =
       BrowserView::GetBrowserViewForNativeWindow(window);
   // We don't allow drops on windows that don't have tabstrips.
   if (!browser_view ||
       !browser_view->browser()->SupportsWindowFeature(
           Browser::FEATURE_TABSTRIP))
     return NULL;
 
   TabStrip* other_tabstrip = browser_view->tabstrip();
   TabStrip* tab_strip =
       attached_tabstrip_ ? attached_tabstrip_ : source_tabstrip_;
   DCHECK(tab_strip);
 
   return other_tabstrip->controller()->IsCompatibleWith(tab_strip) ?
       other_tabstrip : NULL;
 }
 
-bool TabDragController::DoesTabStripContain(
-    TabStrip* tabstrip,
-    const gfx::Point& point_in_screen) const {
+bool TabDragController::DoesTabStripContain(TabStrip* tabstrip,
+                                            const gfx::Point& point_in_screen,
+                                            gfx::NativeWindow window) const {
   // Make sure the specified screen point is actually within the bounds of the
-  // specified tabstrip...
+  // specified tabstrip.
   gfx::Rect tabstrip_bounds = GetViewScreenBounds(tabstrip);
-  return point_in_screen.x() < tabstrip_bounds.right() &&
+#if defined(OS_CHROMEOS)

[sky, 2017/01/17 18:07:01]

Why does this need to be chromeos specific? ImmersiveModeController isn't
chromeos specific. Also, rather than calling ShouldHideTopViews() can you check
BrowserView::IsTabStripVisible?

[Qiang(Joe) Xu, 2017/01/18 01:13:52]

done

+  DCHECK(window);
+  BrowserView* browser_view =
+      BrowserView::GetBrowserViewForNativeWindow(window);
+  DCHECK(browser_view);
+  bool hidden_top_views =
+      browser_view->immersive_mode_controller()->ShouldHideTopViews();
+  // When |window| is immersive non-revealed mode, get |tabstrip_bounds| from
+  // non-client view coordinates where tabstrip should be laid when revealed.
+  if (hidden_top_views) {
+    tabstrip_bounds = browser_view->frame()->GetBoundsForTabStrip(tabstrip);
+    aura::Window::ConvertRectToTarget(window, window->GetRootWindow(),
+                                      &tabstrip_bounds);
+  }
+#endif
+
+  bool res =
+      point_in_screen.x() < tabstrip_bounds.right() &&
       point_in_screen.x() >= tabstrip_bounds.x() &&
-      DoesRectContainVerticalPointExpanded(tabstrip_bounds,
-                                           kVerticalDetachMagnetism,
-                                           point_in_screen.y());
+      DoesRectContainVerticalPointExpanded(
+          tabstrip_bounds, kVerticalDetachMagnetism, point_in_screen.y());
+#if defined(OS_CHROMEOS)
+  if (res && hidden_top_views) {
+    tabstrip->SetVisible(true);

[sky, 2017/01/17 18:07:01]

This function is meant to check whether the tabstrip contains a point. It
shouldn't have side effects. If you need to change visibility that should happen
else where.

[Qiang(Joe) Xu, 2017/01/18 01:13:52]

done by moving to DragBrowserToNewTabStrip

+    tabstrip->SetBoundsRect(tabstrip_bounds);
+  }
+#endif
+  return res;
 }
 
 void TabDragController::Attach(TabStrip* attached_tabstrip,
                                const gfx::Point& point_in_screen) {
   TRACE_EVENT1("views", "TabDragController::Attach",
                "point_in_screen", point_in_screen.ToString());
 
   DCHECK(!attached_tabstrip_);  // We should already have detached by the time
                                 // we get here.
 
@@ skipping 918 matching lines @@
   // TODO(pkotwicz): Fix this properly (crbug.com/358482)
   for (auto* browser : *BrowserList::GetInstance()) {
     if (browser->tab_strip_model()->empty())
       exclude.insert(browser->window()->GetNativeWindow());
   }
 #endif
   base::WeakPtr<TabDragController> ref(weak_factory_.GetWeakPtr());
   *window = window_finder_->GetLocalProcessWindowAtPoint(screen_point, exclude);
   return ref ? Liveness::ALIVE : Liveness::DELETED;
 }