Add WebDragData to blink::WebView::dragtargetDrop

content/browser/renderer_host/render_view_host_impl.cc

Index: content/browser/renderer_host/render_view_host_impl.cc
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 64ddccaff3a5946f1bd00dca5bc888c1ede50c47..4bbde3a7c8c0598e86a94362e907dab124cadcaf 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -80,6 +80,7 @@
 #include "net/url_request/url_request_context_getter.h"
 #include "storage/browser/fileapi/isolated_context.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/base/clipboard/clipboard.h"
 #include "ui/base/touch/touch_device.h"
 #include "ui/base/touch/touch_enabled.h"
 #include "ui/base/ui_base_switches.h"
@@ -164,6 +165,49 @@ void GetWindowsSpecificPrefs(RendererPreferences* prefs) {
 }
 #endif
 
+std::vector<MimeTypeKindPair> DropDataToMetaData(const DropData& drop_data) {
+  std::vector<MimeTypeKindPair> meta_data;
+  if (!drop_data.text.is_null()) {
+    MimeTypeKindPair pair = std::make_pair(
+        base::ASCIIToUTF16(ui::Clipboard::kMimeTypeText), STRING_KIND);
+    meta_data.push_back(pair);

[dcheng, 2016/04/19 06:58:47]

I think it would be appropriate to use emplace_back here.

meta_data.emplace_back(base::ASCIIToUTF16(ui::Clipboard::kMimeTypeText),
STRING_KIND));

etc

[hush (inactive), 2016/05/06 02:25:25]

Done. And I changed "MimeTypeKindPair" into a "MetaData" struct, because it is
not simply a string to string pair any more.

+  }
+
+  if (!drop_data.url.is_empty()) {
+    MimeTypeKindPair pair = std::make_pair(
+        base::ASCIIToUTF16(ui::Clipboard::kMimeTypeURIList), STRING_KIND);
+    meta_data.push_back(pair);
+  }
+
+  if (!drop_data.html.is_null()) {
+    MimeTypeKindPair pair = std::make_pair(
+        base::ASCIIToUTF16(ui::Clipboard::kMimeTypeHTML), STRING_KIND);
+    meta_data.push_back(pair);
+  }
+
+  for (const auto& file_info : drop_data.filenames) {
+    if (!file_info.path.empty()) {
+      MimeTypeKindPair pair = std::make_pair(base::string16(), FILENAME_KIND);
+      meta_data.push_back(pair);
+    }
+  }
+
+  for (const auto& file_system_file : drop_data.file_system_files) {
+    if (!file_system_file.url.is_empty()) {
+      MimeTypeKindPair pair =
+          std::make_pair(base::string16(), FILESYSTEMFILE_KIND);

[dcheng, 2016/04/19 06:58:47]

Unfortunately, making things complicated, I think we might actually want to
plumb through all the information for drop_data.filenames and
drop_data.file_system_files. Otherwise, the MIME type plumbing for
DataTransferItem won't work:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...,
etc.

[hush (inactive), 2016/04/28 22:13:26]

Right... This also means that we just can't do files correctly on Android
platform. The reason is we don't know the exact mime type of the file. I just
shared a public doc with you about this to elaborate. (Android N multi window is
already public in preview)

Our best guess on the mime type during DragEnter, is the mime type from
ClipDescription. Well, this is usually "text/uri-list" which means a Uri to a
file is being dragged, or something random set by the drag source app.

[dcheng, 2016/05/04 05:21:34]

I think it's OK if it doesn't work perfectly on Android, but we should try to
keep the existing bits that already work functional.

[hush (inactive), 2016/05/06 02:25:25]

Done.

+      meta_data.push_back(pair);
+    }
+  }
+
+  for (const auto& custom_data_item : drop_data.custom_data) {
+    MimeTypeKindPair pair = std::make_pair(custom_data_item.first, STRING_KIND);
+    meta_data.push_back(pair);
+  }
+
+  return meta_data;
+}
+
 }  // namespace
 
 // static
