Refactor download image-MIME-type-detection code.

Refactor duplicated image-MIME-type-detection code from
DownloadCommands and DownloadItemNotification into a new
DownloadItemModel::HasSupportedImageMimeType() method.

BUG=605805

Review-Url: https://codereview.chromium.org/2219953004
Cr-Commit-Position: refs/heads/master@{#410382}

ash/common/system/chromeos/palette/palette_utils.cc

@@ -14,4 +14,4 @@ bool IsPaletteEnabled() {
       switches::kAshEnablePalette);
 }
 
-}  // namespace chromeos
+}  // namespace ash

chrome/browser/chromeos/note_taking_app_utils.cc

@@ -0,0 +1,83 @@
+// Copyright 2016 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 "chrome/browser/chromeos/note_taking_app_utils.h"
+
+#include <string>
+#include <vector>
+
+#include "apps/launcher.h"
+#include "ash/common/system/chromeos/palette/palette_utils.h"
+#include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/strings/string_split.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chromeos/chromeos_switches.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/common/extension.h"
+#include "url/gurl.h"
+
+namespace chromeos {
+namespace {
+
+// TODO(derat): Add more IDs.
+const char* const kExtensionIds[] = {
+    "hmjkmjkepdijhoojdojkdfohbdgmmhki",  // Google Keep app (Web Store)
+};
+
+// Returns the first installed and enabled whitelisted note-taking app, or null
+// if none is installed.
+// TODO(derat): Instead of a using hardcoded list of IDs, use an app designated
+// by a pref.
+const extensions::Extension* GetApp(Profile* profile) {
+  std::vector<std::string> ids;
+
+  const std::string switch_value =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kNoteTakingAppIds);
+  if (!switch_value.empty()) {
+    ids = base::SplitString(switch_value, ",", base::TRIM_WHITESPACE,
+                            base::SPLIT_WANT_NONEMPTY);
+  } else {
+    ids.assign(kExtensionIds, kExtensionIds + arraysize(kExtensionIds));
+  }
+
+  const extensions::ExtensionRegistry* extension_registry =
+      extensions::ExtensionRegistry::Get(profile);
+  const extensions::ExtensionSet& enabled_extensions =
+      extension_registry->enabled_extensions();
+  for (const auto& id : ids) {
+    if (enabled_extensions.Contains(id)) {
+      return extension_registry->GetExtensionById(
+          id, extensions::ExtensionRegistry::ENABLED);
+    }
+  }
+  return nullptr;
+}
+
+}  // namespace
+
+bool IsNoteTakingAppAvailable(Profile* profile) {
+  DCHECK(profile);
+  return ash::IsPaletteEnabled() && GetApp(profile);
+}
+
+void LaunchNoteTakingAppForNewNote(Profile* profile,
+                                   const base::FilePath& path) {
+  DCHECK(profile);
+  const extensions::Extension* app = GetApp(profile);
+  if (!app) {
+    LOG(ERROR) << "Failed to find note-taking app";
+    return;
+  }
+
+  // TODO: Launch with an "action" launch source and the appropriate action type
+  // to create a new note once that's been added to chrome.appRuntime.
+  if (path.empty())
+    apps::LaunchPlatformApp(profile, app, extensions::SOURCE_UNTRACKED);
+  else
+    apps::LaunchPlatformAppWithPath(profile, app, path);
+}
+
+}  // namespace chromeos

chrome/browser/chromeos/note_taking_app_utils.h

