Commit 303ba3a3 authored by Jeroen Dhollander's avatar Jeroen Dhollander Committed by Commit Bot

Add BloomScreenshotDelegate

This prompts the user to select an area.
It will be implemented in a follow-up CL.

Bug: b/165356952
Tests: manually deployed
Change-Id: I33c1cc49c4cea542b87256cb00ac3370ef8a4427
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2376026
Auto-Submit: Jeroen Dhollander <jeroendh@chromium.org>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Jeroen Dhollander <jeroendh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#806674}
parent e458d3ca
......@@ -21,6 +21,7 @@
#include "chrome/browser/ui/ash/assistant/device_actions_delegate_impl.h"
#include "chromeos/components/bloom/public/cpp/bloom_controller.h"
#include "chromeos/components/bloom/public/cpp/bloom_controller_factory.h"
#include "chromeos/components/bloom/public/cpp/bloom_screenshot_delegate.h"
#include "chromeos/constants/chromeos_features.h"
#include "chromeos/constants/chromeos_switches.h"
#include "chromeos/services/assistant/public/cpp/features.h"
......@@ -92,7 +93,9 @@ void AssistantClientImpl::MaybeInit(Profile* profile) {
if (chromeos::assistant::features::IsBloomEnabled()) {
bloom_controller_ = chromeos::bloom::BloomControllerFactory::Create(
profile->GetURLLoaderFactory(),
IdentityManagerFactory::GetForProfile(profile));
IdentityManagerFactory::GetForProfile(profile),
nullptr // TODO(jeroendh): pass in real BloomScreenshotDelegate
);
}
}
......
......@@ -14,7 +14,6 @@ source_set("bloom") {
"bloom_server_proxy.h",
"bloom_server_proxy_impl.cc",
"bloom_server_proxy_impl.h",
"screenshot_grabber.h",
]
deps = [
......@@ -25,6 +24,7 @@ source_set("bloom") {
"//chromeos/services/assistant/public/shared",
"//components/signin/public/identity_manager",
"//services/network/public/cpp",
"//ui/gfx",
]
}
......@@ -44,5 +44,7 @@ source_set("unit_tests") {
"//services/network/public/cpp",
"//testing/gmock",
"//testing/gtest",
"//ui/gfx",
"//ui/gfx:test_support",
]
}
......@@ -2,4 +2,5 @@ include_rules = [
"+ash/public",
"+components/signin/public/identity_manager",
"+services/network/public",
"+ui/gfx",
]
......@@ -7,20 +7,20 @@
#include "base/logging.h"
#include "chromeos/components/bloom/bloom_interaction.h"
#include "chromeos/components/bloom/bloom_server_proxy.h"
#include "chromeos/components/bloom/screenshot_grabber.h"
#include "chromeos/components/bloom/public/cpp/bloom_screenshot_delegate.h"
namespace chromeos {
namespace bloom {
BloomControllerImpl::BloomControllerImpl(
signin::IdentityManager* identity_manager,
std::unique_ptr<ScreenshotGrabber> screenshot_grabber,
std::unique_ptr<BloomScreenshotDelegate> screenshot_delegate,
std::unique_ptr<BloomServerProxy> server_proxy)
: identity_manager_(identity_manager),
screenshot_grabber_(std::move(screenshot_grabber)),
screenshot_delegate_(std::move(screenshot_delegate)),
server_proxy_(std::move(server_proxy)) {
DCHECK(identity_manager_);
DCHECK(screenshot_grabber_);
DCHECK(screenshot_delegate_);
DCHECK(server_proxy_);
}
......@@ -76,10 +76,10 @@ void BloomControllerImpl::AddObserver(
owned_interaction_observers_.push_back(std::move(observer));
}
void BloomControllerImpl::SetScreenshotGrabberForTesting(
std::unique_ptr<ScreenshotGrabber> screenshot_grabber) {
DCHECK(screenshot_grabber);
screenshot_grabber_ = std::move(screenshot_grabber);
void BloomControllerImpl::SetScreenshotDelegateForTesting(
std::unique_ptr<BloomScreenshotDelegate> screenshot_delegate) {
DCHECK(screenshot_delegate);
screenshot_delegate_ = std::move(screenshot_delegate);
}
} // namespace bloom
......
......@@ -20,13 +20,14 @@ namespace chromeos {
namespace bloom {
class BloomInteraction;
class BloomScreenshotDelegate;
class BloomServerProxy;
class ScreenshotGrabber;
class BloomControllerImpl : public BloomController {
public:
BloomControllerImpl(signin::IdentityManager* identity_manager,
std::unique_ptr<ScreenshotGrabber> screenshot_grabber,
BloomControllerImpl(
signin::IdentityManager* identity_manager,
std::unique_ptr<BloomScreenshotDelegate> screenshot_delegate,
std::unique_ptr<BloomServerProxy> server_proxy);
BloomControllerImpl(const BloomControllerImpl&) = delete;
BloomControllerImpl& operator=(const BloomControllerImpl&) = delete;
......@@ -43,15 +44,18 @@ class BloomControllerImpl : public BloomController {
void ShowUI();
void ShowResult(const std::string& result);
ScreenshotGrabber* screenshot_grabber() { return screenshot_grabber_.get(); }
BloomScreenshotDelegate* screenshot_delegate() {
return screenshot_delegate_.get();
}
BloomServerProxy* server_proxy() { return server_proxy_.get(); }
signin::IdentityManager* identity_manager() { return identity_manager_; }
void SetScreenshotGrabberForTesting(std::unique_ptr<ScreenshotGrabber>);
void SetScreenshotDelegateForTesting(
std::unique_ptr<BloomScreenshotDelegate>);
private:
signin::IdentityManager* const identity_manager_;
std::unique_ptr<ScreenshotGrabber> screenshot_grabber_;
std::unique_ptr<BloomScreenshotDelegate> screenshot_delegate_;
std::unique_ptr<BloomServerProxy> server_proxy_;
base::ObserverList<BloomInteractionObserver> interaction_observers_;
......
......@@ -7,13 +7,15 @@
#include "chromeos/components/bloom/bloom_interaction_observer.h"
#include "chromeos/components/bloom/bloom_server_proxy.h"
#include "chromeos/components/bloom/public/cpp/bloom_interaction_resolution.h"
#include "chromeos/components/bloom/screenshot_grabber.h"
#include "chromeos/components/bloom/public/cpp/bloom_screenshot_delegate.h"
#include "chromeos/services/assistant/public/shared/constants.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/signin/public/identity_manager/identity_test_environment.h"
#include "components/signin/public/identity_manager/scope_set.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_unittest_util.h"
namespace chromeos {
namespace bloom {
......@@ -28,22 +30,22 @@ const char kEmail[] = "test@gmail.com";
#define EXPECT_NO_CALLS(args...) EXPECT_CALL(args).Times(0)
class ScreenshotGrabberMock : public ScreenshotGrabber {
class ScreenshotDelegateMock : public BloomScreenshotDelegate {
public:
ScreenshotGrabberMock() {
ScreenshotDelegateMock() {
ON_CALL(*this, TakeScreenshot).WillByDefault([this](Callback callback) {
// Store the callback passed to TakeScreenshot() so we can invoke it
// later from our tests.
this->callback_ = std::move(callback);
});
}
~ScreenshotGrabberMock() override = default;
~ScreenshotDelegateMock() override = default;
MOCK_METHOD(void, TakeScreenshot, (Callback callback));
// Sends the given screenshot to the callback passed to TakeScreenshot()
void SendScreenshot(
const base::Optional<Screenshot>& screenshot = Screenshot()) {
const base::Optional<gfx::Image>& screenshot = gfx::Image()) {
EXPECT_TRUE(callback_) << "TakeScreenshot() was never called.";
if (callback_)
......@@ -104,7 +106,7 @@ class BloomServerProxyMock : public BloomServerProxy {
BloomServerProxyMock() {
ON_CALL(*this, AnalyzeProblem)
.WillByDefault([this](const std::string& access_token,
const Screenshot screenshot, Callback callback) {
const gfx::Image& screenshot, Callback callback) {
// Store the callback passed to AnalyzeProblem() so we can invoke it
// later from our tests.
this->callback_ = std::move(callback);
......@@ -113,7 +115,7 @@ class BloomServerProxyMock : public BloomServerProxy {
MOCK_METHOD(void,
AnalyzeProblem,
(const std::string& access_token,
const Screenshot screenshot,
const gfx::Image& screenshot,
Callback callback));
void SendResponse(base::Optional<std::string> html = std::string("<html/>")) {
......@@ -156,10 +158,10 @@ class BloomControllerImplTest : public testing::Test {
protected:
void StartInteractionAndSendAccessTokenAndScreenshot(
std::string access_token = "<access-token>",
Screenshot screenshot = Screenshot()) {
gfx::Image screenshot = gfx::Image()) {
controller().StartInteraction();
IssueAccessToken(access_token);
screenshot_grabber_mock().SendScreenshot(screenshot);
screenshot_delegate_mock().SendScreenshot(screenshot);
}
// Returns the |ScopeSet| that was passed to the access token request.
......@@ -178,9 +180,9 @@ class BloomControllerImplTest : public testing::Test {
BloomControllerImpl& controller() { return controller_; }
ScreenshotGrabberMock& screenshot_grabber_mock() {
return *static_cast<ScreenshotGrabberMock*>(
controller_.screenshot_grabber());
ScreenshotDelegateMock& screenshot_delegate_mock() {
return *static_cast<ScreenshotDelegateMock*>(
controller_.screenshot_delegate());
}
BloomInteractionObserverMock* AddInteractionObserverMock() {
......@@ -217,7 +219,7 @@ class BloomControllerImplTest : public testing::Test {
BloomControllerImpl controller_{
identity_test_env_.identity_manager(),
std::make_unique<NiceMock<ScreenshotGrabberMock>>(),
std::make_unique<NiceMock<ScreenshotDelegateMock>>(),
std::make_unique<NiceMock<BloomServerProxyMock>>()};
};
......@@ -236,7 +238,7 @@ TEST_F(BloomControllerImplTest, ShouldFetchAccessToken) {
}
TEST_F(BloomControllerImplTest, ShouldTakeScreenshot) {
EXPECT_CALL(screenshot_grabber_mock(), TakeScreenshot);
EXPECT_CALL(screenshot_delegate_mock(), TakeScreenshot);
controller().StartInteraction();
}
......@@ -245,7 +247,7 @@ TEST_F(BloomControllerImplTest, ShouldAbortWhenFetchingAccessTokenFails) {
auto* interaction_tracker = AddInteractionTracker();
controller().StartInteraction();
screenshot_grabber_mock().SendScreenshot();
screenshot_delegate_mock().SendScreenshot();
FailAccessToken();
......@@ -260,7 +262,7 @@ TEST_F(BloomControllerImplTest, ShouldAbortWhenFetchingScreenshotFails) {
controller().StartInteraction();
IssueAccessToken();
screenshot_grabber_mock().SendScreenshotFailed();
screenshot_delegate_mock().SendScreenshotFailed();
EXPECT_FALSE(interaction_tracker->HasInteraction());
EXPECT_EQ(BloomInteractionResolution::kNoScreenshot,
......@@ -270,7 +272,7 @@ TEST_F(BloomControllerImplTest, ShouldAbortWhenFetchingScreenshotFails) {
TEST_F(BloomControllerImplTest,
ShouldPassAccessTokenAndScreenshotToBloomServer) {
const std::string& access_token = "<the-access-token>";
const Screenshot screenshot{1, 2, 3, 4, 5};
const gfx::Image screenshot = gfx::test::CreateImage(10, 20);
EXPECT_CALL(bloom_server(), AnalyzeProblem(access_token, screenshot, _));
......@@ -312,7 +314,7 @@ TEST_F(BloomInteractionObserverTest,
controller().StartInteraction();
IssueAccessToken();
screenshot_grabber_mock().SendScreenshot();
screenshot_delegate_mock().SendScreenshot();
}
TEST_F(BloomInteractionObserverTest,
......@@ -322,7 +324,7 @@ TEST_F(BloomInteractionObserverTest,
EXPECT_NO_CALLS(*observer, OnShowUI);
controller().StartInteraction();
screenshot_grabber_mock().SendScreenshot();
screenshot_delegate_mock().SendScreenshot();
}
TEST_F(BloomInteractionObserverTest,
......
......@@ -9,12 +9,13 @@
#include "chromeos/components/bloom/bloom_controller_impl.h"
#include "chromeos/components/bloom/bloom_interaction.h"
#include "chromeos/components/bloom/bloom_server_proxy.h"
#include "chromeos/components/bloom/public/cpp/bloom_screenshot_delegate.h"
#include "chromeos/components/bloom/public/cpp/future_value.h"
#include "chromeos/components/bloom/screenshot_grabber.h"
#include "chromeos/services/assistant/public/shared/constants.h"
#include "components/signin/public/identity_manager/access_token_info.h"
#include "components/signin/public/identity_manager/primary_account_access_token_fetcher.h"
#include "components/signin/public/identity_manager/scope_set.h"
#include "ui/gfx/image/image.h"
namespace chromeos {
namespace bloom {
......@@ -33,8 +34,10 @@ void BloomInteraction::Start() {
}
void BloomInteraction::StartAssistantInteraction(std::string&& access_token,
Screenshot&& screenshot) {
gfx::Image&& screenshot) {
DVLOG(2) << "Opening assistant UI";
controller_->ShowUI();
DVLOG(2) << "Contacting Bloom server";
controller_->server_proxy()->AnalyzeProblem(
access_token, screenshot, Bind(&BloomInteraction::OnServerResponse));
}
......@@ -45,10 +48,13 @@ void BloomInteraction::OnServerResponse(base::Optional<std::string> html) {
return;
}
DVLOG(2) << "Got server response";
controller_->ShowResult(html.value());
}
void BloomInteraction::FetchAccessTokenAsync() {
DVLOG(2) << "Fetching access token";
signin::ScopeSet scopes;
scopes.insert(assistant::kBloomScope);
......@@ -64,7 +70,7 @@ void BloomInteraction::FetchAccessTokenAsync() {
}
void BloomInteraction::FetchScreenshotAsync() {
controller_->screenshot_grabber()->TakeScreenshot(
controller_->screenshot_delegate()->TakeScreenshot(
Bind(&BloomInteraction::OnScreenshotReady));
screenshot_future_ = std::make_unique<ScreenshotFuture>();
}
......@@ -73,20 +79,24 @@ void BloomInteraction::OnAccessTokenRequestCompleted(
GoogleServiceAuthError error,
signin::AccessTokenInfo access_token_info) {
if (error.state() != GoogleServiceAuthError::NONE) {
LOG(WARNING) << "Failed to fetch the access token";
controller_->StopInteraction(BloomInteractionResolution::kNoAccessToken);
return;
}
DVLOG(2) << "Received access token";
access_token_future_->SetValue(std::move(access_token_info.token));
}
void BloomInteraction::OnScreenshotReady(
base::Optional<Screenshot> screenshot) {
base::Optional<gfx::Image> screenshot) {
if (!screenshot) {
LOG(WARNING) << "Failed to take the screenshot";
controller_->StopInteraction(BloomInteractionResolution::kNoScreenshot);
return;
}
DVLOG(2) << "Received screenshot";
screenshot_future_->SetValue(std::move(screenshot.value()));
}
......
......@@ -9,10 +9,13 @@
#include "base/bind.h"
#include "base/callback_forward.h"
#include "base/optional.h"
#include "chromeos/components/bloom/screenshot_grabber.h"
class GoogleServiceAuthError;
namespace gfx {
class Image;
}
namespace signin {
struct AccessTokenInfo;
class PrimaryAccountAccessTokenFetcher;
......@@ -26,7 +29,7 @@ class BloomControllerImpl;
template <typename _Type>
class FutureValue;
using AccessTokenFuture = FutureValue<std::string>;
using ScreenshotFuture = FutureValue<Screenshot>;
using ScreenshotFuture = FutureValue<gfx::Image>;
// A single Bloom interaction. This will:
// * Fetch the access token and screenshot.
......@@ -34,7 +37,7 @@ using ScreenshotFuture = FutureValue<Screenshot>;
// * Fetch the Bloom response and forward it to the Assistant interaction.
class BloomInteraction {
using StartCallback = base::OnceCallback<void(const std::string& access_token,
Screenshot&& screenshot)>;
gfx::Image&& screenshot)>;
public:
explicit BloomInteraction(BloomControllerImpl* controller);
......@@ -47,7 +50,7 @@ class BloomInteraction {
private:
void StartAssistantInteraction(std::string&& access_token,
Screenshot&& screenshot);
gfx::Image&& screenshot);
void OnServerResponse(base::Optional<std::string> html);
......@@ -56,7 +59,7 @@ class BloomInteraction {
void OnAccessTokenRequestCompleted(GoogleServiceAuthError error,
signin::AccessTokenInfo access_token_info);
void OnScreenshotReady(base::Optional<Screenshot> screenshot);
void OnScreenshotReady(base::Optional<gfx::Image> screenshot);
template <typename _Method, typename... Args>
auto Bind(_Method method, Args&&... args) {
......
......@@ -7,7 +7,10 @@
#include <string>
#include "base/callback.h"
#include "chromeos/components/bloom/screenshot_grabber.h"
namespace gfx {
class Image;
}
namespace chromeos {
namespace bloom {
......@@ -26,7 +29,7 @@ class BloomServerProxy {
// Send the screenshot to the Bloom server for analysis, and send the response
// to |callback|.
virtual void AnalyzeProblem(const std::string& access_token,
const Screenshot screenshot,
const gfx::Image& screenshot,
Callback callback) = 0;
};
......
......@@ -11,7 +11,7 @@ BloomServerProxyImpl::BloomServerProxyImpl() = default;
BloomServerProxyImpl::~BloomServerProxyImpl() = default;
void BloomServerProxyImpl::AnalyzeProblem(const std::string& access_token,
const Screenshot screenshot,
const gfx::Image& screenshot,
Callback callback) {
// TODO(jeroendh): implement
}
......
......@@ -16,7 +16,7 @@ class BloomServerProxyImpl : public BloomServerProxy {
~BloomServerProxyImpl() override;
void AnalyzeProblem(const std::string& access_token,
const Screenshot screenshot,
const gfx::Image& screenshot,
Callback callback) override;
};
......
......@@ -10,6 +10,7 @@ component("cpp") {
"bloom_controller.h",
"bloom_interaction_resolution.cc",
"bloom_interaction_resolution.h",
"bloom_screenshot_delegate.h",
]
deps = [ "//base" ]
......@@ -27,6 +28,7 @@ component("bloom_controller_factory") {
]
deps = [
":cpp",
"//base",
"//chromeos/components/bloom",
]
......
......@@ -10,27 +10,18 @@
#include "chromeos/components/bloom/bloom_controller_impl.h"
#include "chromeos/components/bloom/bloom_interaction_observer_impl.h"
#include "chromeos/components/bloom/bloom_server_proxy_impl.h"
#include "chromeos/components/bloom/screenshot_grabber.h"
#include "chromeos/components/bloom/public/cpp/bloom_screenshot_delegate.h"
namespace chromeos {
namespace bloom {
namespace {
// TODO(jeroendh): Replace with actual working screenshot grabber.
class FakeScreenshotGrabber : public ScreenshotGrabber {
public:
void TakeScreenshot(Callback callback) override { NOTIMPLEMENTED(); }
};
} // namespace
// static
std::unique_ptr<BloomController> BloomControllerFactory::Create(
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
signin::IdentityManager* identity_manager) {
signin::IdentityManager* identity_manager,
std::unique_ptr<BloomScreenshotDelegate> screenshot_delegate) {
auto result = std::make_unique<BloomControllerImpl>(
identity_manager, std::make_unique<FakeScreenshotGrabber>(),
identity_manager, std::move(screenshot_delegate),
std::make_unique<BloomServerProxyImpl>());
result->AddObserver(std::make_unique<BloomInteractionObserverImpl>());
......
......@@ -22,13 +22,15 @@ namespace chromeos {
namespace bloom {
class BloomController;
class BloomScreenshotDelegate;
class COMPONENT_EXPORT(BLOOM) BloomControllerFactory {
public:
// Create the Bloom controller. Can only be invoked once.
static std::unique_ptr<BloomController> Create(
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
signin::IdentityManager* identity_manager);
signin::IdentityManager* identity_manager,
std::unique_ptr<BloomScreenshotDelegate> screenshot_delegate);
};
} // namespace bloom
......
......@@ -2,31 +2,31 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROMEOS_COMPONENTS_BLOOM_SCREENSHOT_GRABBER_H_
#define CHROMEOS_COMPONENTS_BLOOM_SCREENSHOT_GRABBER_H_
#ifndef CHROMEOS_COMPONENTS_BLOOM_PUBLIC_CPP_BLOOM_SCREENSHOT_DELEGATE_H_
#define CHROMEOS_COMPONENTS_BLOOM_PUBLIC_CPP_BLOOM_SCREENSHOT_DELEGATE_H_
#include <vector>
#include "base/callback_forward.h"
#include "base/optional.h"
namespace gfx {
class Image;
}
namespace chromeos {
namespace bloom {
using Screenshot = std::vector<uint8_t>;
// Interface to grab a screenshot.
class ScreenshotGrabber {
// Prompts the user to select a part of the screen, and invokes the callback
// with a screenshot of the selected area.
class BloomScreenshotDelegate {
public:
using Callback =
base::OnceCallback<void(base::Optional<Screenshot> screenshot)>;
virtual ~ScreenshotGrabber() = default;
using Callback = base::OnceCallback<void(base::Optional<gfx::Image>)>;
virtual ~BloomScreenshotDelegate() = default;
// Asynchronously takes a screenshot, and passes it to the callback.
// If this fails or is aborted, will pass |base::nullopt| to the callback.
virtual void TakeScreenshot(Callback callback) = 0;
virtual void TakeScreenshot(Callback ready_callback) = 0;
};
} // namespace bloom
} // namespace chromeos
#endif // CHROMEOS_COMPONENTS_BLOOM_SCREENSHOT_GRABBER_H_
#endif // CHROMEOS_COMPONENTS_BLOOM_PUBLIC_CPP_BLOOM_SCREENSHOT_DELEGATE_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