Skip to content

Commit

Permalink
Allow the ProtocolHandlerRegistry to operate without a pref storage
Browse files Browse the repository at this point in the history
The bug 1293295 describes the task to implement the
registerProtocolHandler method in the ContentShell.  The goal is to
be able to define Web Platform Test for the custom handler spec [1].

The current implementation of the ProtocolHandlerRegistry requires the
user preference storage to be always enabled, in order to have a
persistent storage for the registered handlers. However, this feature
is not described in the mentioned spec; it's up to each engine
implementation to provide, if needed, its own persistent storage
approach.

It makes sense that for testing purposes the registry could operate
without any persistent storage at all and just use the data structures
in memory to verify the tests results. This CL changes the preferences
related logic in the registry to consider the possibility of a null
PrefService instance.

[1] https://html.spec.whatwg.org/multipage/system-state.html#custom-handlers

Bug: 1293295
Change-Id: I00e5b0056b6abd3417d1d11aaa9cc83951b826e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3541005
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/main@{#984740}
  • Loading branch information
javifernandez authored and Chromium LUCI CQ committed Mar 24, 2022
1 parent 5c6af8e commit e7db8a1
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 26 deletions.
Expand Up @@ -487,7 +487,7 @@ std::unique_ptr<KeyedService> BuildProtocolHandlerRegistry(
content::BrowserContext* context) {
Profile* profile = Profile::FromBrowserContext(context);
return std::make_unique<custom_handlers::ProtocolHandlerRegistry>(
profile,
profile->GetPrefs(),
std::make_unique<custom_handlers::TestProtocolHandlerRegistryDelegate>());
}

Expand Down
Expand Up @@ -47,7 +47,7 @@ class SiteSettingsCounterTest : public testing::Test {
#endif
handler_registry_ =
std::make_unique<custom_handlers::ProtocolHandlerRegistry>(
profile(),
profile()->GetPrefs(),
std::make_unique<
custom_handlers::TestProtocolHandlerRegistryDelegate>());

Expand Down
Expand Up @@ -33,7 +33,7 @@ class ChromeProtocolHandlerRegistryTest : public testing::Test {
CHECK(profile_->GetPrefs());
auto delegate = std::make_unique<
custom_handlers::TestProtocolHandlerRegistryDelegate>();
registry_ = std::make_unique<ProtocolHandlerRegistry>(profile_.get(),
registry_ = std::make_unique<ProtocolHandlerRegistry>(profile_->GetPrefs(),
std::move(delegate));
registry_->InitProtocolSettings();
}
Expand Down
Expand Up @@ -13,6 +13,8 @@
#include "chrome/browser/profiles/incognito_helpers.h"
#include "components/custom_handlers/protocol_handler_registry.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/user_prefs/user_prefs.h"
#include "content/public/browser/browser_context.h"

// static
ProtocolHandlerRegistryFactory* ProtocolHandlerRegistryFactory::GetInstance() {
Expand Down Expand Up @@ -58,9 +60,11 @@ bool ProtocolHandlerRegistryFactory::ServiceIsNULLWhileTesting() const {

KeyedService* ProtocolHandlerRegistryFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
PrefService* prefs = user_prefs::UserPrefs::Get(context);
DCHECK(prefs);
custom_handlers::ProtocolHandlerRegistry* registry =
new custom_handlers::ProtocolHandlerRegistry(
context, std::make_unique<ChromeProtocolHandlerRegistryDelegate>());
prefs, std::make_unique<ChromeProtocolHandlerRegistryDelegate>());

#if BUILDFLAG(IS_CHROMEOS_ASH)
// If installing defaults, they must be installed prior calling
Expand Down
Expand Up @@ -383,7 +383,7 @@ class RenderViewContextMenuExtensionsTest : public RenderViewContextMenuTest {
RenderViewContextMenuTest::SetUp();
// TestingProfile does not provide a protocol registry.
registry_ = std::make_unique<custom_handlers::ProtocolHandlerRegistry>(
profile(), nullptr);
profile()->GetPrefs(), nullptr);
}

void TearDown() override {
Expand Down Expand Up @@ -447,7 +447,7 @@ class RenderViewContextMenuPrefsTest : public ChromeRenderViewHostTestHarness {
void SetUp() override {
ChromeRenderViewHostTestHarness::SetUp();
registry_ = std::make_unique<custom_handlers::ProtocolHandlerRegistry>(
profile(), nullptr);
profile()->GetPrefs(), nullptr);

TemplateURLServiceFactory::GetInstance()->SetTestingFactoryAndUse(
profile(),
Expand Down
Expand Up @@ -1045,7 +1045,7 @@ TEST_F(ContentSettingBubbleModelTest, RegisterProtocolHandler) {

TEST_F(ContentSettingBubbleModelTest, RPHAllow) {
custom_handlers::ProtocolHandlerRegistry registry(
profile(),
profile()->GetPrefs(),
std::make_unique<custom_handlers::TestProtocolHandlerRegistryDelegate>());
registry.InitProtocolSettings();

Expand Down Expand Up @@ -1113,7 +1113,7 @@ TEST_F(ContentSettingBubbleModelTest, RPHAllow) {

TEST_F(ContentSettingBubbleModelTest, RPHDefaultDone) {
custom_handlers::ProtocolHandlerRegistry registry(
profile(),
profile()->GetPrefs(),
std::make_unique<custom_handlers::TestProtocolHandlerRegistryDelegate>());
registry.InitProtocolSettings();

Expand Down
30 changes: 18 additions & 12 deletions components/custom_handlers/protocol_handler_registry.cc
Expand Up @@ -63,9 +63,9 @@ GURL TranslateUrl(
// ProtocolHandlerRegistry -----------------------------------------------------

ProtocolHandlerRegistry::ProtocolHandlerRegistry(
content::BrowserContext* context,
PrefService* prefs,
std::unique_ptr<Delegate> delegate)
: context_(context),
: prefs_(prefs),
delegate_(std::move(delegate)),
enabled_(true),
is_loading_(false),
Expand Down Expand Up @@ -181,9 +181,13 @@ void ProtocolHandlerRegistry::InitProtocolSettings() {
is_loaded_ = true;
is_loading_ = true;

PrefService* prefs = user_prefs::UserPrefs::Get(context_);
if (prefs->HasPrefPath(prefs::kCustomHandlersEnabled)) {
if (prefs->GetBoolean(prefs::kCustomHandlersEnabled)) {
if (!prefs_) {
is_loading_ = false;
return;
}

if (prefs_->HasPrefPath(prefs::kCustomHandlersEnabled)) {
if (prefs_->GetBoolean(prefs::kCustomHandlersEnabled)) {
Enable();
} else {
Disable();
Expand Down Expand Up @@ -500,13 +504,16 @@ void ProtocolHandlerRegistry::Save() {
if (is_loading_) {
return;
}

if (!prefs_)
return;

base::Value registered_protocol_handlers(EncodeRegisteredHandlers());
base::Value ignored_protocol_handlers(EncodeIgnoredHandlers());
PrefService* prefs = user_prefs::UserPrefs::Get(context_);

prefs->Set(prefs::kRegisteredProtocolHandlers, registered_protocol_handlers);
prefs->Set(prefs::kIgnoredProtocolHandlers, ignored_protocol_handlers);
prefs->SetBoolean(prefs::kCustomHandlersEnabled, enabled_);
prefs_->Set(prefs::kRegisteredProtocolHandlers, registered_protocol_handlers);
prefs_->Set(prefs::kIgnoredProtocolHandlers, ignored_protocol_handlers);
prefs_->SetBoolean(prefs::kCustomHandlersEnabled, enabled_);
}

const ProtocolHandlerRegistry::ProtocolHandlerList*
Expand Down Expand Up @@ -607,12 +614,11 @@ std::vector<const base::Value::Dict*>
ProtocolHandlerRegistry::GetHandlersFromPref(const char* pref_name) const {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
std::vector<const base::Value::Dict*> result;
PrefService* prefs = user_prefs::UserPrefs::Get(context_);
if (!prefs->HasPrefPath(pref_name)) {
if (!prefs_ || !prefs_->HasPrefPath(pref_name)) {
return result;
}

const base::Value* handlers = prefs->GetList(pref_name);
const base::Value* handlers = prefs_->GetList(pref_name);
if (handlers) {
for (const auto& list_item : handlers->GetList()) {
if (const base::Value::Dict* encoded_handler = list_item.GetIfDict()) {
Expand Down
11 changes: 7 additions & 4 deletions components/custom_handlers/protocol_handler_registry.h
Expand Up @@ -17,13 +17,15 @@
#include "base/time/time.h"
#include "base/values.h"
#include "components/keyed_service/core/keyed_service.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"

namespace user_prefs {
class PrefRegistrySyncable;
}

class PrefService;
class GURL;

using DefaultClientCallback = base::OnceCallback<void(bool)>;

namespace custom_handlers {
Expand Down Expand Up @@ -74,7 +76,7 @@ class ProtocolHandlerRegistry : public KeyedService {
};

// Creates a new instance.
ProtocolHandlerRegistry(content::BrowserContext* context,
ProtocolHandlerRegistry(PrefService* prefs,
std::unique_ptr<Delegate> delegate);

ProtocolHandlerRegistry(const ProtocolHandlerRegistry&) = delete;
Expand Down Expand Up @@ -341,8 +343,9 @@ class ProtocolHandlerRegistry : public KeyedService {
// Protocol handlers that are the defaults for a given protocol.
ProtocolHandlerMap default_handlers_;

// The browser context that owns this ProtocolHandlerRegistry.
raw_ptr<content::BrowserContext> context_;
// The PrefService to store the registered handlers in the user profile (it
// may be null for testing)
raw_ptr<PrefService> prefs_ = nullptr;

// The Delegate that registers / deregisters external handlers on our behalf.
std::unique_ptr<Delegate> delegate_;
Expand Down
Expand Up @@ -192,8 +192,8 @@ class ProtocolHandlerRegistryTest : public testing::Test {
DCHECK(browser_context_);
auto delegate = std::make_unique<TestProtocolHandlerRegistryDelegate>();
delegate_ = delegate.get();
registry_ = std::make_unique<ProtocolHandlerRegistry>(
browser_context_.get(), std::move(delegate));
registry_ = std::make_unique<ProtocolHandlerRegistry>(GetPrefs(),
std::move(delegate));
if (initialize)
registry_->InitProtocolSettings();
}
Expand Down

0 comments on commit e7db8a1

Please sign in to comment.