@@ -0,0 +1,27 @@
+// Copyright 2016 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_NOTE_TAKING_APP_UTILS_H_
+#define CHROME_BROWSER_CHROMEOS_NOTE_TAKING_APP_UTILS_H_
+
+class Profile;
+
+namespace base {
+class FilePath;
+}  // namespace base
+
+namespace chromeos {
+
+// Returns true if an app that can be used to take notes is available.
+bool IsNoteTakingAppAvailable(Profile* profile);
+
+// Launches the note-taking app to create a new note, optionally additionally
+// passing a file (|path| may be empty). IsNoteTakingAppAvailable() must be
+// called first.
+void LaunchNoteTakingAppForNewNote(Profile* profile,
+                                   const base::FilePath& path);
+
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_NOTE_TAKING_APP_UTILS_H_

chrome/browser/download/download_commands.cc

@@ -27,7 +27,6 @@
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/google/core/browser/google_util.h"
-#include "components/mime_util/mime_util.h"
 #include "grit/theme_resources.h"
 #include "net/base/url_util.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
@@ -383,23 +382,11 @@ bool DownloadCommands::CanOpenPdfInSystemViewer() const {
 void DownloadCommands::CopyFileAsImageToClipboard() const {
   if (download_item_->GetState() != content::DownloadItem::COMPLETE ||
       download_item_->GetReceivedBytes() > kMaxImageClipboardSize) {
-      return;
+    return;
   }
 
-  // TODO(yoshiki): Refine the code by combining the common logic with the
-  // preview in DownloadItemNotification.
-  std::string mime = download_item_->GetMimeType();
-  if (!mime_util::IsSupportedImageMimeType(mime)) {
-    base::FilePath::StringType extension_with_dot =
-        download_item_->GetTargetFilePath().FinalExtension();
-    if (extension_with_dot.empty() ||
-        !net::GetWellKnownMimeTypeFromExtension(extension_with_dot.substr(1),
-                                                &mime) ||
-        !mime_util::IsSupportedImageMimeType(mime)) {
-      // It seems a non-image file.
-      return;
-    }
-  }
+  if (!DownloadItemModel(download_item_).HasSupportedImageMimeType())
+    return;
 
   base::FilePath file_path = download_item_->GetFullPath();
   ImageClipboardCopyManager::Start(file_path);

chrome/browser/download/download_item_model.cc

@@ -23,9 +23,11 @@
 #include "chrome/common/safe_browsing/download_file_types.pb.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/mime_util/mime_util.h"
 #include "content/public/browser/download_danger_type.h"
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_item.h"
+#include "net/base/mime_util.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/l10n/time_format.h"
 #include "ui/base/text/bytes_formatting.h"
@@ -513,6 +515,23 @@ bool DownloadItemModel::IsMalicious() const {
   return false;
 }
 
+bool DownloadItemModel::HasSupportedImageMimeType() const {
+  if (mime_util::IsSupportedImageMimeType(download_->GetMimeType())) {
+    return true;
+  }
+
+  std::string mime;
+  base::FilePath::StringType extension_with_dot =
+      download_->GetTargetFilePath().FinalExtension();
+  if (!extension_with_dot.empty() && net::GetWellKnownMimeTypeFromExtension(
+                                         extension_with_dot.substr(1), &mime) &&
+      mime_util::IsSupportedImageMimeType(mime)) {
+    return true;
+  }
+
+  return false;
+}
+
 bool DownloadItemModel::ShouldAllowDownloadFeedback() const {
 #if defined(FULL_SAFE_BROWSING)
   if (!IsDangerous())

chrome/browser/download/download_item_model.h

@@ -91,6 +91,10 @@ class DownloadItemModel {
   // Implies IsDangerous() and MightBeMalicious().
   bool IsMalicious() const;
 
+  // Does this download have a MIME type (either explicit or inferred from its
+  // extension) suggesting that it is a supported image type?
+  bool HasSupportedImageMimeType() const;
+
   // Is safe browsing download feedback feature available for this download?
   bool ShouldAllowDownloadFeedback() const;
 

chrome/browser/download/download_item_model_unittest.cc

@@ -30,6 +30,7 @@ using safe_browsing::DownloadFileType;
 using ::testing::Mock;
 using ::testing::NiceMock;
 using ::testing::Return;
+using ::testing::ReturnRef;
 using ::testing::ReturnRefOfCopy;
 using ::testing::SetArgPointee;
 using ::testing::_;
@@ -382,6 +383,33 @@ TEST_F(DownloadItemModelTest, DangerLevel) {
   EXPECT_EQ(DownloadFileType::ALLOW_ON_USER_GESTURE, model().GetDangerLevel());
 }
 
+TEST_F(DownloadItemModelTest, HasSupportedImageMimeType) {
+  SetupDownloadItemDefaults();
+
+  // When the item has a supported image MIME type, true should be returned.
+  ON_CALL(item(), GetMimeType()).WillByDefault(Return("image/png"));
+  EXPECT_TRUE(model().HasSupportedImageMimeType());
+
+  // An unsupported MIME type should result in false being returned...
+  ON_CALL(item(), GetMimeType()).WillByDefault(Return("image/unsupported"));
+  EXPECT_FALSE(model().HasSupportedImageMimeType());
+
+  // ... unless the target path has a well-known image extension.
+  const base::FilePath kImagePath(FILE_PATH_LITERAL("/foo/image.png"));
+  ON_CALL(item(), GetTargetFilePath()).WillByDefault(ReturnRef(kImagePath));
+  EXPECT_TRUE(model().HasSupportedImageMimeType());
+
+  // .txt and missing extensions should also result in false being returned.
+  const base::FilePath kTextPath(FILE_PATH_LITERAL("/foo/image.txt"));
+  ON_CALL(item(), GetTargetFilePath()).WillByDefault(ReturnRef(kTextPath));
+  EXPECT_FALSE(model().HasSupportedImageMimeType());
+
+  const base::FilePath kNoExtensionPath(FILE_PATH_LITERAL("/foo/image."));
+  ON_CALL(item(), GetTargetFilePath())
+      .WillByDefault(ReturnRef(kNoExtensionPath));
+  EXPECT_FALSE(model().HasSupportedImageMimeType());
+}
+
 TEST_F(DownloadItemModelTest, ShouldRemoveFromShelfWhenComplete) {
   const struct TestCase {
     DownloadItem::DownloadState state;

chrome/browser/download/notification/download_item_notification.cc

@@ -470,22 +470,7 @@ void DownloadItemNotification::UpdateNotificationData(
 
     image_decode_status_ = IN_PROGRESS;
 
-    bool maybe_image = false;
-    if (mime_util::IsSupportedImageMimeType(item_->GetMimeType())) {
-      maybe_image = true;
-    } else {
-      std::string mime;
-      base::FilePath::StringType extension_with_dot =
-          item_->GetTargetFilePath().FinalExtension();
-      if (!extension_with_dot.empty() &&
-          net::GetWellKnownMimeTypeFromExtension(extension_with_dot.substr(1),
-                                                 &mime) &&
-          mime_util::IsSupportedImageMimeType(mime)) {
-        maybe_image = true;
-      }
-    }
-
-    if (maybe_image) {
+    if (model.HasSupportedImageMimeType()) {
       base::FilePath file_path = item_->GetFullPath();
       base::PostTaskAndReplyWithResult(
           content::BrowserThread::GetBlockingPool(), FROM_HERE,

chrome/chrome_browser_chromeos.gypi

@@ -771,6 +771,8 @@
         'browser/chromeos/net/wake_on_wifi_connection_observer.h',
         'browser/chromeos/net/wake_on_wifi_manager.cc',
         'browser/chromeos/net/wake_on_wifi_manager.h',
+        'browser/chromeos/note_taking_app_utils.cc',
+        'browser/chromeos/note_taking_app_utils.h',
         'browser/chromeos/options/cert_library.cc',
         'browser/chromeos/options/cert_library.h',
         'browser/chromeos/options/network_config_view.cc',

chromeos/chromeos_switches.cc

@@ -363,6 +363,10 @@ const char kMemoryPressureThresholds[] = "memory-pressure-thresholds";
 // Enables natural scroll by default.
 const char kNaturalScrollDefault[] = "enable-natural-scroll-default";
 
+// An optional comma-separated list of IDs of apps that can be used to take
+// notes. If unset, a hardcoded list is used instead.
+const char kNoteTakingAppIds[] = "note-taking-app-ids";
+
 // Indicates that if we should start bootstrapping Master OOBE.
 const char kOobeBootstrappingMaster[] = "oobe-bootstrapping-master";
 

chromeos/chromeos_switches.h

@@ -115,6 +115,7 @@ CHROMEOS_EXPORT extern const char kLoginProfile[];
 CHROMEOS_EXPORT extern const char kLoginUser[];
 CHROMEOS_EXPORT extern const char kMemoryPressureThresholds[];
 CHROMEOS_EXPORT extern const char kNaturalScrollDefault[];
+CHROMEOS_EXPORT extern const char kNoteTakingAppIds[];
 CHROMEOS_EXPORT extern const char kOobeBootstrappingMaster[];
 CHROMEOS_EXPORT extern const char kOobeGuestSession[];
 CHROMEOS_EXPORT extern const char kOobeSkipPostLogin[];