Fix the HTML5 drag and drop demos on Linux and CrOS

content/browser/web_contents/web_contents_view_aura.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 "content/browser/web_contents/web_contents_view_aura.h"
 
 #include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/metrics/histogram.h"
@@ skipping 351 matching lines @@
   provider->MarkOriginatedFromRenderer();
 #if defined(OS_WIN)
   // Put download before file contents to prefer the download of a image over
   // its thumbnail link.
   if (!drop_data.download_metadata.empty())
     PrepareDragForDownload(drop_data, provider, web_contents);
 #endif
 #if (!defined(OS_CHROMEOS) && defined(USE_X11)) || defined(OS_WIN)
   // We set the file contents before the URL because the URL also sets file
   // contents (to a .URL shortcut).  We want to prefer file content data over
   // a shortcut so we add it first.

[pkotwicz, 2014/06/20 22:12:01]

I am not changing this comment because it is not a lie wrt to
OSExchangeDataProviderWin().

As part of a separate CL, it may be good to change OSExchangeDataProviderWin to
have a consistent behavior wrt to duplicate calls to
OSExchangeDataProviderWin::SetString() and
OSExchangeDataProviderWin::SetData()

[pkotwicz, 2014/06/20 22:17:25]

I take the comment about consistency back. SetData() is an implementation of the
win32 IDataObject interface while SetString() is not

[Peter Kasting, 2014/06/20 22:26:22]

All I can say about this comment is that even after both your notes here I have
no idea what it means and it sounds just as confusing as the other comment I had
you changed.

"We set A before calling foo(), because foo() also sets A, and we want to prefer
our value to foo()'s" sounds inherently contradictory.

If foo() checks whether A has been set, use text like what I say below to
indicate that.

   if (!drop_data.file_contents.empty())
     PrepareDragForFileContents(drop_data, provider);
 #endif
+  // Call SetString() before SetURL() when we actually have a custom string.
+  // SetURL() will itself do SetString() when a string hasn't been set yet,
+  // but we want to prefer drop_data.text.string() over the URL string if it
+  // exists.
   if (!drop_data.text.string().empty())
     provider->SetString(drop_data.text.string());
   if (drop_data.url.is_valid())
     provider->SetURL(drop_data.url, drop_data.url_title);
   if (!drop_data.html.string().empty())
     provider->SetHtml(drop_data.html.string(), drop_data.html_base_url);
   if (!drop_data.filenames.empty())
     provider->SetFilenames(drop_data.filenames);
   if (!drop_data.file_system_files.empty()) {
     Pickle pickle;
@@ skipping 1183 matching lines @@
 void WebContentsViewAura::OnWindowVisibilityChanged(aura::Window* window,
                                                     bool visible) {
   // Ignore any visibility changes in the hierarchy below.
   if (window != window_.get() && window_->Contains(window))
     return;
 
   UpdateWebContentsVisibility(web_contents_, visible);
 }
 
 }  // namespace content