Run ReadImage callback when read not allowed

- Changed ClipboardNonBacked::ReadImage to run
ReadImageCallback with empty SKBitmap as a crash
happens when it returns without running the callback.

Bug: 1133717
Test: New unit test added.
Change-Id: I6a7264b8b0d0c56e78852ed1822883cdd9c4efc8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2440656
Commit-Queue: Aya Elsayed <ayaelattar@chromium.org>
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814149}

ui/base/clipboard/clipboard_non_backed.cc

@@ -242,11 +242,10 @@ class ClipboardInternal {
 
   bool IsReadAllowed(const ClipboardDataEndpoint* data_dst) const {
     ClipboardDlpController* dlp_controller = ClipboardDlpController::Get();
-    if (!dlp_controller)
-      return true;
     auto* data = GetData();
-    return dlp_controller->IsDataReadAllowed(data ? data->source() : nullptr,
-                                             data_dst);
+    if (!dlp_controller || !data)
+      return true;
+    return dlp_controller->IsDataReadAllowed(data->source(), data_dst);
   }
 
  private:
@@ -587,8 +586,10 @@ void ClipboardNonBacked::ReadImage(ClipboardBuffer buffer,
                                    ReadImageCallback callback) const {
   DCHECK(CalledOnValidThread());
 
-  if (!clipboard_internal_->IsReadAllowed(data_dst))
+  if (!clipboard_internal_->IsReadAllowed(data_dst)) {
+    std::move(callback).Run(SkBitmap());
     return;
+  }
 
   RecordRead(ClipboardFormatMetric::kImage);
   std::move(callback).Run(clipboard_internal_->ReadImage());

ui/base/clipboard/clipboard_test_template.h

@@ -1106,7 +1106,7 @@ TYPED_TEST(ClipboardTest, DlpAllowDataRead) {
 
 // Test that pasting clipboard data would not work if the dlp controller
 // restricted it.
-TYPED_TEST(ClipboardTest, DlpDisallowDataRead) {
+TYPED_TEST(ClipboardTest, DlpDisallow_ReadText) {
   auto* dlp_controller = MockClipboardDlpController::Init();
   const base::string16 kTestText(base::UTF8ToUTF16("World"));
   {
@@ -1124,6 +1124,17 @@ TYPED_TEST(ClipboardTest, DlpDisallowDataRead) {
   EXPECT_EQ(base::string16(), read_result);
   MockClipboardDlpController::DeleteInstance();
 }
+
+TYPED_TEST(ClipboardTest, DlpDisallow_ReadImage) {
+  auto* dlp_controller = MockClipboardDlpController::Init();
+  EXPECT_CALL(*dlp_controller, IsDataReadAllowed)
+      .WillRepeatedly(testing::Return(false));
+  const SkBitmap& image = clipboard_test_util::ReadImage(&this->clipboard());
+  ::testing::Mock::VerifyAndClearExpectations(dlp_controller);
+  EXPECT_EQ(true, image.empty());
+  MockClipboardDlpController::DeleteInstance();
+}
+
 #endif  // defined(OS_CHROMEOS)
 
 }  // namespace ui

ui/base/clipboard/test/test_clipboard.cc

@@ -201,7 +201,12 @@ void TestClipboard::ReadRTF(ClipboardBuffer buffer,
 void TestClipboard::ReadImage(ClipboardBuffer buffer,
                               const ClipboardDataEndpoint* data_dst,
                               ReadImageCallback callback) const {
-  std::move(callback).Run(GetStore(buffer).image);
+  const DataStore& store = GetStore(buffer);
+  if (!IsDataReadAllowed(store.data_src.get(), data_dst)) {
+    std::move(callback).Run(SkBitmap());
+    return;
+  }
+  std::move(callback).Run(store.image);
 }
 
 // TODO(crbug.com/1103215): |data_dst| should be supported.