Fix dragging supported files to tab strip.

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

Index: chrome/browser/ui/views/tabs/tab_strip.cc
diff --git a/chrome/browser/ui/views/tabs/tab_strip.cc b/chrome/browser/ui/views/tabs/tab_strip.cc
index 1308d240a57f28d5ff6569fb8f77e5044af05327..13d3c7f0d0c810746a6de7bbee3f5e9379d85009 100644
--- a/chrome/browser/ui/views/tabs/tab_strip.cc
+++ b/chrome/browser/ui/views/tabs/tab_strip.cc
@@ -990,6 +990,11 @@ void TabStrip::StopAnimating(bool 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();
 }
@@ -1400,10 +1405,28 @@ void TabStrip::OnDragEntered(const DropTargetEvent& event) {
   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);
 }
 
@@ -1415,18 +1438,19 @@ 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);
 }
@@ -2432,7 +2456,8 @@ TabStrip::DropInfo::DropInfo(int drop_index,
                              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));