Commit c79ca567 authored by Will Cassella's avatar Will Cassella Committed by Commit Bot

Refactor DefaultWebClientWorker methods to take a callback

This moves DefaultWebClientWorkerCallback out of the constructor for
DefaultWebClientWorker and changes it to be a parameter for the
`Start(CheckIs|SetAs)Default` methods. This allows it to be a
base::OnceCallback instead of a base::RepeatingCallback.

Bug: 1106803
Change-Id: I47887ce0ca1db3b469fd1edf86f4b824c45685eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2315084Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Will Cassella <cassew@google.com>
Cr-Commit-Position: refs/heads/master@{#791335}
parent 72d70865
......@@ -1370,13 +1370,12 @@ void BrowserProcessImpl::ApplyDefaultBrowserPolicy() {
// message loops of the FILE and UI thread will hold references to it
// and it will be automatically freed once all its tasks have finished.
auto set_browser_worker =
base::MakeRefCounted<shell_integration::DefaultBrowserWorker>(
shell_integration::DefaultWebClientWorkerCallback());
base::MakeRefCounted<shell_integration::DefaultBrowserWorker>();
// The user interaction must always be disabled when applying the default
// browser policy since it is done at each browser startup and the result
// of the interaction cannot be forced.
set_browser_worker->set_interactive_permitted(false);
set_browser_worker->StartSetAsDefault();
set_browser_worker->StartSetAsDefault(base::NullCallback());
}
}
......
......@@ -101,9 +101,8 @@ void ProtocolHandlerRegistry::Delegate::RegisterWithOSAsDefaultClient(
// The worker pointer is reference counted. While it is running, the
// sequence it runs on will hold references it will be automatically freed
// once all its tasks have finished.
base::MakeRefCounted<shell_integration::DefaultProtocolClientWorker>(
std::move(callback), protocol)
->StartSetAsDefault();
base::MakeRefCounted<shell_integration::DefaultProtocolClientWorker>(protocol)
->StartSetAsDefault(std::move(callback));
}
void ProtocolHandlerRegistry::Delegate::CheckDefaultClientWithOS(
......@@ -112,9 +111,8 @@ void ProtocolHandlerRegistry::Delegate::CheckDefaultClientWithOS(
// The worker pointer is reference counted. While it is running, the
// sequence it runs on will hold references it will be automatically freed
// once all its tasks have finished.
base::MakeRefCounted<shell_integration::DefaultProtocolClientWorker>(
std::move(callback), protocol)
->StartCheckIsDefault();
base::MakeRefCounted<shell_integration::DefaultProtocolClientWorker>(protocol)
->StartCheckIsDefault(std::move(callback));
}
// ProtocolHandlerRegistry -----------------------------------------------------
......
......@@ -72,9 +72,9 @@ class FakeDelegate : public ProtocolHandlerRegistry::Delegate {
// the result with a task to the current thread.
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::BindOnce(callback, force_os_failure_
? shell_integration::NOT_DEFAULT
: shell_integration::IS_DEFAULT));
base::BindOnce(std::move(callback),
force_os_failure_ ? shell_integration::NOT_DEFAULT
: shell_integration::IS_DEFAULT));
if (!force_os_failure_)
os_registered_protocols_.insert(protocol);
......
......@@ -5,6 +5,7 @@
#include "chrome/browser/external_protocol/external_protocol_handler.h"
#include <stddef.h>
#include <utility>
#include "base/bind.h"
#include "base/check_op.h"
......@@ -79,13 +80,12 @@ constexpr const char* kAllowedSchemes[] = {
// Functions enabling unit testing. Using a NULL delegate will use the default
// behavior; if a delegate is provided it will be used instead.
scoped_refptr<shell_integration::DefaultProtocolClientWorker> CreateShellWorker(
const shell_integration::DefaultWebClientWorkerCallback& callback,
const std::string& protocol,
ExternalProtocolHandler::Delegate* delegate) {
if (delegate)
return delegate->CreateShellWorker(callback, protocol);
return delegate->CreateShellWorker(protocol);
return base::MakeRefCounted<shell_integration::DefaultProtocolClientWorker>(
callback, protocol);
protocol);
}
ExternalProtocolHandler::BlockState GetBlockStateWithDelegate(
......@@ -436,7 +436,7 @@ void ExternalProtocolHandler::LaunchUrl(
// The worker creates tasks with references to itself and puts them into
// message loops.
shell_integration::DefaultWebClientWorkerCallback callback = base::Bind(
shell_integration::DefaultWebClientWorkerCallback callback = base::BindOnce(
&OnDefaultProtocolClientWorkerFinished, escaped_url,
render_process_host_id, render_view_routing_id, block_state == UNKNOWN,
page_transition, has_user_gesture, initiating_origin_or_precursor,
......@@ -445,9 +445,8 @@ void ExternalProtocolHandler::LaunchUrl(
// Start the check process running. This will send tasks to a worker task
// runner and when the answer is known will send the result back to
// OnDefaultProtocolClientWorkerFinished().
CreateShellWorker(callback, escaped_url.scheme(),
g_external_protocol_handler_delegate)
->StartCheckIsDefault();
CreateShellWorker(escaped_url.scheme(), g_external_protocol_handler_delegate)
->StartCheckIsDefault(std::move(callback));
}
// static
......
......@@ -47,9 +47,7 @@ class ExternalProtocolHandler {
class Delegate {
public:
virtual scoped_refptr<shell_integration::DefaultProtocolClientWorker>
CreateShellWorker(
const shell_integration::DefaultWebClientWorkerCallback& callback,
const std::string& protocol) = 0;
CreateShellWorker(const std::string& protocol) = 0;
virtual BlockState GetBlockState(const std::string& scheme,
Profile* profile) = 0;
virtual void BlockRequest() = 0;
......
......@@ -4,6 +4,7 @@
#include "chrome/browser/external_protocol/external_protocol_handler.h"
#include <memory>
#include <utility>
#include "base/run_loop.h"
......@@ -25,10 +26,9 @@ class FakeExternalProtocolHandlerWorker
: public shell_integration::DefaultProtocolClientWorker {
public:
FakeExternalProtocolHandlerWorker(
const shell_integration::DefaultWebClientWorkerCallback& callback,
const std::string& protocol,
shell_integration::DefaultWebClientState os_state)
: shell_integration::DefaultProtocolClientWorker(callback, protocol),
: shell_integration::DefaultProtocolClientWorker(protocol),
os_state_(os_state) {}
private:
......@@ -59,9 +59,8 @@ class FakeExternalProtocolHandlerDelegate
scoped_refptr<shell_integration::DefaultProtocolClientWorker>
CreateShellWorker(
const shell_integration::DefaultWebClientWorkerCallback& callback,
const std::string& protocol) override {
return new FakeExternalProtocolHandlerWorker(callback, protocol, os_state_);
return new FakeExternalProtocolHandlerWorker(protocol, os_state_);
}
ExternalProtocolHandler::BlockState GetBlockState(const std::string& scheme,
......
......@@ -69,10 +69,8 @@ void WaitForTitle(content::WebContents* contents, const char* expected_title) {
class FakeDefaultProtocolClientWorker
: public shell_integration::DefaultProtocolClientWorker {
public:
FakeDefaultProtocolClientWorker(
const shell_integration::DefaultWebClientWorkerCallback& callback,
const std::string& protocol)
: DefaultProtocolClientWorker(callback, protocol) {}
explicit FakeDefaultProtocolClientWorker(const std::string& protocol)
: DefaultProtocolClientWorker(protocol) {}
private:
~FakeDefaultProtocolClientWorker() override = default;
......@@ -100,11 +98,8 @@ class FakeProtocolHandlerDelegate : public ExternalProtocolHandler::Delegate {
private:
scoped_refptr<shell_integration::DefaultProtocolClientWorker>
CreateShellWorker(
const shell_integration::DefaultWebClientWorkerCallback& callback,
const std::string& protocol) override {
return base::MakeRefCounted<FakeDefaultProtocolClientWorker>(callback,
protocol);
CreateShellWorker(const std::string& protocol) override {
return base::MakeRefCounted<FakeDefaultProtocolClientWorker>(protocol);
}
ExternalProtocolHandler::BlockState GetBlockState(const std::string& scheme,
......
......@@ -54,9 +54,7 @@ class NeverRunsExternalProtocolHandlerDelegate
: public ExternalProtocolHandler::Delegate {
public:
scoped_refptr<shell_integration::DefaultProtocolClientWorker>
CreateShellWorker(
const shell_integration::DefaultWebClientWorkerCallback& callback,
const std::string& protocol) override {
CreateShellWorker(const std::string& protocol) override {
NOTREACHED();
// This will crash, but it shouldn't get this far with BlockState::BLOCK
// anyway.
......
......@@ -64,6 +64,23 @@ base::LazyThreadPoolSequencedTaskRunner g_sequenced_task_runner =
base::TaskTraits(base::MayBlock()));
#endif
void RunCallback(DefaultWebClientWorkerCallback callback,
DefaultWebClientState state) {
if (!callback.is_null()) {
switch (state) {
case NOT_DEFAULT:
case IS_DEFAULT:
case UNKNOWN_DEFAULT:
case OTHER_MODE_IS_DEFAULT:
std::move(callback).Run(state);
return;
case NUM_DEFAULT_STATES:
break;
}
NOTREACHED();
}
}
} // namespace
bool CanSetAsDefaultBrowser() {
......@@ -85,7 +102,7 @@ const struct AppModeInfo* AppModeInfo() {
}
bool IsRunningInAppMode() {
return gAppModeInfo != NULL;
return gAppModeInfo != nullptr;
}
base::CommandLine CommandLineArgsForLauncher(
......@@ -156,32 +173,34 @@ base::string16 GetAppShortcutsSubdirName() {
// DefaultWebClientWorker
//
void DefaultWebClientWorker::StartCheckIsDefault() {
void DefaultWebClientWorker::StartCheckIsDefault(
DefaultWebClientWorkerCallback callback) {
g_sequenced_task_runner.Get()->PostTask(
FROM_HERE,
base::BindOnce(&DefaultWebClientWorker::CheckIsDefault, this, false));
FROM_HERE, base::BindOnce(&DefaultWebClientWorker::CheckIsDefault, this,
false, std::move(callback)));
}
void DefaultWebClientWorker::StartSetAsDefault() {
void DefaultWebClientWorker::StartSetAsDefault(
DefaultWebClientWorkerCallback callback) {
g_sequenced_task_runner.Get()->PostTask(
FROM_HERE, base::BindOnce(&DefaultWebClientWorker::SetAsDefault, this));
FROM_HERE, base::BindOnce(&DefaultWebClientWorker::SetAsDefault, this,
std::move(callback)));
}
///////////////////////////////////////////////////////////////////////////////
// DefaultWebClientWorker, protected:
DefaultWebClientWorker::DefaultWebClientWorker(
const DefaultWebClientWorkerCallback& callback,
const char* worker_name)
: callback_(callback), worker_name_(worker_name) {}
DefaultWebClientWorker::DefaultWebClientWorker(const char* worker_name)
: worker_name_(worker_name) {}
DefaultWebClientWorker::~DefaultWebClientWorker() = default;
void DefaultWebClientWorker::OnCheckIsDefaultComplete(
DefaultWebClientState state,
bool is_following_set_as_default) {
bool is_following_set_as_default,
DefaultWebClientWorkerCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
UpdateUI(state);
RunCallback(std::move(callback), state);
if (is_following_set_as_default)
ReportSetDefaultResult(state);
......@@ -190,23 +209,27 @@ void DefaultWebClientWorker::OnCheckIsDefaultComplete(
///////////////////////////////////////////////////////////////////////////////
// DefaultWebClientWorker, private:
void DefaultWebClientWorker::CheckIsDefault(bool is_following_set_as_default) {
void DefaultWebClientWorker::CheckIsDefault(
bool is_following_set_as_default,
DefaultWebClientWorkerCallback callback) {
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
base::BlockingType::MAY_BLOCK);
DefaultWebClientState state = CheckIsDefaultImpl();
content::GetUIThreadTaskRunner({})->PostTask(
FROM_HERE, base::BindOnce(&DefaultBrowserWorker::OnCheckIsDefaultComplete,
this, state, is_following_set_as_default));
FROM_HERE,
base::BindOnce(&DefaultBrowserWorker::OnCheckIsDefaultComplete, this,
state, is_following_set_as_default, std::move(callback)));
}
void DefaultWebClientWorker::SetAsDefault() {
void DefaultWebClientWorker::SetAsDefault(
DefaultWebClientWorkerCallback callback) {
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
base::BlockingType::MAY_BLOCK);
// SetAsDefaultImpl will make sure the callback is executed exactly once.
SetAsDefaultImpl(
base::BindOnce(&DefaultWebClientWorker::CheckIsDefault, this, true));
SetAsDefaultImpl(base::BindOnce(&DefaultWebClientWorker::CheckIsDefault, this,
true, std::move(callback)));
}
void DefaultWebClientWorker::ReportSetDefaultResult(
......@@ -219,29 +242,12 @@ void DefaultWebClientWorker::ReportSetDefaultResult(
->Add(state);
}
void DefaultWebClientWorker::UpdateUI(DefaultWebClientState state) {
if (!callback_.is_null()) {
switch (state) {
case NOT_DEFAULT:
case IS_DEFAULT:
case UNKNOWN_DEFAULT:
case OTHER_MODE_IS_DEFAULT:
callback_.Run(state);
return;
case NUM_DEFAULT_STATES:
break;
}
NOTREACHED();
}
}
///////////////////////////////////////////////////////////////////////////////
// DefaultBrowserWorker
//
DefaultBrowserWorker::DefaultBrowserWorker(
const DefaultWebClientWorkerCallback& callback)
: DefaultWebClientWorker(callback, "DefaultBrowser") {}
DefaultBrowserWorker::DefaultBrowserWorker()
: DefaultWebClientWorker("DefaultBrowser") {}
///////////////////////////////////////////////////////////////////////////////
// DefaultBrowserWorker, private:
......@@ -287,10 +293,8 @@ void DefaultBrowserWorker::SetAsDefaultImpl(
//
DefaultProtocolClientWorker::DefaultProtocolClientWorker(
const DefaultWebClientWorkerCallback& callback,
const std::string& protocol)
: DefaultWebClientWorker(callback, "DefaultProtocolClient"),
protocol_(protocol) {}
: DefaultWebClientWorker("DefaultProtocolClient"), protocol_(protocol) {}
///////////////////////////////////////////////////////////////////////////////
// DefaultProtocolClientWorker, protected:
......
......@@ -143,7 +143,7 @@ base::string16 GetAppShortcutsSubdirName();
// The type of callback used to communicate processing state to consumers of
// DefaultBrowserWorker and DefaultProtocolClientWorker.
using DefaultWebClientWorkerCallback =
base::RepeatingCallback<void(DefaultWebClientState)>;
base::OnceCallback<void(DefaultWebClientState)>;
// Helper objects that handle checking if Chrome is the default browser
// or application for a url protocol on Windows and Linux, and also setting
......@@ -165,27 +165,27 @@ class DefaultWebClientWorker
}
// Checks to see if Chrome is the default web client application. The
// instance's callback will be run to communicate the default state to the
// provided callback will be run to communicate the default state to the
// caller.
void StartCheckIsDefault();
void StartCheckIsDefault(DefaultWebClientWorkerCallback callback);
// Sets Chrome as the default web client application. Once done, it will
// trigger a check for the default state using StartCheckIsDefault() to return
// the default state to the caller.
void StartSetAsDefault();
void StartSetAsDefault(DefaultWebClientWorkerCallback callback);
protected:
friend class base::RefCountedThreadSafe<DefaultWebClientWorker>;
DefaultWebClientWorker(const DefaultWebClientWorkerCallback& callback,
const char* worker_name);
explicit DefaultWebClientWorker(const char* worker_name);
virtual ~DefaultWebClientWorker();
// Communicates the result via the |callback_|. When
// Communicates the result via |callback|. When
// |is_following_set_as_default| is true, |state| will be reported to UMA as
// the result of the set-as-default operation.
void OnCheckIsDefaultComplete(DefaultWebClientState state,
bool is_following_set_as_default);
bool is_following_set_as_default,
DefaultWebClientWorkerCallback callback);
// When false, the operation to set as default will fail for interactive
// flows.
......@@ -196,11 +196,12 @@ class DefaultWebClientWorker
// blocking sequence. When |is_following_set_as_default| is true, The default
// state will be reported to UMA as the result of the set-as-default
// operation.
void CheckIsDefault(bool is_following_set_as_default);
void CheckIsDefault(bool is_following_set_as_default,
DefaultWebClientWorkerCallback callback);
// Sets Chrome as the default web client. Always called on a blocking
// sequence.
void SetAsDefault();
void SetAsDefault(DefaultWebClientWorkerCallback callback);
// Implementation of CheckIsDefault() and SetAsDefault() for subclasses.
virtual DefaultWebClientState CheckIsDefaultImpl() = 0;
......@@ -213,13 +214,6 @@ class DefaultWebClientWorker
// Reports the result for the set-as-default operation.
void ReportSetDefaultResult(DefaultWebClientState state);
// Updates the UI in our associated view with the current default web
// client state.
void UpdateUI(DefaultWebClientState state);
// Called with the default state after the worker is done.
DefaultWebClientWorkerCallback callback_;
// Used to differentiate UMA metrics for setting the default browser and
// setting the default protocol client. The pointer must be valid for the
// lifetime of the worker.
......@@ -231,7 +225,7 @@ class DefaultWebClientWorker
// Worker for checking and setting the default browser.
class DefaultBrowserWorker : public DefaultWebClientWorker {
public:
explicit DefaultBrowserWorker(const DefaultWebClientWorkerCallback& callback);
DefaultBrowserWorker();
protected:
~DefaultBrowserWorker() override;
......@@ -252,8 +246,7 @@ class DefaultBrowserWorker : public DefaultWebClientWorker {
// multiple protocols you should use multiple worker objects.
class DefaultProtocolClientWorker : public DefaultWebClientWorker {
public:
DefaultProtocolClientWorker(const DefaultWebClientWorkerCallback& callback,
const std::string& protocol);
explicit DefaultProtocolClientWorker(const std::string& protocol);
const std::string& protocol() const { return protocol_; }
......
......@@ -448,10 +448,8 @@ class MailtoExternalProtocolHandlerDelegate
scoped_refptr<shell_integration::DefaultProtocolClientWorker>
CreateShellWorker(
const shell_integration::DefaultWebClientWorkerCallback& callback,
const std::string& protocol) override {
return new shell_integration::DefaultProtocolClientWorker(callback,
protocol);
return new shell_integration::DefaultProtocolClientWorker(protocol);
}
ExternalProtocolHandler::BlockState GetBlockState(const std::string& scheme,
......@@ -1497,7 +1495,8 @@ IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessTest, JSPrintDuringSwap) {
"a.com", "/print_during_load_with_broken_pdf_then_navigate.html"));
ui_test_utils::NavigateToURL(browser(), main_url);
// Ensure the first process did not crash when the queued print() fires during frame detach.
// Ensure the first process did not crash when the queued print() fires
// during frame detach.
EXPECT_TRUE(WaitForLoadStop(contents));
watcher.Wait();
EXPECT_TRUE(watcher.did_exit_normally());
......
......@@ -175,12 +175,11 @@ ChromeAppDelegate::NewWindowContentsDelegate::OpenURLFromTab(
// tasks.
scoped_refptr<shell_integration::DefaultBrowserWorker>
check_if_default_browser_worker =
new shell_integration::DefaultBrowserWorker(
base::Bind(&OpenURLAfterCheckIsDefaultBrowser,
base::Passed(&owned_source), params));
check_if_default_browser_worker->StartCheckIsDefault();
new shell_integration::DefaultBrowserWorker();
check_if_default_browser_worker->StartCheckIsDefault(base::BindOnce(
&OpenURLAfterCheckIsDefaultBrowser, std::move(owned_source), params));
}
return NULL;
return nullptr;
}
ChromeAppDelegate::ChromeAppDelegate(bool keep_alive)
......
......@@ -110,9 +110,8 @@ bool DefaultBrowserInfoBarDelegate::Accept() {
// The worker pointer is reference counted. While it is running, the
// message loops of the FILE and UI thread will hold references to it
// and it will be automatically freed once all its tasks have finished.
base::MakeRefCounted<shell_integration::DefaultBrowserWorker>(
shell_integration::DefaultWebClientWorkerCallback())
->StartSetAsDefault();
base::MakeRefCounted<shell_integration::DefaultBrowserWorker>()
->StartSetAsDefault(base::NullCallback());
return true;
}
......
......@@ -158,10 +158,10 @@ void ShowDefaultBrowserPrompt(Profile* profile) {
}
scoped_refptr<shell_integration::DefaultBrowserWorker>(
new shell_integration::DefaultBrowserWorker(
base::Bind(&OnCheckIsDefaultBrowserFinished, profile->GetPath(),
ShouldShowDefaultBrowserPrompt(profile))))
->StartCheckIsDefault();
new shell_integration::DefaultBrowserWorker())
->StartCheckIsDefault(
base::BindOnce(&OnCheckIsDefaultBrowserFinished, profile->GetPath(),
ShouldShowDefaultBrowserPrompt(profile)));
}
void DefaultBrowserPromptDeclined(Profile* profile) {
......
......@@ -71,9 +71,7 @@ class ExternalProtocolDialogBrowserTest
// ExternalProtocolHander::Delegate:
scoped_refptr<shell_integration::DefaultProtocolClientWorker>
CreateShellWorker(
const shell_integration::DefaultWebClientWorkerCallback& callback,
const std::string& protocol) override {
CreateShellWorker(const std::string& protocol) override {
return nullptr;
}
ExternalProtocolHandler::BlockState GetBlockState(const std::string& scheme,
......
......@@ -30,9 +30,9 @@ bool DefaultBrowserIsDisabledByPolicy() {
} // namespace
DefaultBrowserHandler::DefaultBrowserHandler() {}
DefaultBrowserHandler::DefaultBrowserHandler() = default;
DefaultBrowserHandler::~DefaultBrowserHandler() {}
DefaultBrowserHandler::~DefaultBrowserHandler() = default;
void DefaultBrowserHandler::RegisterMessages() {
web_ui()->RegisterMessageCallback(
......@@ -52,9 +52,7 @@ void DefaultBrowserHandler::OnJavascriptAllowed() {
prefs::kDefaultBrowserSettingEnabled,
base::Bind(&DefaultBrowserHandler::RequestDefaultBrowserState,
base::Unretained(this), nullptr));
default_browser_worker_ = new shell_integration::DefaultBrowserWorker(
base::Bind(&DefaultBrowserHandler::OnDefaultBrowserWorkerFinished,
weak_ptr_factory_.GetWeakPtr()));
default_browser_worker_ = new shell_integration::DefaultBrowserWorker();
}
void DefaultBrowserHandler::OnJavascriptDisallowed() {
......@@ -70,7 +68,9 @@ void DefaultBrowserHandler::RequestDefaultBrowserState(
CHECK_EQ(args->GetSize(), 1U);
CHECK(args->GetString(0, &check_default_callback_id_));
default_browser_worker_->StartCheckIsDefault();
default_browser_worker_->StartCheckIsDefault(
base::BindOnce(&DefaultBrowserHandler::OnDefaultBrowserWorkerFinished,
weak_ptr_factory_.GetWeakPtr()));
}
void DefaultBrowserHandler::SetAsDefaultBrowser(const base::ListValue* args) {
......@@ -78,7 +78,9 @@ void DefaultBrowserHandler::SetAsDefaultBrowser(const base::ListValue* args) {
AllowJavascript();
RecordSetAsDefaultUMA();
default_browser_worker_->StartSetAsDefault();
default_browser_worker_->StartSetAsDefault(
base::BindOnce(&DefaultBrowserHandler::OnDefaultBrowserWorkerFinished,
weak_ptr_factory_.GetWeakPtr()));
// If the user attempted to make Chrome the default browser, notify
// them when this changes.
......
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