Commit 33a62e25 authored by Samuel Huang's avatar Samuel Huang Committed by Commit Bot

[DevUI DFM] Improve DevUiModuleProvider testability.

Previously, class DevUiModuleProvider consists of static functions
{ModuleInstalled(), InstallModule(), LoadModule} that are called
directly. Unfortunately, these cannot be mocked for testing.

This CL makes DevUiModuleProvider a singleton class that's accessed
via GetInstance(). SetTestInstance() is added to allow a test to
override DevUiModuleProvider functions, and inject a test instance
that replaces the original singleton obtained from GetInstance().

Bug: 927131
TBR: yfriedman@chromium.org
Change-Id: Ifee6899ebf67be17ad2ed15103b9c7b6d8e322a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863553Reviewed-by: default avatarSamuel Huang <huangs@chromium.org>
Reviewed-by: default avatarTibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706160}
parent 88ae02d7
......@@ -13,12 +13,30 @@
namespace dev_ui {
// static
DevUiModuleProvider* DevUiModuleProvider::test_instance_ = nullptr;
// Destructor is public to enable management by std::unique_ptr<>.
DevUiModuleProvider::~DevUiModuleProvider() = default;
// static
DevUiModuleProvider* DevUiModuleProvider::GetInstance() {
if (test_instance_)
return test_instance_;
static DevUiModuleProvider instance;
return &instance;
}
// static
void DevUiModuleProvider::SetTestInstance(DevUiModuleProvider* test_instance) {
test_instance_ = test_instance;
}
bool DevUiModuleProvider::ModuleInstalled() {
return Java_DevUiModuleProvider_isModuleInstalled(
base::android::AttachCurrentThread());
}
// static
void DevUiModuleProvider::InstallModule(
base::OnceCallback<void(bool)> on_complete) {
auto* listener = DevUiInstallListener::Create(std::move(on_complete));
......@@ -28,13 +46,10 @@ void DevUiModuleProvider::InstallModule(
listener->j_listener());
}
// static
void DevUiModuleProvider::LoadModule() {
Java_DevUiModuleProvider_loadModule(base::android::AttachCurrentThread());
}
DevUiModuleProvider::DevUiModuleProvider() = default;
DevUiModuleProvider::~DevUiModuleProvider() = default;
} // namespace dev_ui
......@@ -11,23 +11,33 @@ namespace dev_ui {
class DevUiModuleProvider {
public:
// Returns the singleton, which can be overridden using SetTestInstance().
static DevUiModuleProvider* GetInstance();
// Overrides the singleton with caller-owned |test_instance|. Caller tests
// are responsible for resetting this to null on cleanup.
static void SetTestInstance(DevUiModuleProvider* test_instance);
// Returns true if the DevUI module is installed.
static bool ModuleInstalled();
virtual bool ModuleInstalled();
// Asynchronously requests to install the DevUI module. |on_complete| is
// called after the module install is completed, and takes a bool to indicate
// whether module install is successful.
static void InstallModule(base::OnceCallback<void(bool)> on_complete);
virtual void InstallModule(base::OnceCallback<void(bool)> on_complete);
// Assuming that the DevUI module is installed, loads DevUI resources if not
// already loaded.
static void LoadModule();
virtual void LoadModule();
private:
protected:
DevUiModuleProvider();
~DevUiModuleProvider();
virtual ~DevUiModuleProvider();
DevUiModuleProvider(const DevUiModuleProvider&) = delete;
DevUiModuleProvider& operator=(const DevUiModuleProvider&) = delete;
private:
static DevUiModuleProvider* test_instance_;
};
} // namespace dev_ui
......
......@@ -83,9 +83,9 @@ bool HandleDfmifiedDevUiPageURL(
}
// If module is already installed, ensure that it is loaded.
if (dev_ui::DevUiModuleProvider::ModuleInstalled()) {
if (dev_ui::DevUiModuleProvider::GetInstance()->ModuleInstalled()) {
// Synchronously load module (if not already loaded).
dev_ui::DevUiModuleProvider::LoadModule();
dev_ui::DevUiModuleProvider::GetInstance()->LoadModule();
return false;
}
// Create URL to the DevUI loader with "?url=<escaped original URL>" so that
......
......@@ -28,9 +28,10 @@ void DevUiLoaderMessageHandler::HandleGetDevUiDfmState(
const base::ListValue* args) {
const base::Value* callback_id = nullptr;
CHECK(args->Get(0, &callback_id));
const char* response = dev_ui::DevUiModuleProvider::ModuleInstalled()
? "installed"
: "not-installed";
const char* response =
dev_ui::DevUiModuleProvider::GetInstance()->ModuleInstalled()
? "installed"
: "not-installed";
AllowJavascript();
ResolveJavascriptCallback(*callback_id, base::Value(response));
}
......@@ -49,8 +50,8 @@ void DevUiLoaderMessageHandler::HandleInstallDevUiDfm(
const base::Value* callback_id = nullptr;
CHECK(args->Get(0, &callback_id));
if (!dev_ui::DevUiModuleProvider::ModuleInstalled()) {
dev_ui::DevUiModuleProvider::InstallModule(base::BindOnce(
if (!dev_ui::DevUiModuleProvider::GetInstance()->ModuleInstalled()) {
dev_ui::DevUiModuleProvider::GetInstance()->InstallModule(base::BindOnce(
&DevUiLoaderMessageHandler::OnDevUiDfmInstallWithStatus,
weak_ptr_factory_.GetWeakPtr(), callback_id->GetString()));
} else {
......
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