Commit 823f4b92 authored by Xiyuan Xia's avatar Xiyuan Xia Committed by Commit Bot

cros: Fix documentScan.scan calling dbus on wrong thread.

- Make DocumentScanScanFunction a UIThreadExtensionFunction so that
  API -> DocumentScanInterface -> LorgnetteManagerClient runs on UI
  thread;
- Update DocumentScanInterface to use OnceCallback;

Bug: 966178
Change-Id: I0428299174dcd949ff06b2a17fb492aef0df7349
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1637874
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarKaran Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665685}
parent 478c75ac
......@@ -582,8 +582,8 @@ source_set("unit_tests") {
"api/declarative_webrequest/webrequest_condition_unittest.cc",
"api/document_scan/document_scan_api_unittest.cc",
"api/document_scan/document_scan_interface_chromeos_unittest.cc",
"api/document_scan/mock_document_scan_interface.cc",
"api/document_scan/mock_document_scan_interface.h",
"api/document_scan/fake_document_scan_interface.cc",
"api/document_scan/fake_document_scan_interface.h",
"api/file_handlers/app_file_handler_util_unittest.cc",
"api/file_handlers/directory_util_unittest.cc",
"api/file_handlers/mime_util_unittest.cc",
......
......@@ -6,11 +6,6 @@
#include "base/bind.h"
#include "base/stl_util.h"
#include "base/task/post_task.h"
#include "content/public/browser/browser_thread.h"
#include "extensions/browser/extension_system.h"
using content::BrowserThread;
namespace {
......@@ -21,7 +16,6 @@ const char kUserGestureRequiredError[] =
} // namespace
namespace extensions {
namespace api {
DocumentScanScanFunction::DocumentScanScanFunction()
......@@ -29,28 +23,16 @@ DocumentScanScanFunction::DocumentScanScanFunction()
DocumentScanScanFunction::~DocumentScanScanFunction() {}
bool DocumentScanScanFunction::Prepare() {
set_work_task_runner(base::CreateSequencedTaskRunnerWithTraits(
{base::MayBlock(), base::TaskPriority::BEST_EFFORT}));
ExtensionFunction::ResponseAction DocumentScanScanFunction::Run() {
params_ = document_scan::Scan::Params::Create(*args_);
EXTENSION_FUNCTION_VALIDATE(params_.get());
return true;
}
void DocumentScanScanFunction::AsyncWorkStart() {
if (!user_gesture()) {
error_ = kUserGestureRequiredError;
AsyncWorkCompleted();
return;
}
// Add a reference, which is balanced in OnScannerListReceived to keep the
// object around and allow the callback to be invoked.
AddRef();
if (!user_gesture())
return RespondNow(Error(kUserGestureRequiredError));
document_scan_interface_->ListScanners(
base::Bind(&DocumentScanScanFunction::OnScannerListReceived,
base::Unretained(this)));
base::BindOnce(&DocumentScanScanFunction::OnScannerListReceived, this));
return did_respond() ? AlreadyResponded() : RespondLater();
}
void DocumentScanScanFunction::OnScannerListReceived(
......@@ -73,20 +55,14 @@ void DocumentScanScanFunction::OnScannerListReceived(
}
if (scanner_i == scanner_descriptions.end()) {
error_ = kScannerNotAvailable;
AsyncWorkCompleted();
// Balance the AddRef in AsyncWorkStart().
Release();
Respond(Error(kScannerNotAvailable));
return;
}
// TODO(pstew): Call a delegate method here to select a scanner and options.
document_scan_interface_->Scan(
scanner_i->name, DocumentScanInterface::kScanModeColor, 0,
base::Bind(&DocumentScanScanFunction::OnResultsReceived,
base::Unretained(this)));
base::BindOnce(&DocumentScanScanFunction::OnResultsReceived, this));
}
void DocumentScanScanFunction::OnResultsReceived(
......@@ -98,25 +74,18 @@ void DocumentScanScanFunction::OnResultsReceived(
// is a multi-page scan, provide a means for adding additional scanned
// images up to the requested limit.
if (error.empty()) {
document_scan::ScanResults scan_results;
if (!scanned_image.empty()) {
scan_results.data_urls.push_back(scanned_image);
}
scan_results.mime_type = mime_type;
results_ = document_scan::Scan::Results::Create(scan_results);
if (!error.empty()) {
Respond(Error(error));
return;
}
error_ = error;
AsyncWorkCompleted();
// Balance the AddRef in AsyncWorkStart().
Release();
}
bool DocumentScanScanFunction::Respond() {
return error_.empty();
document_scan::ScanResults scan_results;
if (!scanned_image.empty()) {
scan_results.data_urls.push_back(scanned_image);
}
scan_results.mime_type = mime_type;
Respond(ArgumentList(document_scan::Scan::Results::Create(scan_results)));
}
} // namespace api
} // namespace extensions
......@@ -10,15 +10,14 @@
#include <vector>
#include "base/macros.h"
#include "extensions/browser/api/async_api_function.h"
#include "extensions/browser/api/document_scan/document_scan_interface.h"
#include "extensions/browser/extension_function.h"
#include "extensions/common/api/document_scan.h"
namespace extensions {
namespace api {
class DocumentScanScanFunction : public AsyncApiFunction {
class DocumentScanScanFunction : public UIThreadExtensionFunction {
public:
DECLARE_EXTENSION_FUNCTION("documentScan.scan", DOCUMENT_SCAN_SCAN)
DocumentScanScanFunction();
......@@ -26,10 +25,8 @@ class DocumentScanScanFunction : public AsyncApiFunction {
protected:
~DocumentScanScanFunction() override;
// AsyncApiFunction:
bool Prepare() override;
void AsyncWorkStart() override;
bool Respond() override;
// UIThreadExtensionFunction:
ResponseAction Run() override;
private:
friend class DocumentScanScanFunctionTest;
......@@ -49,7 +46,6 @@ class DocumentScanScanFunction : public AsyncApiFunction {
};
} // namespace api
} // namespace extensions
#endif // EXTENSIONS_BROWSER_API_DOCUMENT_SCAN_DOCUMENT_SCAN_API_H_
......@@ -5,26 +5,24 @@
#include "extensions/browser/api/document_scan/document_scan_api.h"
#include <string>
#include <tuple>
#include <vector>
#include "extensions/browser/api/document_scan/mock_document_scan_interface.h"
#include "base/memory/ref_counted.h"
#include "extensions/browser/api/document_scan/fake_document_scan_interface.h"
#include "extensions/browser/api_test_utils.h"
#include "extensions/browser/api_unittest.h"
#include "testing/gmock/include/gmock/gmock-matchers.h"
#include "testing/gtest/include/gtest/gtest.h"
using testing::_;
namespace extensions {
namespace api {
// Tests of networking_private_crypto support for Networking Private API.
class DocumentScanScanFunctionTest : public ApiUnitTest {
public:
DocumentScanScanFunctionTest()
: function_(new DocumentScanScanFunction()),
document_scan_interface_(new MockDocumentScanInterface()) {}
: function_(base::MakeRefCounted<DocumentScanScanFunction>()),
document_scan_interface_(new FakeDocumentScanInterface()) {}
~DocumentScanScanFunctionTest() override {}
void SetUp() override {
......@@ -37,22 +35,14 @@ class DocumentScanScanFunctionTest : public ApiUnitTest {
std::string RunFunctionAndReturnError(const std::string& args) {
function_->set_extension(extension());
std::string error = api_test_utils::RunFunctionAndReturnError(
function_, args, browser_context(), api_test_utils::NONE);
function_.get(), args, browser_context(), api_test_utils::NONE);
return error;
}
DocumentScanScanFunction* function_;
MockDocumentScanInterface* document_scan_interface_; // Owned by function_.
scoped_refptr<DocumentScanScanFunction> function_;
FakeDocumentScanInterface* document_scan_interface_; // Owned by function_.
};
ACTION_P2(InvokeListScannersCallback, scanner_list, error) {
std::get<0>(args).Run(scanner_list, error);
}
ACTION_P3(InvokeScanCallback, data, mime_type, error) {
std::get<3>(args).Run(data, mime_type, error);
}
TEST_F(DocumentScanScanFunctionTest, GestureRequired) {
EXPECT_EQ("User gesture required to perform scan",
RunFunctionAndReturnError("[{}]"));
......@@ -60,9 +50,7 @@ TEST_F(DocumentScanScanFunctionTest, GestureRequired) {
TEST_F(DocumentScanScanFunctionTest, NoScanners) {
function_->set_user_gesture(true);
EXPECT_CALL(*document_scan_interface_, ListScanners(_))
.WillOnce(InvokeListScannersCallback(
std::vector<DocumentScanInterface::ScannerDescription>(), ""));
document_scan_interface_->SetListScannersResult({}, "");
EXPECT_EQ("Scanner not available", RunFunctionAndReturnError("[{}]"));
}
......@@ -72,8 +60,7 @@ TEST_F(DocumentScanScanFunctionTest, NoMatchingScanners) {
DocumentScanInterface::ScannerDescription scanner;
scanner.image_mime_type = "img/fresco";
scanner_list.push_back(scanner);
EXPECT_CALL(*document_scan_interface_, ListScanners(_))
.WillOnce(InvokeListScannersCallback(scanner_list, ""));
document_scan_interface_->SetListScannersResult(scanner_list, "");
EXPECT_EQ(
"Scanner not available",
RunFunctionAndReturnError("[{\"mimeTypes\": [\"img/silverpoint\"]}]"));
......@@ -88,11 +75,9 @@ TEST_F(DocumentScanScanFunctionTest, ScanFailure) {
scanner.name = kScannerName;
scanner.image_mime_type = kMimeType;
scanner_list.push_back(scanner);
EXPECT_CALL(*document_scan_interface_, ListScanners(_))
.WillOnce(InvokeListScannersCallback(scanner_list, ""));
document_scan_interface_->SetListScannersResult(scanner_list, "");
const char kScanError[] = "Someone ate all the eggs";
EXPECT_CALL(*document_scan_interface_, Scan(kScannerName, _, _, _))
.WillOnce(InvokeScanCallback("", "", kScanError));
document_scan_interface_->SetScanResult("", "", kScanError);
EXPECT_EQ(kScanError,
RunFunctionAndReturnError("[{\"mimeTypes\": [\"img/tempera\"]}]"));
}
......@@ -100,15 +85,13 @@ TEST_F(DocumentScanScanFunctionTest, ScanFailure) {
TEST_F(DocumentScanScanFunctionTest, Success) {
std::vector<DocumentScanInterface::ScannerDescription> scanner_list;
scanner_list.push_back(DocumentScanInterface::ScannerDescription());
EXPECT_CALL(*document_scan_interface_, ListScanners(_))
.WillOnce(InvokeListScannersCallback(scanner_list, ""));
document_scan_interface_->SetListScannersResult(scanner_list, "");
const char kScanData[] = "A beautiful picture";
const char kMimeType[] = "img/encaustic";
EXPECT_CALL(*document_scan_interface_, Scan(_, _, _, _))
.WillOnce(InvokeScanCallback(kScanData, kMimeType, ""));
document_scan_interface_->SetScanResult(kScanData, kMimeType, "");
function_->set_user_gesture(true);
std::unique_ptr<base::DictionaryValue> result(
RunFunctionAndReturnDictionary(function_, "[{}]"));
RunFunctionAndReturnDictionary(function_.get(), "[{}]"));
ASSERT_NE(nullptr, result.get());
document_scan::ScanResults scan_results;
EXPECT_TRUE(document_scan::ScanResults::Populate(*result, &scan_results));
......@@ -117,5 +100,4 @@ TEST_F(DocumentScanScanFunctionTest, Success) {
}
} // namespace api
} // namespace extensions
......@@ -12,7 +12,6 @@
#include "base/callback.h"
namespace extensions {
namespace api {
class DocumentScanInterface {
......@@ -30,21 +29,22 @@ class DocumentScanInterface {
enum ScanMode { kScanModeColor, kScanModeGray, kScanModeLineart };
typedef base::Callback<void(
using ListScannersResultsCallback = base::OnceCallback<void(
const std::vector<ScannerDescription>& scanner_descriptions,
const std::string& error)> ListScannersResultsCallback;
const std::string& error)>;
typedef base::Callback<void(const std::string& scanned_image,
using ScanResultsCallback =
base::OnceCallback<void(const std::string& scanned_image,
const std::string& mime_type,
const std::string& error)> ScanResultsCallback;
const std::string& error)>;
virtual ~DocumentScanInterface();
virtual void Scan(const std::string& scanner_name,
ScanMode mode,
int resolution_dpi,
const ScanResultsCallback& callback) = 0;
virtual void ListScanners(const ListScannersResultsCallback& callback) = 0;
ScanResultsCallback callback) = 0;
virtual void ListScanners(ListScannersResultsCallback callback) = 0;
// Creates a platform-specific DocumentScanInterface instance.
static DocumentScanInterface* CreateInstance();
......@@ -54,7 +54,6 @@ class DocumentScanInterface {
};
} // namespace api
} // namespace extensions
#endif // EXTENSIONS_BROWSER_API_DOCUMENT_SCAN_DOCUMENT_SCAN_INTERFACE_H_
......@@ -4,6 +4,7 @@
#include "extensions/browser/api/document_scan/document_scan_interface_chromeos.h"
#include <utility>
#include <vector>
#include "base/base64.h"
......@@ -26,7 +27,6 @@ chromeos::LorgnetteManagerClient* GetLorgnetteManagerClient() {
} // namespace
namespace extensions {
namespace api {
DocumentScanInterfaceChromeos::DocumentScanInterfaceChromeos() = default;
......@@ -34,14 +34,14 @@ DocumentScanInterfaceChromeos::DocumentScanInterfaceChromeos() = default;
DocumentScanInterfaceChromeos::~DocumentScanInterfaceChromeos() = default;
void DocumentScanInterfaceChromeos::ListScanners(
const ListScannersResultsCallback& callback) {
ListScannersResultsCallback callback) {
GetLorgnetteManagerClient()->ListScanners(
base::BindOnce(&DocumentScanInterfaceChromeos::OnScannerListReceived,
base::Unretained(this), callback));
base::Unretained(this), std::move(callback)));
}
void DocumentScanInterfaceChromeos::OnScannerListReceived(
const ListScannersResultsCallback& callback,
ListScannersResultsCallback callback,
base::Optional<chromeos::LorgnetteManagerClient::ScannerTable> scanners) {
std::vector<ScannerDescription> scanner_descriptions;
if (scanners.has_value()) {
......@@ -66,13 +66,13 @@ void DocumentScanInterfaceChromeos::OnScannerListReceived(
}
}
const std::string kNoError;
callback.Run(scanner_descriptions, kNoError);
std::move(callback).Run(scanner_descriptions, kNoError);
}
void DocumentScanInterfaceChromeos::Scan(const std::string& scanner_name,
ScanMode mode,
int resolution_dpi,
const ScanResultsCallback& callback) {
ScanResultsCallback callback) {
VLOG(1) << "Choosing scanner " << scanner_name;
chromeos::LorgnetteManagerClient::ScanProperties properties;
switch (mode) {
......@@ -96,21 +96,22 @@ void DocumentScanInterfaceChromeos::Scan(const std::string& scanner_name,
GetLorgnetteManagerClient()->ScanImageToString(
scanner_name, properties,
base::BindOnce(&DocumentScanInterfaceChromeos::OnScanCompleted,
base::Unretained(this), callback));
base::Unretained(this), std::move(callback)));
}
void DocumentScanInterfaceChromeos::OnScanCompleted(
const ScanResultsCallback& callback,
ScanResultsCallback callback,
base::Optional<std::string> image_data) {
VLOG(1) << "ScanImage returns " << image_data.has_value();
if (!image_data.has_value()) {
callback.Run(std::string(), std::string(), kImageScanFailedError);
std::move(callback).Run(std::string(), std::string(),
kImageScanFailedError);
return;
}
std::string image_base64;
base::Base64Encode(image_data.value(), &image_base64);
callback.Run(kPngImageDataUrlPrefix + image_base64, kScannerImageMimeTypePng,
std::string() /* error */);
std::move(callback).Run(kPngImageDataUrlPrefix + image_base64,
kScannerImageMimeTypePng, std::string() /* error */);
}
// static
......@@ -119,5 +120,4 @@ DocumentScanInterface* DocumentScanInterface::CreateInstance() {
}
} // namespace api
} // namespace extensions
......@@ -13,7 +13,6 @@
#include "extensions/browser/api/document_scan/document_scan_interface.h"
namespace extensions {
namespace api {
class DocumentScanInterfaceChromeos : public DocumentScanInterface {
......@@ -21,24 +20,24 @@ class DocumentScanInterfaceChromeos : public DocumentScanInterface {
DocumentScanInterfaceChromeos();
~DocumentScanInterfaceChromeos() override;
void ListScanners(const ListScannersResultsCallback& callback) override;
// DocumentScanInterface:
void ListScanners(ListScannersResultsCallback callback) override;
void Scan(const std::string& scanner_name,
ScanMode mode,
int resolution_dpi,
const ScanResultsCallback& callback) override;
ScanResultsCallback callback) override;
private:
void OnScannerListReceived(
const ListScannersResultsCallback& callback,
ListScannersResultsCallback callback,
base::Optional<chromeos::LorgnetteManagerClient::ScannerTable> scanners);
void OnScanCompleted(const ScanResultsCallback& callback,
void OnScanCompleted(ScanResultsCallback callback,
base::Optional<std::string> image_data);
DISALLOW_COPY_AND_ASSIGN(DocumentScanInterfaceChromeos);
};
} // namespace api
} // namespace extensions
#endif // EXTENSIONS_BROWSER_API_DOCUMENT_SCAN_DOCUMENT_SCAN_INTERFACE_CHROMEOS_H_
......@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include <utility>
#include "base/macros.h"
#include "extensions/browser/api/document_scan/document_scan_interface.h"
......@@ -12,7 +14,6 @@ const char kScanFunctionNotImplementedError[] = "Scan function not implemented";
} // namespace
namespace extensions {
namespace api {
class DocumentScanInterfaceImpl : public DocumentScanInterface {
......@@ -20,14 +21,14 @@ class DocumentScanInterfaceImpl : public DocumentScanInterface {
DocumentScanInterfaceImpl() {}
~DocumentScanInterfaceImpl() override {}
void ListScanners(const ListScannersResultsCallback& callback) override {
callback.Run(std::vector<ScannerDescription>(), "");
void ListScanners(ListScannersResultsCallback callback) override {
std::move(callback).Run(std::vector<ScannerDescription>(), "");
}
void Scan(const std::string& scanner_name,
ScanMode mode,
int resolution_dpi,
const ScanResultsCallback& callback) override {
callback.Run("", "", kScanFunctionNotImplementedError);
ScanResultsCallback callback) override {
std::move(callback).Run("", "", kScanFunctionNotImplementedError);
}
private:
......@@ -40,5 +41,4 @@ DocumentScanInterface* DocumentScanInterface::CreateInstance() {
}
} // namespace api
} // namespace extensions
// Copyright 2019 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 "extensions/browser/api/document_scan/fake_document_scan_interface.h"
#include <utility>
namespace extensions {
namespace api {
FakeDocumentScanInterface::FakeDocumentScanInterface() = default;
FakeDocumentScanInterface::~FakeDocumentScanInterface() = default;
void FakeDocumentScanInterface::SetListScannersResult(
const std::vector<ScannerDescription>& scanner_descriptions,
const std::string& error) {
scanner_descriptions_ = scanner_descriptions;
error_ = error;
}
void FakeDocumentScanInterface::SetScanResult(const std::string& scanned_image,
const std::string& mime_type,
const std::string& error) {
scanned_image_ = scanned_image;
mime_type_ = mime_type;
error_ = error;
}
void FakeDocumentScanInterface::ListScanners(
ListScannersResultsCallback callback) {
std::move(callback).Run(scanner_descriptions_, error_);
}
void FakeDocumentScanInterface::Scan(const std::string& scanner_name,
ScanMode mode,
int resolution_dpi,
ScanResultsCallback callback) {
std::move(callback).Run(scanned_image_, mime_type_, error_);
}
} // namespace api
} // namespace extensions
// Copyright 2019 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 EXTENSIONS_BROWSER_API_DOCUMENT_SCAN_FAKE_DOCUMENT_SCAN_INTERFACE_H_
#define EXTENSIONS_BROWSER_API_DOCUMENT_SCAN_FAKE_DOCUMENT_SCAN_INTERFACE_H_
#include <string>
#include "extensions/browser/api/document_scan/document_scan_interface.h"
namespace extensions {
namespace api {
class FakeDocumentScanInterface : public DocumentScanInterface {
public:
FakeDocumentScanInterface();
~FakeDocumentScanInterface() override;
void SetListScannersResult(
const std::vector<ScannerDescription>& scanner_descriptions,
const std::string& error);
void SetScanResult(const std::string& scanned_image,
const std::string& mime_type,
const std::string& error);
// DocumentScanInterface:
void ListScanners(ListScannersResultsCallback callback) override;
void Scan(const std::string& scanner_name,
ScanMode mode,
int resolution_dpi,
ScanResultsCallback callback) override;
private:
std::vector<ScannerDescription> scanner_descriptions_;
std::string scanned_image_;
std::string mime_type_;
std::string error_;
};
} // namespace api
} // namespace extensions
#endif // EXTENSIONS_BROWSER_API_DOCUMENT_SCAN_FAKE_DOCUMENT_SCAN_INTERFACE_H_
// Copyright 2014 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 "extensions/browser/api/document_scan/mock_document_scan_interface.h"
namespace extensions {
namespace api {
MockDocumentScanInterface::MockDocumentScanInterface() {
}
MockDocumentScanInterface::~MockDocumentScanInterface() {
}
} // namespace api
} // namespace extensions
// Copyright 2014 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 EXTENSIONS_BROWSER_API_DOCUMENT_SCAN_MOCK_DOCUMENT_SCAN_INTERFACE_H_
#define EXTENSIONS_BROWSER_API_DOCUMENT_SCAN_MOCK_DOCUMENT_SCAN_INTERFACE_H_
#include <string>
#include <gmock/gmock.h>
#include "extensions/browser/api/document_scan/document_scan_interface.h"
namespace extensions {
namespace api {
class MockDocumentScanInterface : public DocumentScanInterface {
public:
MockDocumentScanInterface();
~MockDocumentScanInterface() override;
MOCK_METHOD4(Scan,
void(const std::string& scanner_name,
ScanMode mode,
int resolution_dpi,
const ScanResultsCallback& callback));
MOCK_METHOD1(ListScanners, void(const ListScannersResultsCallback& callback));
MOCK_CONST_METHOD0(GetImageMimeType, std::string());
};
} // namespace api
} // namespace extensions
#endif // EXTENSIONS_BROWSER_API_DOCUMENT_SCAN_MOCK_DOCUMENT_SCAN_INTERFACE_H_
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