Fix dragging supported files to tab strip.

chrome/browser/ui/views/tabs/tab_strip.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_strip.h"
 
 #if defined(OS_WIN)
 #include <windowsx.h>
 #endif
 
@@ skipping 972 matching lines @@
 void TabStrip::StopAnimating(bool layout) {
   if (!IsAnimating())
     return;
 
   bounds_animator_.Cancel();
 
   if (layout)
     DoLayout();
 }
 
+void TabStrip::FileSupported(const GURL& url, bool supported) {
+  if (drop_info_->url == url)

[sky, 2013/11/26 00:12:29]

How do you know drop_info_ is non-null?

[michaelpg, 2013/11/26 04:50:13]

It definitely could be null, thanks.

+    drop_info_->file_supported = supported;

[sky, 2013/11/26 00:12:29]

If supported is false should we destroy drop_info_?

[michaelpg, 2013/11/26 04:50:13]

No, it would just get re-created in SetDropIndex. We also want to persist
drop_info_ because we want to support spring-loaded tabs, i.e., you can hold an
unsupported file over a tab like imgur.com to switch to that tab and then drop
it onto that page.

+}
+
 const ui::ListSelectionModel& TabStrip::GetSelectionModel() {
   return controller_->GetSelectionModel();
 }
 
 bool TabStrip::SupportsMultipleSelection() {
   // TODO: currently only allow single selection in touch layout mode.
   return touch_layout_.get() == NULL;
 }
 
 void TabStrip::SelectTab(Tab* tab) {
@@ skipping 390 matching lines @@
   if (immersive_style_)
     return gfx::Size(needed_width, Tab::GetImmersiveHeight());
   return gfx::Size(needed_width, Tab::GetMinimumUnselectedSize().height());
 }
 
 void TabStrip::OnDragEntered(const DropTargetEvent& event) {
   // Force animations to stop, otherwise it makes the index calculation tricky.
   StopAnimating(true);
 
   UpdateDropIndex(event);
+
+  GURL url;
+  string16 title;
+
+  // Check whether the event data includes supported drop data.
+  if (event.data().GetURLAndTitle(&url, &title) && url.is_valid()) {
+    drop_info_->url = url;
+
+    // For file:// URLs, kick off a MIME type request in case they're dropped.
+    if (url.SchemeIsFile())
+      controller()->CheckFileSupported(url);
+  } else {
+    drop_info_->file_supported = false;

[sky, 2013/11/26 00:12:29]

Can't we set drop_info_ to NULL here?

[michaelpg, 2013/11/26 04:50:13]

No, we need drop_info in case text is being dragged (unless of course the text
being dragged starts with "file:").

Also because we could be dragging text, we shouldn't even set file_supported =
false here.

+  }
 }
 
 int TabStrip::OnDragUpdated(const DropTargetEvent& event) {
   UpdateDropIndex(event);
+
+  if (!drop_info_->file_supported)
+    return ui::DragDropTypes::DRAG_NONE;
+
   return GetDropEffect(event);
 }
 
 void TabStrip::OnDragExited() {
   SetDropIndex(-1, false);
 }
 
 int TabStrip::OnPerformDrop(const DropTargetEvent& event) {
   if (!drop_info_.get())
     return ui::DragDropTypes::DRAG_NONE;
 
+  if (!drop_info_->file_supported) {
+    SetDropIndex(-1, false);
+    return ui::DragDropTypes::DRAG_NONE;
+  }
+
   const int drop_index = drop_info_->drop_index;
   const bool drop_before = drop_info_->drop_before;
+  const GURL drop_url = drop_info_->url;
 
   // Hide the drop indicator.
   SetDropIndex(-1, false);
 
-  GURL url;
-  string16 title;
-  if (!event.data().GetURLAndTitle(&url, &title) || !url.is_valid())
-    return ui::DragDropTypes::DRAG_NONE;
-
-  controller()->PerformDrop(drop_before, drop_index, url);
+  controller()->PerformDrop(drop_before, drop_index, drop_url);
 
   return GetDropEffect(event);
 }
 
 void TabStrip::GetAccessibleState(ui::AccessibleViewState* state) {
   state->role = ui::AccessibilityTypes::ROLE_PAGETABLIST;
 }
 
 views::View* TabStrip::GetEventHandlerForRect(const gfx::Rect& rect) {
   if (!views::UsePointBasedTargeting(rect))
@@ skipping 985 matching lines @@
 }
 
 // TabStrip::DropInfo ----------------------------------------------------------
 
 TabStrip::DropInfo::DropInfo(int drop_index,
                              bool drop_before,
                              bool point_down,
                              views::Widget* context)
     : drop_index(drop_index),
       drop_before(drop_before),
-      point_down(point_down) {
+      point_down(point_down),
+      file_supported(true) {

[sky, 2013/11/26 00:12:29]

Is initializing to true really the right thing? What if the drag completes
before we figure out the type?

[michaelpg, 2013/11/26 04:50:13]

I would rather we try to open it even if it means creating a copy of the file to
"redownload" instead of doing nothing or waiting on the IO which in this
hypothetical would be slower than the user.

   arrow_view = new views::ImageView;
   arrow_view->SetImage(GetDropArrowImage(point_down));
 
   arrow_window = new views::Widget;
   views::Widget::InitParams params(views::Widget::InitParams::TYPE_POPUP);
   params.keep_on_top = true;
   params.opacity = views::Widget::InitParams::TRANSLUCENT_WINDOW;
   params.accept_events = false;
   params.can_activate = false;
   params.bounds = gfx::Rect(drop_indicator_width, drop_indicator_height);
@@ skipping 297 matching lines @@
 #if defined(USE_AURA)
   return chrome::GetHostDesktopTypeForNativeView(
       GetWidget()->GetNativeView()) == chrome::HOST_DESKTOP_TYPE_ASH;
 #else
   if (ui::GetDisplayLayout() != ui::LAYOUT_TOUCH)
     return false;
 #endif
 
   return true;
 }