@@ -591,11 +635,11 @@ void RenderViewHostImpl::DragTargetDragEnter(
     const gfx::Point& screen_pt,
     WebDragOperationsMask operations_allowed,
     int key_modifiers) {
+#if defined(OS_CHROMEOS)

[dcheng, 2016/04/19 06:58:47]

This block should also be in FilterDropData.

[hush (inactive), 2016/05/06 02:25:25]

Done.

   const int renderer_id = GetProcess()->GetID();
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
 
-#if defined(OS_CHROMEOS)
   // The externalfile:// scheme is used in Chrome OS to open external files in a
   // browser tab.
   if (drop_data.url.SchemeIs(content::kExternalFileScheme))
@@ -605,82 +649,11 @@ void RenderViewHostImpl::DragTargetDragEnter(
   // The URL could have been cobbled together from any highlighted text string,
   // and can't be interpreted as a capability.

[dcheng, 2016/04/19 06:58:47]

This comment should be moved into FilterDropData

[hush (inactive), 2016/05/06 02:25:25]

Done.

   DropData filtered_data(drop_data);
-  GetProcess()->FilterURL(true, &filtered_data.url);
-  if (drop_data.did_originate_from_renderer) {
-    filtered_data.filenames.clear();
-  }
-
-  // The filenames vector, on the other hand, does represent a capability to
-  // access the given files.
-  storage::IsolatedContext::FileInfoSet files;
-  for (std::vector<ui::FileInfo>::iterator iter(
-           filtered_data.filenames.begin());
-       iter != filtered_data.filenames.end();
-       ++iter) {
-    // A dragged file may wind up as the value of an input element, or it
-    // may be used as the target of a navigation instead.  We don't know
-    // which will happen at this point, so generously grant both access
-    // and request permissions to the specific file to cover both cases.
-    // We do not give it the permission to request all file:// URLs.
-
-    // Make sure we have the same display_name as the one we register.
-    if (iter->display_name.empty()) {
-      std::string name;
-      files.AddPath(iter->path, &name);
-      iter->display_name = base::FilePath::FromUTF8Unsafe(name);
-    } else {
-      files.AddPathWithName(iter->path, iter->display_name.AsUTF8Unsafe());
-    }
-
-    policy->GrantRequestSpecificFileURL(renderer_id,
-                                        net::FilePathToFileURL(iter->path));
-
-    // If the renderer already has permission to read these paths, we don't need
-    // to re-grant them. This prevents problems with DnD for files in the CrOS
-    // file manager--the file manager already had read/write access to those
-    // directories, but dragging a file would cause the read/write access to be
-    // overwritten with read-only access, making them impossible to delete or
-    // rename until the renderer was killed.
-    if (!policy->CanReadFile(renderer_id, iter->path))
-      policy->GrantReadFile(renderer_id, iter->path);
-  }
-
-  storage::IsolatedContext* isolated_context =
-      storage::IsolatedContext::GetInstance();
-  DCHECK(isolated_context);
-  std::string filesystem_id = isolated_context->RegisterDraggedFileSystem(
-      files);
-  if (!filesystem_id.empty()) {
-    // Grant the permission iff the ID is valid.
-    policy->GrantReadFileSystem(renderer_id, filesystem_id);
-  }
-  filtered_data.filesystem_id = base::UTF8ToUTF16(filesystem_id);
-
-  storage::FileSystemContext* file_system_context =
-      BrowserContext::GetStoragePartition(GetProcess()->GetBrowserContext(),
-                                          GetSiteInstance())
-          ->GetFileSystemContext();
-  for (size_t i = 0; i < filtered_data.file_system_files.size(); ++i) {
-    storage::FileSystemURL file_system_url =
-        file_system_context->CrackURL(filtered_data.file_system_files[i].url);
-
-    std::string register_name;
-    std::string filesystem_id = isolated_context->RegisterFileSystemForPath(
-        file_system_url.type(), file_system_url.filesystem_id(),
-        file_system_url.path(), &register_name);
-    policy->GrantReadFileSystem(renderer_id, filesystem_id);
-
-    // Note: We are using the origin URL provided by the sender here. It may be
-    // different from the receiver's.
-    filtered_data.file_system_files[i].url =
-        GURL(storage::GetIsolatedFileSystemRootURIString(
-                 file_system_url.origin(), filesystem_id, std::string())
-                 .append(register_name));
-  }
+  FilterDropData(&filtered_data);

[dcheng, 2016/04/19 06:58:47]

Filtering grants privileges, etc, and it seems like most of these permission
grants should only happen in the drop.

1) Is it possible to refactor this interface to take ownership of the drop data
(e.g. pass by unique_ptr) so we only have to filter it once? Or is it possible
for the caller pre-filter before calling this? Would this work with the model
you have in mind for Android?

2) Let's split the permission grants out of filtering if possible.

[hush (inactive), 2016/05/06 02:25:25]

I refactored it by splitting into 2 parts:
A. register in isolated file systems and crack the urls
B. grant permissions 
Part A is done in DragEnter and B is done in Drop.

