Commit f849948b authored by derat's avatar derat Committed by Commit bot

Reland "Refactor download image-MIME-type-detection code."

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

(Relanding since original commit 759969db accidentally
pulled in another change.)

BUG=605805
TBR=rdsmith@chromium.org,svaldez@chromium.org

Review-Url: https://codereview.chromium.org/2224053002
Cr-Commit-Position: refs/heads/master@{#410428}
parent 3fcbfceb
...@@ -27,7 +27,6 @@ ...@@ -27,7 +27,6 @@
#include "chrome/common/url_constants.h" #include "chrome/common/url_constants.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
#include "components/google/core/browser/google_util.h" #include "components/google/core/browser/google_util.h"
#include "components/mime_util/mime_util.h"
#include "grit/theme_resources.h" #include "grit/theme_resources.h"
#include "net/base/url_util.h" #include "net/base/url_util.h"
#include "ui/base/clipboard/scoped_clipboard_writer.h" #include "ui/base/clipboard/scoped_clipboard_writer.h"
...@@ -383,23 +382,11 @@ bool DownloadCommands::CanOpenPdfInSystemViewer() const { ...@@ -383,23 +382,11 @@ bool DownloadCommands::CanOpenPdfInSystemViewer() const {
void DownloadCommands::CopyFileAsImageToClipboard() const { void DownloadCommands::CopyFileAsImageToClipboard() const {
if (download_item_->GetState() != content::DownloadItem::COMPLETE || if (download_item_->GetState() != content::DownloadItem::COMPLETE ||
download_item_->GetReceivedBytes() > kMaxImageClipboardSize) { download_item_->GetReceivedBytes() > kMaxImageClipboardSize) {
return; return;
} }
// TODO(yoshiki): Refine the code by combining the common logic with the if (!DownloadItemModel(download_item_).HasSupportedImageMimeType())
// preview in DownloadItemNotification. return;
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;
}
}
base::FilePath file_path = download_item_->GetFullPath(); base::FilePath file_path = download_item_->GetFullPath();
ImageClipboardCopyManager::Start(file_path); ImageClipboardCopyManager::Start(file_path);
......
...@@ -23,9 +23,11 @@ ...@@ -23,9 +23,11 @@
#include "chrome/common/safe_browsing/download_file_types.pb.h" #include "chrome/common/safe_browsing/download_file_types.pb.h"
#include "chrome/grit/chromium_strings.h" #include "chrome/grit/chromium_strings.h"
#include "chrome/grit/generated_resources.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_danger_type.h"
#include "content/public/browser/download_interrupt_reasons.h" #include "content/public/browser/download_interrupt_reasons.h"
#include "content/public/browser/download_item.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/l10n_util.h"
#include "ui/base/l10n/time_format.h" #include "ui/base/l10n/time_format.h"
#include "ui/base/text/bytes_formatting.h" #include "ui/base/text/bytes_formatting.h"
...@@ -513,6 +515,23 @@ bool DownloadItemModel::IsMalicious() const { ...@@ -513,6 +515,23 @@ bool DownloadItemModel::IsMalicious() const {
return false; 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 { bool DownloadItemModel::ShouldAllowDownloadFeedback() const {
#if defined(FULL_SAFE_BROWSING) #if defined(FULL_SAFE_BROWSING)
if (!IsDangerous()) if (!IsDangerous())
......
...@@ -91,6 +91,10 @@ class DownloadItemModel { ...@@ -91,6 +91,10 @@ class DownloadItemModel {
// Implies IsDangerous() and MightBeMalicious(). // Implies IsDangerous() and MightBeMalicious().
bool IsMalicious() const; 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? // Is safe browsing download feedback feature available for this download?
bool ShouldAllowDownloadFeedback() const; bool ShouldAllowDownloadFeedback() const;
......
...@@ -30,6 +30,7 @@ using safe_browsing::DownloadFileType; ...@@ -30,6 +30,7 @@ using safe_browsing::DownloadFileType;
using ::testing::Mock; using ::testing::Mock;
using ::testing::NiceMock; using ::testing::NiceMock;
using ::testing::Return; using ::testing::Return;
using ::testing::ReturnRef;
using ::testing::ReturnRefOfCopy; using ::testing::ReturnRefOfCopy;
using ::testing::SetArgPointee; using ::testing::SetArgPointee;
using ::testing::_; using ::testing::_;
...@@ -382,6 +383,33 @@ TEST_F(DownloadItemModelTest, DangerLevel) { ...@@ -382,6 +383,33 @@ TEST_F(DownloadItemModelTest, DangerLevel) {
EXPECT_EQ(DownloadFileType::ALLOW_ON_USER_GESTURE, model().GetDangerLevel()); 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) { TEST_F(DownloadItemModelTest, ShouldRemoveFromShelfWhenComplete) {
const struct TestCase { const struct TestCase {
DownloadItem::DownloadState state; DownloadItem::DownloadState state;
......
...@@ -470,22 +470,7 @@ void DownloadItemNotification::UpdateNotificationData( ...@@ -470,22 +470,7 @@ void DownloadItemNotification::UpdateNotificationData(
image_decode_status_ = IN_PROGRESS; image_decode_status_ = IN_PROGRESS;
bool maybe_image = false; if (model.HasSupportedImageMimeType()) {
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) {
base::FilePath file_path = item_->GetFullPath(); base::FilePath file_path = item_->GetFullPath();
base::PostTaskAndReplyWithResult( base::PostTaskAndReplyWithResult(
content::BrowserThread::GetBlockingPool(), FROM_HERE, content::BrowserThread::GetBlockingPool(), FROM_HERE,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment