Add verbs API to file handlers. Modify the Chrome OS UI so that it displayes the internationalized …

chrome/browser/chromeos/file_manager/file_tasks.cc

Index: chrome/browser/chromeos/file_manager/file_tasks.cc
diff --git a/chrome/browser/chromeos/file_manager/file_tasks.cc b/chrome/browser/chromeos/file_manager/file_tasks.cc
index fb7b55490e4bb34a7779e8cae9c18d42bccba2b3..911a70d7c05926a03389aa2295b66649def80e22 100644
--- a/chrome/browser/chromeos/file_manager/file_tasks.cc
+++ b/chrome/browser/chromeos/file_manager/file_tasks.cc
@@ -175,18 +175,20 @@ void PostProcessFoundTasks(
 
 }  // namespace
 
-FullTaskDescriptor::FullTaskDescriptor(
-    const TaskDescriptor& task_descriptor,
-    const std::string& task_title,
-    const GURL& icon_url,
-    bool is_default,
-    bool is_generic_file_handler)
+FullTaskDescriptor::FullTaskDescriptor(const TaskDescriptor& task_descriptor,
+                                       const std::string& task_title,
+                                       const std::string& task_verb,
+                                       const GURL& icon_url,
+                                       bool is_default,
+                                       bool is_generic_file_handler)
     : task_descriptor_(task_descriptor),
       task_title_(task_title),
+      task_verb_(task_verb),

[Devlin, 2016/05/16 16:17:32]

Should we DCHECK that task_verb_ is a valid verb?

Or, looking at this more, maybe we should introduce an enum on the
fileManagerPrivate API (which would give us compile errors)?

[cmihail, 2016/05/18 02:35:31]

Done. Thanks, I didn't really know about enum's support from idl files, so good
to keep it in mind for the future :).

       icon_url_(icon_url),
       is_default_(is_default),
-      is_generic_file_handler_(is_generic_file_handler) {
-}
+      is_generic_file_handler_(is_generic_file_handler) {}
+
+FullTaskDescriptor::~FullTaskDescriptor() {}
 
 FullTaskDescriptor::FullTaskDescriptor(const FullTaskDescriptor& other) =
     default;
@@ -411,12 +413,11 @@ void FindDriveAppTasks(const drive::DriveAppRegistry& drive_app_registry,
     GURL icon_url = drive::util::FindPreferredIcon(
         app_info.app_icons,
         drive::util::kPreferredIconSize);
-    result_list->push_back(
-        FullTaskDescriptor(descriptor,
-                           app_info.app_name,
-                           icon_url,
-                           false /* is_default */,
-                           false /* is_generic_file_handler */));
+
+    result_list->push_back(FullTaskDescriptor(
+        descriptor, app_info.app_name,
+        extensions::file_handler_verbs::kOpenWith, icon_url,
+        false /* is_default */, false /* is_generic_file_handler */));
   }
 }
 
@@ -483,35 +484,46 @@ void FindFileHandlerTasks(Profile* profile,
       continue;
     }
 
-    // Show the first good matching handler of each app. If there doesn't exist
-    // such handler, show the first matching handler of the app.
-    const extensions::FileHandlerInfo* file_handler = file_handlers.front();
-    for (auto handler : file_handlers) {
-      if (IsGoodMatchFileHandler(*handler, entries)) {
-        file_handler = handler;
-        break;
+    // A map which has as key a handler verb, and as value a pair of the
+    // handler with which to open the given entries and a boolean marking
+    // if the handler is a good match.
+    std::map<std::string, std::pair<const extensions::FileHandlerInfo*, bool>>
+        handlers_for_entries;
+    // Show the first good matching handler of each verb supporting the given
+    // entries that corresponds to the app. If there doesn't exist such handler,
+    // show the first matching handler of the verb.
+    for (const auto handler : file_handlers) {

[Devlin, 2016/05/16 16:17:32]

nit: I personally think auto here reduces readability (partially because it
makes me have to think "Does this need to be const auto&?")

[cmihail, 2016/05/18 02:35:31]

Done.

+      bool good_match = IsGoodMatchFileHandler(*handler, entries);
+      auto it = handlers_for_entries.find(handler->verb);
+      if (it == handlers_for_entries.end() ||
+          (!it->second.second /* existing handler not a good match */ &&
+           good_match)) {
+        handlers_for_entries[handler->verb] =
+            std::make_pair(handler, good_match);
       }
     }
 
-    std::string task_id = file_tasks::MakeTaskID(
-        extension->id(), file_tasks::TASK_TYPE_FILE_HANDLER, file_handler->id);
-
-    GURL best_icon = extensions::ExtensionIconSource::GetIconURL(
-        extension,
-        drive::util::kPreferredIconSize,
-        ExtensionIconSet::MATCH_BIGGER,
-        false,  // grayscale
-        NULL);  // exists
-
-    // If file handler doesn't match as good match, regards it as generic file
-    // handler.
-    const bool is_generic_file_handler =
-        !IsGoodMatchFileHandler(*file_handler, entries);
-    result_list->push_back(FullTaskDescriptor(
-        TaskDescriptor(extension->id(), file_tasks::TASK_TYPE_FILE_HANDLER,
-                       file_handler->id),
-        extension->name(), best_icon, false /* is_default */,
-        is_generic_file_handler));
+    for (const auto entry : handlers_for_entries) {

[Devlin, 2016/05/16 16:17:32]

this results in a copy - please make it const auto&

[cmihail, 2016/05/18 02:35:31]

Done.

+      const extensions::FileHandlerInfo* handler = entry.second.first;
+      std::string task_id = file_tasks::MakeTaskID(
+          extension->id(), file_tasks::TASK_TYPE_FILE_HANDLER, handler->id);
+
+      GURL best_icon = extensions::ExtensionIconSource::GetIconURL(
+          extension, drive::util::kPreferredIconSize,
+          ExtensionIconSet::MATCH_BIGGER,
+          false,  // grayscale
+          NULL);  // exists
+
+      // If file handler doesn't match as good match, regards it as generic file
+      // handler.
+      const bool is_generic_file_handler =
+          !IsGoodMatchFileHandler(*handler, entries);
+      result_list->push_back(FullTaskDescriptor(
+          TaskDescriptor(extension->id(), file_tasks::TASK_TYPE_FILE_HANDLER,
+                         handler->id),
+          extension->name(), handler->verb, best_icon, false /* is_default */,
+          is_generic_file_handler));
+    }
   }
 }
 
@@ -548,13 +560,10 @@ void FindFileBrowserHandlerTasks(
         NULL);  // exists
 
     result_list->push_back(FullTaskDescriptor(
-        TaskDescriptor(extension_id,
-                       file_tasks::TASK_TYPE_FILE_BROWSER_HANDLER,
+        TaskDescriptor(extension_id, file_tasks::TASK_TYPE_FILE_BROWSER_HANDLER,
                        handler->id()),
-        handler->title(),
-        icon_url,
-        false /* is_default */,
-        false /* is_generic_file_handler */));
+        handler->title(), "" /* no verb for FileBrowserHandler */, icon_url,
+        false /* is_default */, false /* is_generic_file_handler */));
   }
 }