Files.app: Let Files.app pass mutliple files to file handlers.

apps/launcher.cc

 // Copyright 2013 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 "apps/launcher.h"
 
 #include "apps/browser/api/app_runtime/app_runtime_api.h"
 #include "apps/browser/file_handler_util.h"
 #include "apps/common/api/app_runtime.h"
 #include "base/command_line.h"
@@ skipping 71 matching lines @@
 }
 
 // Helper method to launch the platform app |extension| with no data. This
 // should be called in the fallback case, where it has been impossible to
 // load or obtain file launch data.
 void LaunchPlatformAppWithNoData(Profile* profile, const Extension* extension) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   AppEventRouter::DispatchOnLaunchedEvent(profile, extension);
 }
 
-// Class to handle launching of platform apps to open a specific path.
+// Class to handle launching of platform apps to open specific paths.
 // An instance of this class is created for each launch. The lifetime of these
 // instances is managed by reference counted pointers. As long as an instance
 // has outstanding tasks on a message queue it will be retained; once all
 // outstanding tasks are completed it will be deleted.
 class PlatformAppPathLauncher
     : public base::RefCountedThreadSafe<PlatformAppPathLauncher> {
  public:
   PlatformAppPathLauncher(Profile* profile,
                           const Extension* extension,
+                          const std::vector<base::FilePath>& file_paths)
+      : profile_(profile), extension_(extension), file_paths_(file_paths) {}
+
+  PlatformAppPathLauncher(Profile* profile,
+                          const Extension* extension,
                           const base::FilePath& file_path)
-      : profile_(profile), extension_(extension), file_path_(file_path) {}
+      : profile_(profile), extension_(extension) {
+    if (!file_path.empty())
+      file_paths_.push_back(file_path);
+  }
 
   void Launch() {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
-    if (file_path_.empty()) {
+    if (file_paths_.empty()) {
       LaunchPlatformAppWithNoData(profile_, extension_);
       return;
     }
 
-    DCHECK(file_path_.IsAbsolute());
+    for (size_t i = 0; i < file_paths_.size(); ++i) {
+      DCHECK(file_paths_[i].IsAbsolute());
+    }
 
     if (HasFileSystemWritePermission(extension_)) {
-      std::vector<base::FilePath> paths;
-      paths.push_back(file_path_);
       CheckWritableFiles(
-          paths,
+          file_paths_,
           profile_,
           false,
           base::Bind(&PlatformAppPathLauncher::OnFileValid, this),
           base::Bind(&PlatformAppPathLauncher::OnFileInvalid, this));
       return;
     }
 
     OnFileValid();
   }
 
@@ skipping 12 matching lines @@
   }
 
  private:
   friend class base::RefCountedThreadSafe<PlatformAppPathLauncher>;
 
   virtual ~PlatformAppPathLauncher() {}
 
   void MakePathAbsolute(const base::FilePath& current_directory) {
     DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-    if (!DoMakePathAbsolute(current_directory, &file_path_)) {
-      LOG(WARNING) << "Cannot make absolute path from " << file_path_.value();
-      file_path_ = base::FilePath();
+    for (std::vector<base::FilePath>::iterator it = file_paths_.begin();
+         it != file_paths_.end();
+         ++it) {
+      if (!DoMakePathAbsolute(current_directory, &*it)) {
+        LOG(WARNING) << "Cannot make absolute path from " << it->value();
+        BrowserThread::PostTask(
+            BrowserThread::UI,
+            FROM_HERE,
+            base::Bind(&PlatformAppPathLauncher::LaunchWithNoLaunchData, this));
+        return;
+      }
     }
 
     BrowserThread::PostTask(BrowserThread::UI,
                             FROM_HERE,
                             base::Bind(&PlatformAppPathLauncher::Launch, this));
   }
 
   void OnFileValid() {
-#if defined(OS_CHROMEOS)
-    if (file_manager::util::IsUnderNonNativeLocalPath(profile_, file_path_)) {
-      file_manager::util::GetNonNativeLocalPathMimeType(
-          profile_,
-          file_path_,
-          base::Bind(&PlatformAppPathLauncher::OnGotMimeType, this));
-      return;
-    }
-#endif
-
-    BrowserThread::PostTask(
-        BrowserThread::FILE,
-        FROM_HERE,
-        base::Bind(&PlatformAppPathLauncher::GetMimeTypeAndLaunch, this));
+    mime_types_.resize(file_paths_.size());
+    StepToFillMimeTypes();

[benwells, 2014/05/30 00:59:35]

Sorry to be so nitpicky. I'm finding this logic pretty confusing. I don't think
it will deal with mixed non-native and native files on chromeos but maybe i'm
wrong. I don't think it has to deal with that case but that should be more
explicit.

Can you do this:
- remove the functionStepToFillMimeTypes
- instead, in OnFileValid, if onchromeos check if all files are nonnative or
native. dcheck if mixed.
- if they are all nonnative call a function GetNextNonNativeMimeType (like
StepToFillMimeTypes but just for the next non native chromeos path)
- otherwise, in OnFileValid pass off to GetMimeTypesAndLaunch
- GetNextNonNativeMimeType and OnGotMimeType is chromeos only

[hirono, 2014/05/30 02:53:44]

Currently the mixed case should not happen, but it can after Files.app support a
cross-volume search feature. In the search result, users can select non-native
and native files and launch an application.

The current logic handles the mixed case as followings:

1. Prepare empty strings by mime_types_.resize(file_paths.size());
2. Check if file_types[i] is non-native. If so, it assigns a mime type string to
file_types_[i]. Otherwise it keeps file_types[i] as empty.
3. After checking all file_paths, it checks if all the mime_types_ are filled or
not. (filled variables shows it) If there is empty strings in mime_types_, it
calls GetMimeTypeAndLaunch to fill remaining mime_types_. Otherwise it call
LaunchWithMimeType directly.

The logic is a bit confusing. But I think this logic minimizes the number of
hopping threads for the three cases: native files, non-native files, mixed
files. WDYT?

[benwells, 2014/05/30 06:33:57]

Ah, OK. Hmm. Maybe with some comments / changed names this will be easier to
understand ...

Non-chromeos platforms don't need to do any of that iteration, they should be
able to jump straight to GetMimeTypesAndLaunch (maybe it should be called
GetNativeMimeTypesAndLaunch).

[hirono, 2014/06/02 03:33:25]

Done.

   }
 
   void OnFileInvalid(const base::FilePath& /* error_path */) {
     LaunchWithNoLaunchData();
   }
 
+  void StepToFillMimeTypes() {

[benwells, 2014/05/30 06:33:57]

I think the name GetNextNonNativeMimeType would be clearer.

[hirono, 2014/06/02 03:33:25]

Done.

+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+    bool filled = true;

[benwells, 2014/05/30 06:33:57]

Changing filled to any_native_files would be clearer.

[hirono, 2014/06/02 03:33:25]

Done.

+
+    for (size_t i = 0; i < mime_types_.size(); ++i) {
+      if (!mime_types_[i].empty())
+        continue;
+      const base::FilePath& file_path = file_paths_[i];
+#if defined(OS_CHROMEOS)
+      if (file_manager::util::IsUnderNonNativeLocalPath(profile_, file_path)) {
+        file_manager::util::GetNonNativeLocalPathMimeType(
+            profile_,
+            file_path,
+            base::Bind(&PlatformAppPathLauncher::OnGotMimeType, this, i));
+        return;
+      }
+#endif
+      filled = false;
+    }
+
+    if (!filled) {
+      BrowserThread::PostTask(
+          BrowserThread::FILE,
+          FROM_HERE,
+          base::Bind(&PlatformAppPathLauncher::GetMimeTypeAndLaunch, this));
+      return;
+    }
+
+    LaunchWithMimeType();
+  }
+
   void GetMimeTypeAndLaunch() {

[benwells, 2014/05/30 00:59:35]

Nit: GetMimeTypesAndLaunch

[hirono, 2014/06/02 03:33:25]

Done.

     DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
-    // If the file doesn't exist, or is a directory, launch with no launch data.
-    if (!base::PathExists(file_path_) ||
-        base::DirectoryExists(file_path_)) {
-      LOG(WARNING) << "No file exists with path " << file_path_.value();
-      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(
-              &PlatformAppPathLauncher::LaunchWithNoLaunchData, this));
-      return;
+    for (size_t i = 0; i < mime_types_.size(); ++i) {
+      if (!this->mime_types_[i].empty())

[benwells, 2014/05/30 00:59:35]

I think this would be a programming error. Maybe DCHECK.

[hirono, 2014/05/30 02:53:44]

It can happen, if the given files are mixed (native and non-native).
The previous logic fills mime types for non native paths, but does not for
native paths.

[benwells, 2014/05/30 06:33:57]

OK, great, thanks for explaining.

+        continue;
+      const base::FilePath& file_path = file_paths_[i];
+
+      // If the file doesn't exist, or is a directory, launch with no launch
+      // data.
+      if (!base::PathExists(file_path) || base::DirectoryExists(file_path)) {
+        LOG(WARNING) << "No file exists with path " << file_path.value();
+        BrowserThread::PostTask(
+            BrowserThread::UI,
+            FROM_HERE,
+            base::Bind(&PlatformAppPathLauncher::LaunchWithNoLaunchData, this));
+        return;
+      }
+
+      std::string mime_type;
+      if (!net::GetMimeTypeFromFile(file_path, &mime_type)) {
+        // If MIME type of the file can't be determined by its path,
+        // try to sniff it by its content.
+        std::vector<char> content(net::kMaxBytesToSniff);
+        int bytes_read = base::ReadFile(file_path, &content[0], content.size());
+        if (bytes_read >= 0) {
+          net::SniffMimeType(&content[0],
+                             bytes_read,
+                             net::FilePathToFileURL(file_path),
+                             std::string(),  // type_hint (passes no hint)
+                             &mime_type);
+        }
+        if (mime_type.empty())
+          mime_type = kFallbackMimeType;
+      }
+      mime_types_[i] = mime_type;
     }
 
-    std::string mime_type;
-    if (!net::GetMimeTypeFromFile(file_path_, &mime_type)) {
-      // If MIME type of the file can't be determined by its path,
-      // try to sniff it by its content.
-      std::vector<char> content(net::kMaxBytesToSniff);
-      int bytes_read = base::ReadFile(file_path_, &content[0], content.size());
-      if (bytes_read >= 0) {
-        net::SniffMimeType(&content[0],
-                           bytes_read,
-                           net::FilePathToFileURL(file_path_),
-                           std::string(),  // type_hint (passes no hint)
-                           &mime_type);
-      }
-      if (mime_type.empty())
-        mime_type = kFallbackMimeType;
-    }
-
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(
-            &PlatformAppPathLauncher::LaunchWithMimeType, this, mime_type));
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&PlatformAppPathLauncher::LaunchWithMimeType, this));
   }
 
 #if defined(OS_CHROMEOS)
-  void OnGotMimeType(bool success, const std::string& mime_type) {
+  void OnGotMimeType(size_t index, bool success, const std::string& mime_type) {
     if (!success) {
       LaunchWithNoLaunchData();
       return;
     }
-    LaunchWithMimeType(mime_type.empty() ? kFallbackMimeType : mime_type);
+    mime_types_[index] = mime_type.empty() ? kFallbackMimeType : mime_type;
+    StepToFillMimeTypes();
   }
 #endif
 
   void LaunchWithNoLaunchData() {
     // This method is required as an entry point on the UI thread.
     LaunchPlatformAppWithNoData(profile_, extension_);
   }
 
-  void LaunchWithMimeType(const std::string& mime_type) {
+  void LaunchWithMimeType() {

[benwells, 2014/05/30 00:59:35]

Nit: LaunchWithMimeTypes

[hirono, 2014/06/02 03:33:25]

Done.

+    DCHECK(file_paths_.size() == mime_types_.size());
+
     // Find file handler from the platform app for the file being opened.
     const extensions::FileHandlerInfo* handler = NULL;
-    if (!handler_id_.empty())
+    if (!handler_id_.empty()) {
       handler = FileHandlerForId(*extension_, handler_id_);
-    else
-      handler = FirstFileHandlerForFile(*extension_, mime_type, file_path_);
-    if (handler && !FileHandlerCanHandleFile(*handler, mime_type, file_path_)) {
-      LOG(WARNING) << "Extension does not provide a valid file handler for "
-                   << file_path_.value();
-      LaunchWithNoLaunchData();
-      return;
+      if (handler) {
+        for (size_t i = 0; i < file_paths_.size(); ++i) {
+          if (!FileHandlerCanHandleFile(
+                  *handler, mime_types_[i], file_paths_[i])) {
+            LOG(WARNING)
+                << "Extension does not provide a valid file handler for "
+                << file_paths_[i].value();
+            handler = NULL;
+            break;
+          }
+        }
+      }
+    } else {
+      std::set<std::pair<base::FilePath, std::string> > path_and_file_type_set;
+      for (size_t i = 0; i < file_paths_.size(); ++i) {
+        path_and_file_type_set.insert(
+            std::make_pair(file_paths_[i], mime_types_[i]));
+      }
+      const std::vector<const extensions::FileHandlerInfo*>& handlers =
+          extensions::app_file_handler_util::FindFileHandlersForFiles(
+              *extension_, path_and_file_type_set);
+      if (!handlers.empty())
+        handler = handlers[0];
     }
 
     // If this app doesn't have a file handler that supports the file, launch
     // with no launch data.
     if (!handler) {
-      LOG(WARNING) << "Extension does not provide a valid file handler for "
-                   << file_path_.value();
+      LOG(WARNING) << "Extension does not provide a valid file handler.";
       LaunchWithNoLaunchData();
       return;
     }
 
     if (handler_id_.empty())
       handler_id_ = handler->id;
 
     // Access needs to be granted to the file for the process associated with
     // the extension. To do this the ExtensionHost is needed. This might not be
     // available, or it might be in the process of being unloaded, in which case
     // the lazy background task queue is used to load the extension and then
     // call back to us.
-    extensions::LazyBackgroundTaskQueue* queue =
+    extensions::LazyBackgroundTaskQueue* const queue =
         ExtensionSystem::Get(profile_)->lazy_background_task_queue();
     if (queue->ShouldEnqueueTask(profile_, extension_)) {
-      queue->AddPendingTask(profile_, extension_->id(), base::Bind(
-              &PlatformAppPathLauncher::GrantAccessToFileAndLaunch,
-              this, mime_type));
+      queue->AddPendingTask(
+          profile_,
+          extension_->id(),
+          base::Bind(&PlatformAppPathLauncher::GrantAccessToFilesAndLaunch,
+                     this));
       return;
     }
 
-    extensions::ProcessManager* process_manager =
+    extensions::ProcessManager* const process_manager =
         ExtensionSystem::Get(profile_)->process_manager();
-    ExtensionHost* host =
+    ExtensionHost* const host =
         process_manager->GetBackgroundHostForExtension(extension_->id());
     DCHECK(host);
-    GrantAccessToFileAndLaunch(mime_type, host);
+    GrantAccessToFilesAndLaunch(host);
   }
 
-  void GrantAccessToFileAndLaunch(const std::string& mime_type,
-                                  ExtensionHost* host) {
+  void GrantAccessToFilesAndLaunch(ExtensionHost* host) {
     // If there was an error loading the app page, |host| will be NULL.
     if (!host) {
       LOG(ERROR) << "Could not load app page for " << extension_->id();
       return;
     }
 
-    GrantedFileEntry file_entry =
-        CreateFileEntry(profile_,
-                        extension_,
-                        host->render_process_host()->GetID(),
-                        file_path_,
-                        false);
-    AppEventRouter::DispatchOnLaunchedEventWithFileEntry(
-        profile_, extension_, handler_id_, mime_type, file_entry);
+    std::vector<GrantedFileEntry> file_entries;
+    for (size_t i = 0; i < file_paths_.size(); ++i) {
+      file_entries.push_back(
+          CreateFileEntry(profile_,
+                          extension_,
+                          host->render_process_host()->GetID(),
+                          file_paths_[i],
+                          false));
+    }
+
+    AppEventRouter::DispatchOnLaunchedEventWithFileEntries(
+        profile_, extension_, handler_id_, mime_types_, file_entries);
   }
 
   // The profile the app should be run in.
   Profile* profile_;
   // The extension providing the app.
   // TODO(benwells): Hold onto the extension ID instead of a pointer as it
   // is possible the extension will be unloaded while we're doing our thing.
   // See http://crbug.com/372270 for details.
   const Extension* extension_;
   // The path to be passed through to the app.
-  base::FilePath file_path_;
+  std::vector<base::FilePath> file_paths_;
+  std::vector<std::string> mime_types_;
   // The ID of the file handler used to launch the app.
   std::string handler_id_;
 
   DISALLOW_COPY_AND_ASSIGN(PlatformAppPathLauncher);
 };
 
 }  // namespace
 
 void LaunchPlatformAppWithCommandLine(Profile* profile,
                                       const Extension* extension,
@@ skipping 47 matching lines @@
   launcher->Launch();
 }
 
 void LaunchPlatformApp(Profile* profile, const Extension* extension) {
   LaunchPlatformAppWithCommandLine(profile,
                                    extension,
                                    CommandLine(CommandLine::NO_PROGRAM),
                                    base::FilePath());
 }
 
-void LaunchPlatformAppWithFileHandler(Profile* profile,
-                                      const Extension* extension,
-                                      const std::string& handler_id,
-                                      const base::FilePath& file_path) {
+void LaunchPlatformAppWithFileHandler(
+    Profile* profile,
+    const Extension* extension,
+    const std::string& handler_id,
+    const std::vector<base::FilePath>& file_paths) {
   scoped_refptr<PlatformAppPathLauncher> launcher =
-      new PlatformAppPathLauncher(profile, extension, file_path);
+      new PlatformAppPathLauncher(profile, extension, file_paths);
   launcher->LaunchWithHandler(handler_id);
 }
 
 void RestartPlatformApp(Profile* profile, const Extension* extension) {
   EventRouter* event_router = EventRouter::Get(profile);
   bool listening_to_restart = event_router->
       ExtensionHasEventListener(extension->id(),
                                 app_runtime::OnRestarted::kEventName);
 
   if (listening_to_restart) {
@@ skipping 16 matching lines @@
 void LaunchPlatformAppWithUrl(Profile* profile,
                               const Extension* extension,
                               const std::string& handler_id,
                               const GURL& url,
                               const GURL& referrer_url) {
   AppEventRouter::DispatchOnLaunchedEventWithUrl(
       profile, extension, handler_id, url, referrer_url);
 }
 
 }  // namespace apps