Commit 37af16a9 authored by Tetsui Ohkubo's avatar Tetsui Ohkubo Committed by Commit Bot

Implement image preview in Screenshot taken notification.

Previously, user has no way to know what does the taken screenshot looks
like before opening the image. With this CL, user can know that because
the preview image is shown in the notification.
This improvement is included in MD notification mock (internal:
go/fuwow), but should also be useful in the existing style notification.

BUG=746194
TEST=manual

Change-Id: I566b15209b5a7c18b8c706117896c21dde728088
Reviewed-on: https://chromium-review.googlesource.com/580178
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: default avatarDan Erat <derat@chromium.org>
Reviewed-by: default avatarNaoki Fukino <fukino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491907}
parent 6d51c918
...@@ -1460,6 +1460,7 @@ split_static_library("ui") { ...@@ -1460,6 +1460,7 @@ split_static_library("ui") {
"//ash/strings", "//ash/strings",
"//components/session_manager/core", "//components/session_manager/core",
"//components/user_manager", "//components/user_manager",
"//services/data_decoder/public/cpp",
"//services/ui/public/cpp", "//services/ui/public/cpp",
"//services/ui/public/interfaces", "//services/ui/public/interfaces",
"//ui/app_list/presenter", "//ui/app_list/presenter",
......
...@@ -38,6 +38,9 @@ ...@@ -38,6 +38,9 @@
#include "components/drive/chromeos/file_system_interface.h" #include "components/drive/chromeos/file_system_interface.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/common/service_manager_connection.h"
#include "services/data_decoder/public/cpp/decode_image.h"
#include "services/service_manager/public/cpp/connector.h"
#include "ui/base/clipboard/clipboard.h" #include "ui/base/clipboard/clipboard.h"
#include "ui/base/clipboard/scoped_clipboard_writer.h" #include "ui/base/clipboard/scoped_clipboard_writer.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
...@@ -282,6 +285,14 @@ std::string GetScreenshotBaseFilename() { ...@@ -282,6 +285,14 @@ std::string GetScreenshotBaseFilename() {
return file_name; return file_name;
} }
// Read a file to a string and return.
std::string ReadFileToString(const base::FilePath& path) {
std::string data;
// It may fail, but it doesn't matter for our purpose.
base::ReadFileToString(path, &data);
return data;
}
} // namespace } // namespace
ChromeScreenshotGrabber::ChromeScreenshotGrabber() ChromeScreenshotGrabber::ChromeScreenshotGrabber()
...@@ -290,7 +301,8 @@ ChromeScreenshotGrabber::ChromeScreenshotGrabber() ...@@ -290,7 +301,8 @@ ChromeScreenshotGrabber::ChromeScreenshotGrabber()
base::CreateTaskRunnerWithTraits( base::CreateTaskRunnerWithTraits(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}))), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}))),
profile_for_test_(NULL) { profile_for_test_(NULL),
weak_factory_(this) {
screenshot_grabber_->AddObserver(this); screenshot_grabber_->AddObserver(this);
} }
...@@ -416,16 +428,138 @@ void ChromeScreenshotGrabber::OnScreenshotCompleted( ...@@ -416,16 +428,138 @@ void ChromeScreenshotGrabber::OnScreenshotCompleted(
if (notifier_state_tracker->IsNotifierEnabled(message_center::NotifierId( if (notifier_state_tracker->IsNotifierEnabled(message_center::NotifierId(
message_center::NotifierId::SYSTEM_COMPONENT, message_center::NotifierId::SYSTEM_COMPONENT,
ash::system_notifier::kNotifierScreenshot))) { ash::system_notifier::kNotifierScreenshot))) {
std::unique_ptr<Notification> notification( if (result != ui::ScreenshotGrabberObserver::SCREENSHOT_SUCCESS) {
CreateNotification(result, screenshot_path)); content::BrowserThread::PostTask(
g_browser_process->notification_ui_manager()->Add(*notification, content::BrowserThread::UI, FROM_HERE,
GetProfile()); base::BindOnce(
&ChromeScreenshotGrabber::OnReadScreenshotFileForPreviewCompleted,
weak_factory_.GetWeakPtr(), result, screenshot_path,
gfx::Image()));
return;
}
if (drive::util::IsUnderDriveMountPoint(screenshot_path)) {
drive::FileSystemInterface* file_system =
drive::util::GetFileSystemByProfile(GetProfile());
if (!file_system) {
LOG(ERROR) << "Failed to get file system of current profile";
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE,
base::BindOnce(&ChromeScreenshotGrabber::
OnReadScreenshotFileForPreviewCompleted,
weak_factory_.GetWeakPtr(), result, screenshot_path,
gfx::Image()));
return;
}
file_system->GetFile(
drive::util::ExtractDrivePath(screenshot_path),
base::BindRepeating(
&ChromeScreenshotGrabber::ReadScreenshotFileForPreviewDrive,
weak_factory_.GetWeakPtr(), screenshot_path));
} else {
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE,
base::BindOnce(
&ChromeScreenshotGrabber::ReadScreenshotFileForPreviewLocal,
weak_factory_.GetWeakPtr(), screenshot_path, screenshot_path));
}
} }
} }
void ChromeScreenshotGrabber::ReadScreenshotFileForPreviewLocal(
const base::FilePath& screenshot_path,
const base::FilePath& screenshot_cache_path) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
base::PostTaskWithTraitsAndReplyWithResult(
FROM_HERE, kBlockingTaskTraits,
base::BindOnce(&ReadFileToString, screenshot_cache_path),
base::BindOnce(&ChromeScreenshotGrabber::DecodeScreenshotFileForPreview,
weak_factory_.GetWeakPtr(), screenshot_path));
}
void ChromeScreenshotGrabber::ReadScreenshotFileForPreviewDrive(
const base::FilePath& screenshot_path,
drive::FileError error,
const base::FilePath& screenshot_cache_path,
std::unique_ptr<drive::ResourceEntry> entry) {
if (error != drive::FILE_ERROR_OK) {
LOG(ERROR) << "Failed to read the screenshot path on drive: "
<< drive::FileErrorToString(error);
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE,
base::BindOnce(
&ChromeScreenshotGrabber::OnReadScreenshotFileForPreviewCompleted,
weak_factory_.GetWeakPtr(),
ui::ScreenshotGrabberObserver::SCREENSHOT_SUCCESS, screenshot_path,
gfx::Image()));
return;
}
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE,
base::BindOnce(
&ChromeScreenshotGrabber::ReadScreenshotFileForPreviewLocal,
weak_factory_.GetWeakPtr(), screenshot_path, screenshot_cache_path));
}
void ChromeScreenshotGrabber::DecodeScreenshotFileForPreview(
const base::FilePath& screenshot_path,
std::string image_data) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (image_data.empty()) {
LOG(ERROR) << "Failed to read the screenshot file: "
<< screenshot_path.value();
OnReadScreenshotFileForPreviewCompleted(
ui::ScreenshotGrabberObserver::SCREENSHOT_SUCCESS, screenshot_path,
gfx::Image());
return;
}
service_manager::mojom::ConnectorRequest connector_request;
std::unique_ptr<service_manager::Connector> connector =
service_manager::Connector::Create(&connector_request);
content::ServiceManagerConnection::GetForProcess()
->GetConnector()
->BindConnectorRequest(std::move(connector_request));
// Decode the image in sandboxed process becuase decode image_data comes from
// external storage.
data_decoder::DecodeImage(
connector.get(),
std::vector<uint8_t>(image_data.begin(), image_data.end()),
data_decoder::mojom::ImageCodec::DEFAULT, false,
data_decoder::kDefaultMaxSizeInBytes, gfx::Size(),
base::BindOnce(
&ChromeScreenshotGrabber::OnScreenshotFileForPreviewDecoded,
weak_factory_.GetWeakPtr(), screenshot_path));
}
void ChromeScreenshotGrabber::OnScreenshotFileForPreviewDecoded(
const base::FilePath& screenshot_path,
const SkBitmap& decoded_image) {
OnReadScreenshotFileForPreviewCompleted(
ui::ScreenshotGrabberObserver::SCREENSHOT_SUCCESS, screenshot_path,
gfx::Image::CreateFrom1xBitmap(decoded_image));
}
void ChromeScreenshotGrabber::OnReadScreenshotFileForPreviewCompleted(
ui::ScreenshotGrabberObserver::Result result,
const base::FilePath& screenshot_path,
gfx::Image image) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
std::unique_ptr<Notification> notification(
CreateNotification(result, screenshot_path, image));
g_browser_process->notification_ui_manager()->Add(*notification,
GetProfile());
}
Notification* ChromeScreenshotGrabber::CreateNotification( Notification* ChromeScreenshotGrabber::CreateNotification(
ui::ScreenshotGrabberObserver::Result screenshot_result, ui::ScreenshotGrabberObserver::Result screenshot_result,
const base::FilePath& screenshot_path) { const base::FilePath& screenshot_path,
gfx::Image image) {
const std::string notification_id(kNotificationId); const std::string notification_id(kNotificationId);
// We cancel a previous screenshot notification, if any, to ensure we get // We cancel a previous screenshot notification, if any, to ensure we get
// a fresh notification pop-up. // a fresh notification pop-up.
...@@ -453,10 +587,14 @@ Notification* ChromeScreenshotGrabber::CreateNotification( ...@@ -453,10 +587,14 @@ Notification* ChromeScreenshotGrabber::CreateNotification(
IDR_NOTIFICATION_SCREENSHOT_ANNOTATE); IDR_NOTIFICATION_SCREENSHOT_ANNOTATE);
optional_field.buttons.push_back(annotate_button); optional_field.buttons.push_back(annotate_button);
} }
// Assign image for notification preview. It might be empty.
optional_field.image = image;
} }
return new Notification( return new Notification(
message_center::NOTIFICATION_TYPE_SIMPLE, image.IsEmpty() ? message_center::NOTIFICATION_TYPE_SIMPLE
: message_center::NOTIFICATION_TYPE_IMAGE,
l10n_util::GetStringUTF16( l10n_util::GetStringUTF16(
GetScreenshotNotificationTitle(screenshot_result)), GetScreenshotNotificationTitle(screenshot_result)),
l10n_util::GetStringUTF16( l10n_util::GetStringUTF16(
......
...@@ -9,6 +9,9 @@ ...@@ -9,6 +9,9 @@
#include "base/macros.h" #include "base/macros.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/notifications/notification.h" #include "chrome/browser/notifications/notification.h"
#include "components/drive/chromeos/file_system_interface.h"
#include "components/drive/drive.pb.h"
#include "ui/gfx/image/image.h"
#include "ui/snapshot/screenshot_grabber.h" #include "ui/snapshot/screenshot_grabber.h"
class Profile; class Profile;
...@@ -48,9 +51,66 @@ class ChromeScreenshotGrabber : public ash::ScreenshotDelegate, ...@@ -48,9 +51,66 @@ class ChromeScreenshotGrabber : public ash::ScreenshotDelegate,
private: private:
friend class ash::ChromeScreenshotGrabberTest; friend class ash::ChromeScreenshotGrabberTest;
// Callback method of FileSystemInterface::GetFile().
// Runs ReadScreenshotFileForPreviewLocal if successful. Otherwise, runs
// OnReadScreenshotFileForPreviewCompleted to show notification without
// screenshot preview.
// One way to try screenshot saved to Drive is to change the download
// directory from Settings > Downloads > Location.
//
// |screenshot_path| is screenshot path on Drive.
// Parameters after |screenshot_path| is set by GetFile().
// |screenshot_cache_path| is a local cache of remote |screenshot_path|.
void ReadScreenshotFileForPreviewDrive(
const base::FilePath& screenshot_path,
drive::FileError error,
const base::FilePath& screenshot_cache_path,
std::unique_ptr<drive::ResourceEntry> entry);
// Reads a screenshot file in |screenshot_cache_path|, and runs
// DecodeScreenshotFileForPreview.
//
// |screenshot_path| and |screenshot_cache_path| are different when the
// screenshot is saved to Drive. Otherwise, they should be same.
// |screenshot_path| can be a Drive path while |screenshot_cache_path| is
// always a local path.
void ReadScreenshotFileForPreviewLocal(
const base::FilePath& screenshot_path,
const base::FilePath& screenshot_cache_path);
// Decodes |image_data| and run OnScreenshotFileForPreviewDecoded.
// Although |image_data| is originally generated by the screenshot grabber,
// it is saved to local or remote storage, so we have to decode it in
// a sandboxed process.
//
// |screenshot_path| is a path that is opened by file manager when the
// notification is clicked.
void DecodeScreenshotFileForPreview(const base::FilePath& screenshot_path,
std::string image_data);
// Callback method of DecodeScreenshotFileForPreview.
// Converts SkBitmap to gfx::Image and calls
// OnReadScreenshotFileForPreviewCompleted.
//
// |screenshot_path| is a path that is opened by file manager when the
// notification is clicked.
void OnScreenshotFileForPreviewDecoded(const base::FilePath& screenshot_path,
const SkBitmap& decoded_image);
// Shows "Screenshot taken" notification.
//
// |screenshot_path| is a path that is opened by file manager when the
// notification is clicked.
// |image| is a preview image attached to the notification. It can be empty.
void OnReadScreenshotFileForPreviewCompleted(
ui::ScreenshotGrabberObserver::Result result,
const base::FilePath& screenshot_path,
gfx::Image image);
Notification* CreateNotification( Notification* CreateNotification(
ui::ScreenshotGrabberObserver::Result screenshot_result, ui::ScreenshotGrabberObserver::Result screenshot_result,
const base::FilePath& screenshot_path); const base::FilePath& screenshot_path,
gfx::Image image);
void SetProfileForTest(Profile* profile); void SetProfileForTest(Profile* profile);
Profile* GetProfile(); Profile* GetProfile();
...@@ -58,6 +118,8 @@ class ChromeScreenshotGrabber : public ash::ScreenshotDelegate, ...@@ -58,6 +118,8 @@ class ChromeScreenshotGrabber : public ash::ScreenshotDelegate,
std::unique_ptr<ui::ScreenshotGrabber> screenshot_grabber_; std::unique_ptr<ui::ScreenshotGrabber> screenshot_grabber_;
Profile* profile_for_test_; Profile* profile_for_test_;
base::WeakPtrFactory<ChromeScreenshotGrabber> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(ChromeScreenshotGrabber); DISALLOW_COPY_AND_ASSIGN(ChromeScreenshotGrabber);
}; };
......
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