Android situation is quite different. We only have the Uri in Drop time. So no
"filtering" is needed at DragEnter. And we only need to read the Uri at Drop. No
permission granting is needed either. If I understand correctly, the point of
isolated context is to prevent the renderer from poking around in the OS' file
system, which is already achieved by Android's own permission granting
mechanisms. The permission to read the Uri is granted by the source app that the
drag initiates via a flag to DnD operation.

 
-  Send(new DragMsg_TargetDragEnter(GetRoutingID(), filtered_data, client_pt,
-                                   screen_pt, operations_allowed,
-                                   key_modifiers));
+  Send(new DragMsg_TargetDragEnter(
+      GetRoutingID(), DropDataToMetaData(filtered_data), client_pt, screen_pt,
+      operations_allowed, key_modifiers));
 }
 
 void RenderViewHostImpl::DragTargetDragOver(
@@ -696,12 +669,14 @@ void RenderViewHostImpl::DragTargetDragLeave() {
   Send(new DragMsg_TargetDragLeave(GetRoutingID()));
 }
 
-void RenderViewHostImpl::DragTargetDrop(
-    const gfx::Point& client_pt,
-    const gfx::Point& screen_pt,
-    int key_modifiers) {
-  Send(new DragMsg_TargetDrop(GetRoutingID(), client_pt, screen_pt,
-                              key_modifiers));
+void RenderViewHostImpl::DragTargetDrop(const DropData& drop_data,
+                                        const gfx::Point& client_pt,
+                                        const gfx::Point& screen_pt,
+                                        int key_modifiers) {
+  DropData filtered_data(drop_data);
+  FilterDropData(&filtered_data);
+  Send(new DragMsg_TargetDrop(GetRoutingID(), filtered_data, client_pt,
+                              screen_pt, key_modifiers));
 }
 
 void RenderViewHostImpl::DragSourceEndedAt(
@@ -1355,4 +1330,81 @@ void RenderViewHostImpl::RenderViewReady() {
   delegate_->RenderViewReady(this);
 }
 
+void RenderViewHostImpl::FilterDropData(DropData* filtered_data) {
+  const int renderer_id = GetProcess()->GetID();
+  ChildProcessSecurityPolicyImpl* policy =
+      ChildProcessSecurityPolicyImpl::GetInstance();
+
+  GetProcess()->FilterURL(true, &filtered_data->url);
+  if (filtered_data->did_originate_from_renderer) {
+    filtered_data->filenames.clear();
+  }
+
+  // The filenames vector, on the other hand, does represent a capability to
+  // access the given files.
+  storage::IsolatedContext::FileInfoSet files;
+  for (auto& filename : filtered_data->filenames) {
+    // A dragged file may wind up as the value of an input element, or it
+    // may be used as the target of a navigation instead.  We don't know
+    // which will happen at this point, so generously grant both access
+    // and request permissions to the specific file to cover both cases.
+    // We do not give it the permission to request all file:// URLs.
+
+    // Make sure we have the same display_name as the one we register.
+    if (filename.display_name.empty()) {
+      std::string name;
+      files.AddPath(filename.path, &name);
+      filename.display_name = base::FilePath::FromUTF8Unsafe(name);
+    } else {
+      files.AddPathWithName(filename.path,
+                            filename.display_name.AsUTF8Unsafe());
+    }
+
+    policy->GrantRequestSpecificFileURL(renderer_id,
+                                        net::FilePathToFileURL(filename.path));
+
+    // If the renderer already has permission to read these paths, we don't need
+    // to re-grant them. This prevents problems with DnD for files in the CrOS
+    // file manager--the file manager already had read/write access to those
+    // directories, but dragging a file would cause the read/write access to be
+    // overwritten with read-only access, making them impossible to delete or
+    // rename until the renderer was killed.
+    if (!policy->CanReadFile(renderer_id, filename.path))
+      policy->GrantReadFile(renderer_id, filename.path);
+  }
+
+  storage::IsolatedContext* isolated_context =
+      storage::IsolatedContext::GetInstance();
+  DCHECK(isolated_context);
+  std::string filesystem_id =
+      isolated_context->RegisterDraggedFileSystem(files);
+  if (!filesystem_id.empty()) {
+    // Grant the permission iff the ID is valid.
+    policy->GrantReadFileSystem(renderer_id, filesystem_id);
+  }
+  filtered_data->filesystem_id = base::UTF8ToUTF16(filesystem_id);
+
+  storage::FileSystemContext* file_system_context =
+      BrowserContext::GetStoragePartition(GetProcess()->GetBrowserContext(),
+                                          GetSiteInstance())
+          ->GetFileSystemContext();
+  for (auto& file_system_file : filtered_data->file_system_files) {
+    storage::FileSystemURL file_system_url =
+        file_system_context->CrackURL(file_system_file.url);
+
+    std::string register_name;
+    std::string filesystem_id = isolated_context->RegisterFileSystemForPath(
+        file_system_url.type(), file_system_url.filesystem_id(),
+        file_system_url.path(), &register_name);
+    policy->GrantReadFileSystem(renderer_id, filesystem_id);
+
+    // Note: We are using the origin URL provided by the sender here. It may be
+    // different from the receiver's.
+    file_system_file.url =
+        GURL(storage::GetIsolatedFileSystemRootURIString(
+                 file_system_url.origin(), filesystem_id, std::string())
+                 .append(register_name));
+  }
+}
+
 }  // namespace content