Commit 6f315b85 authored by rvargas's avatar rvargas Committed by Commit bot

Remove raw handles from base::win::RegKey

BUG=419210, 423634
R=cpu@chromium.org, eroman@chromium.org, sky@chromium.org

Committed: https://crrev.com/1aa0fa75b65f403e08ae0f3f2fcb053c02cd9ef2
Cr-Commit-Position: refs/heads/master@{#299737}

Review URL: https://codereview.chromium.org/632833002

Cr-Commit-Position: refs/heads/master@{#300036}
parent fde47145
......@@ -34,23 +34,67 @@ const REGSAM kWow64AccessMask = KEY_WOW64_32KEY | KEY_WOW64_64KEY;
} // namespace
// Watches for modifications to a key.
class RegKey::Watcher : public ObjectWatcher::Delegate {
public:
explicit Watcher(RegKey* owner) : owner_(owner) {}
~Watcher() {}
bool StartWatching(HKEY key, const ChangeCallback& callback);
// Implementation of ObjectWatcher::Delegate.
void OnObjectSignaled(HANDLE object) override {
DCHECK(watch_event_.IsValid() && watch_event_.Get() == object);
ChangeCallback callback = callback_;
callback_.Reset();
callback.Run();
}
private:
RegKey* owner_;
ScopedHandle watch_event_;
ObjectWatcher object_watcher_;
ChangeCallback callback_;
DISALLOW_COPY_AND_ASSIGN(Watcher);
};
bool RegKey::Watcher::StartWatching(HKEY key, const ChangeCallback& callback) {
DCHECK(key);
DCHECK(callback_.is_null());
if (!watch_event_.IsValid())
watch_event_.Set(CreateEvent(NULL, TRUE, FALSE, NULL));
if (!watch_event_.IsValid())
return false;
DWORD filter = REG_NOTIFY_CHANGE_NAME |
REG_NOTIFY_CHANGE_ATTRIBUTES |
REG_NOTIFY_CHANGE_LAST_SET |
REG_NOTIFY_CHANGE_SECURITY;
// Watch the registry key for a change of value.
LONG result = RegNotifyChangeKeyValue(key, TRUE, filter, watch_event_.Get(),
TRUE);
if (result != ERROR_SUCCESS) {
watch_event_.Close();
return false;
}
callback_ = callback;
return object_watcher_.StartWatching(watch_event_.Get(), this);
}
// RegKey ----------------------------------------------------------------------
RegKey::RegKey()
: key_(NULL),
watch_event_(0),
wow64access_(0) {
RegKey::RegKey() : key_(NULL), wow64access_(0) {
}
RegKey::RegKey(HKEY key)
: key_(key),
watch_event_(0),
wow64access_(0) {
RegKey::RegKey(HKEY key) : key_(key), wow64access_(0) {
}
RegKey::RegKey(HKEY rootkey, const wchar_t* subkey, REGSAM access)
: key_(NULL),
watch_event_(0),
wow64access_(0) {
if (rootkey) {
if (access & (KEY_SET_VALUE | KEY_CREATE_SUB_KEY | KEY_CREATE_LINK))
......@@ -150,7 +194,6 @@ LONG RegKey::OpenKey(const wchar_t* relative_key_name, REGSAM access) {
}
void RegKey::Close() {
StopWatching();
if (key_) {
::RegCloseKey(key_);
key_ = NULL;
......@@ -168,7 +211,6 @@ void RegKey::Set(HKEY key) {
HKEY RegKey::Take() {
DCHECK(wow64access_ == 0);
StopWatching();
HKEY key = key_;
key_ = NULL;
return key;
......@@ -367,44 +409,14 @@ LONG RegKey::WriteValue(const wchar_t* name,
return result;
}
LONG RegKey::StartWatching() {
DCHECK(key_);
if (!watch_event_)
watch_event_ = CreateEvent(NULL, TRUE, FALSE, NULL);
DWORD filter = REG_NOTIFY_CHANGE_NAME |
REG_NOTIFY_CHANGE_ATTRIBUTES |
REG_NOTIFY_CHANGE_LAST_SET |
REG_NOTIFY_CHANGE_SECURITY;
// Watch the registry key for a change of value.
LONG result = RegNotifyChangeKeyValue(key_, TRUE, filter, watch_event_, TRUE);
if (result != ERROR_SUCCESS) {
CloseHandle(watch_event_);
watch_event_ = 0;
}
bool RegKey::StartWatching(const ChangeCallback& callback) {
if (!key_watcher_)
key_watcher_.reset(new Watcher(this));
return result;
}
bool RegKey::HasChanged() {
if (watch_event_) {
if (WaitForSingleObject(watch_event_, 0) == WAIT_OBJECT_0) {
StartWatching();
return true;
}
}
return false;
}
if (!key_watcher_.get()->StartWatching(key_, callback))
return false;
LONG RegKey::StopWatching() {
LONG result = ERROR_INVALID_HANDLE;
if (watch_event_) {
CloseHandle(watch_event_);
watch_event_ = 0;
result = ERROR_SUCCESS;
}
return result;
return true;
}
// static
......
......@@ -12,6 +12,8 @@
#include "base/base_export.h"
#include "base/basictypes.h"
#include "base/stl_util.h"
#include "base/win/object_watcher.h"
#include "base/win/scoped_handle.h"
namespace base {
namespace win {
......@@ -25,6 +27,9 @@ namespace win {
// are not touched in case of failure.
class BASE_EXPORT RegKey {
public:
// Called from the MessageLoop when the key changes.
typedef base::Callback<void()> ChangeCallback;
RegKey();
explicit RegKey(HKEY key);
RegKey(HKEY rootkey, const wchar_t* subkey, REGSAM access);
......@@ -120,22 +125,16 @@ class BASE_EXPORT RegKey {
// Starts watching the key to see if any of its values have changed.
// The key must have been opened with the KEY_NOTIFY access privilege.
LONG StartWatching();
// If StartWatching hasn't been called, always returns false.
// Otherwise, returns true if anything under the key has changed.
// This can't be const because the |watch_event_| may be refreshed.
bool HasChanged();
// Will automatically be called by destructor if not manually called
// beforehand. Returns true if it was watching, false otherwise.
LONG StopWatching();
// Returns true on success.
// To stop watching, delete this RegKey object. To continue watching the
// object after the callback is invoked, call StartWatching again.
bool StartWatching(const ChangeCallback& callback);
inline bool IsWatching() const { return watch_event_ != 0; }
HANDLE watch_event() const { return watch_event_; }
HKEY Handle() const { return key_; }
private:
class Watcher;
// Calls RegDeleteKeyEx on supported platforms, alternatively falls back to
// RegDeleteKey.
static LONG RegDeleteKeyExWrapper(HKEY hKey,
......@@ -147,9 +146,10 @@ class BASE_EXPORT RegKey {
static LONG RegDelRecurse(HKEY root_key,
const std::wstring& name,
REGSAM access);
HKEY key_; // The registry key being iterated.
HANDLE watch_event_;
REGSAM wow64access_;
scoped_ptr<Watcher> key_watcher_;
DISALLOW_COPY_AND_ASSIGN(RegKey);
};
......
......@@ -7,7 +7,10 @@
#include <cstring>
#include <vector>
#include "base/bind.h"
#include "base/compiler_specific.h"
#include "base/message_loop/message_loop.h"
#include "base/run_loop.h"
#include "base/stl_util.h"
#include "base/win/windows_version.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -349,6 +352,68 @@ TEST_F(RegistryTest, OpenSubKey) {
ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(L"foo"));
}
class TestChangeDelegate {
public:
TestChangeDelegate() : called_(false) {}
~TestChangeDelegate() {}
void OnKeyChanged() {
MessageLoop::current()->QuitWhenIdle();
called_ = true;
}
bool WasCalled() {
bool was_called = called_;
called_ = false;
return was_called;
}
private:
bool called_;
};
TEST_F(RegistryTest, ChangeCallback) {
RegKey key;
TestChangeDelegate delegate;
MessageLoop message_loop;
std::wstring foo_key(kRootKey);
foo_key += L"\\Foo";
ASSERT_EQ(ERROR_SUCCESS, key.Create(HKEY_CURRENT_USER, foo_key.c_str(),
KEY_READ));
ASSERT_TRUE(key.StartWatching(Bind(&TestChangeDelegate::OnKeyChanged,
Unretained(&delegate))));
EXPECT_FALSE(delegate.WasCalled());
// Make some change.
RegKey key2;
ASSERT_EQ(ERROR_SUCCESS, key2.Open(HKEY_CURRENT_USER, foo_key.c_str(),
KEY_READ | KEY_SET_VALUE));
ASSERT_TRUE(key2.Valid());
EXPECT_EQ(ERROR_SUCCESS, key2.WriteValue(L"name", L"data"));
// Allow delivery of the notification.
EXPECT_FALSE(delegate.WasCalled());
base::RunLoop().Run();
ASSERT_TRUE(delegate.WasCalled());
EXPECT_FALSE(delegate.WasCalled());
ASSERT_TRUE(key.StartWatching(Bind(&TestChangeDelegate::OnKeyChanged,
Unretained(&delegate))));
// Change something else.
EXPECT_EQ(ERROR_SUCCESS, key2.WriteValue(L"name2", L"data2"));
base::RunLoop().Run();
ASSERT_TRUE(delegate.WasCalled());
ASSERT_TRUE(key.StartWatching(Bind(&TestChangeDelegate::OnKeyChanged,
Unretained(&delegate))));
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(delegate.WasCalled());
}
} // namespace
} // namespace win
......
......@@ -157,15 +157,6 @@ PluginServiceImpl::PluginServiceImpl()
}
PluginServiceImpl::~PluginServiceImpl() {
#if defined(OS_WIN)
// Release the events since they're owned by RegKey, not WaitableEvent.
hkcu_watcher_.StopWatching();
hklm_watcher_.StopWatching();
if (hkcu_event_)
hkcu_event_->Release();
if (hklm_event_)
hklm_event_->Release();
#endif
// Make sure no plugin channel requests have been leaked.
DCHECK(pending_plugin_clients_.empty());
}
......@@ -200,24 +191,18 @@ void PluginServiceImpl::StartWatchingPlugins() {
if (hkcu_key_.Create(HKEY_CURRENT_USER,
kRegistryMozillaPlugins,
KEY_NOTIFY) == ERROR_SUCCESS) {
if (hkcu_key_.StartWatching() == ERROR_SUCCESS) {
hkcu_event_.reset(new base::WaitableEvent(hkcu_key_.watch_event()));
base::WaitableEventWatcher::EventCallback callback =
base::Bind(&PluginServiceImpl::OnWaitableEventSignaled,
base::Unretained(this));
hkcu_watcher_.StartWatching(hkcu_event_.get(), callback);
}
base::win::RegKey::ChangeCallback callback =
base::Bind(&PluginServiceImpl::OnKeyChanged, base::Unretained(this),
base::Unretained(&hkcu_key_));
hkcu_key_.StartWatching(callback);
}
if (hklm_key_.Create(HKEY_LOCAL_MACHINE,
kRegistryMozillaPlugins,
KEY_NOTIFY) == ERROR_SUCCESS) {
if (hklm_key_.StartWatching() == ERROR_SUCCESS) {
hklm_event_.reset(new base::WaitableEvent(hklm_key_.watch_event()));
base::WaitableEventWatcher::EventCallback callback =
base::Bind(&PluginServiceImpl::OnWaitableEventSignaled,
base::Unretained(this));
hklm_watcher_.StartWatching(hklm_event_.get(), callback);
}
base::win::RegKey::ChangeCallback callback =
base::Bind(&PluginServiceImpl::OnKeyChanged, base::Unretained(this),
base::Unretained(&hkcu_key_));
hklm_key_.StartWatching(callback);
}
#endif
#if defined(OS_POSIX) && !defined(OS_OPENBSD) && !defined(OS_ANDROID)
......@@ -642,22 +627,16 @@ void PluginServiceImpl::GetPluginsOnIOThread(
}
#endif
void PluginServiceImpl::OnWaitableEventSignaled(
base::WaitableEvent* waitable_event) {
#if defined(OS_WIN)
if (waitable_event == hkcu_event_) {
hkcu_key_.StartWatching();
} else {
hklm_key_.StartWatching();
}
void PluginServiceImpl::OnKeyChanged(base::win::RegKey* key) {
key->StartWatching(base::Bind(&PluginServiceImpl::OnKeyChanged,
base::Unretained(this),
base::Unretained(&hkcu_key_)));
PluginList::Singleton()->RefreshPlugins();
PurgePluginListCache(NULL, false);
#else
// This event should only get signaled on a Windows machine.
NOTREACHED();
#endif // defined(OS_WIN)
}
#endif // defined(OS_WIN)
void PluginServiceImpl::RegisterPepperPlugins() {
ComputePepperPluginList(&ppapi_plugins_);
......
......@@ -166,7 +166,9 @@ class CONTENT_EXPORT PluginServiceImpl
PluginServiceImpl();
virtual ~PluginServiceImpl();
void OnWaitableEventSignaled(base::WaitableEvent* waitable_event);
#if defined(OS_WIN)
void OnKeyChanged(base::win::RegKey* key);
#endif
// Returns the plugin process host corresponding to the plugin process that
// has been started by this service. Returns NULL if no process has been
......@@ -224,10 +226,6 @@ class CONTENT_EXPORT PluginServiceImpl
// Registry keys for getting notifications when new plugins are installed.
base::win::RegKey hkcu_key_;
base::win::RegKey hklm_key_;
scoped_ptr<base::WaitableEvent> hkcu_event_;
scoped_ptr<base::WaitableEvent> hklm_event_;
base::WaitableEventWatcher hkcu_watcher_;
base::WaitableEventWatcher hklm_watcher_;
#endif
#if defined(OS_POSIX) && !defined(OS_OPENBSD) && !defined(OS_ANDROID)
......
......@@ -23,8 +23,8 @@
#include "base/threading/non_thread_safe.h"
#include "base/threading/thread_restrictions.h"
#include "base/time/time.h"
#include "base/win/object_watcher.h"
#include "base/win/registry.h"
#include "base/win/scoped_handle.h"
#include "base/win/windows_version.h"
#include "net/base/net_util.h"
#include "net/base/network_change_notifier.h"
......@@ -292,8 +292,7 @@ HostsParseWinResult AddLocalhostEntries(DnsHosts* hosts) {
}
// Watches a single registry key for changes.
class RegistryWatcher : public base::win::ObjectWatcher::Delegate,
public base::NonThreadSafe {
class RegistryWatcher : public base::NonThreadSafe {
public:
typedef base::Callback<void(bool succeeded)> CallbackType;
RegistryWatcher() {}
......@@ -305,35 +304,31 @@ class RegistryWatcher : public base::win::ObjectWatcher::Delegate,
callback_ = callback;
if (key_.Open(HKEY_LOCAL_MACHINE, key, KEY_NOTIFY) != ERROR_SUCCESS)
return false;
if (key_.StartWatching() != ERROR_SUCCESS)
return false;
if (!watcher_.StartWatching(key_.watch_event(), this))
return false;
return true;
return key_.StartWatching(base::Bind(&RegistryWatcher::OnObjectSignaled,
base::Unretained(this)));
}
virtual void OnObjectSignaled(HANDLE object) override {
void OnObjectSignaled() {
// TODO(vadimt): Remove ScopedProfile below once crbug.com/418183 is fixed.
tracked_objects::ScopedProfile tracking_profile(
FROM_HERE_WITH_EXPLICIT_FUNCTION(
"RegistryWatcher_OnObjectSignaled"));
DCHECK(CalledOnValidThread());
bool succeeded = (key_.StartWatching() == ERROR_SUCCESS) &&
watcher_.StartWatching(key_.watch_event(), this);
if (!succeeded && key_.Valid()) {
watcher_.StopWatching();
key_.StopWatching();
DCHECK(!callback_.is_null());
if (key_.StartWatching(base::Bind(&RegistryWatcher::OnObjectSignaled,
base::Unretained(this)))) {
callback_.Run(true);
} else {
key_.Close();
callback_.Run(false);
}
if (!callback_.is_null())
callback_.Run(succeeded);
}
private:
CallbackType callback_;
base::win::RegKey key_;
base::win::ObjectWatcher watcher_;
DISALLOW_COPY_AND_ASSIGN(RegistryWatcher);
};
......@@ -740,9 +735,8 @@ bool DnsConfigServiceWin::StartWatching() {
void DnsConfigServiceWin::OnConfigChanged(bool succeeded) {
InvalidateConfig();
if (succeeded) {
config_reader_->WorkNow();
} else {
config_reader_->WorkNow();
if (!succeeded) {
LOG(ERROR) << "DNS config watch failed.";
set_watch_failed(true);
UMA_HISTOGRAM_ENUMERATION("AsyncDNS.WatchStatus",
......
......@@ -7,6 +7,8 @@
#include <windows.h>
#include <winhttp.h>
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/logging.h"
#include "base/memory/scoped_ptr.h"
#include "base/profiler/scoped_profile.h"
......@@ -16,6 +18,7 @@
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_restrictions.h"
#include "base/win/registry.h"
#include "base/win/scoped_handle.h"
#include "net/base/net_errors.h"
#include "net/proxy/proxy_config.h"
......@@ -38,36 +41,6 @@ void FreeIEConfig(WINHTTP_CURRENT_USER_IE_PROXY_CONFIG* ie_config) {
} // namespace
// RegKey and ObjectWatcher pair.
class ProxyConfigServiceWin::KeyEntry {
public:
bool StartWatching(base::win::ObjectWatcher::Delegate* delegate) {
// Try to create a watch event for the registry key (which watches the
// sibling tree as well).
if (key_.StartWatching() != ERROR_SUCCESS)
return false;
// Now setup an ObjectWatcher for this event, so we get OnObjectSignaled()
// invoked on this message loop once it is signalled.
if (!watcher_.StartWatching(key_.watch_event(), delegate))
return false;
return true;
}
bool CreateRegKey(HKEY rootkey, const wchar_t* subkey) {
return key_.Create(rootkey, subkey, KEY_NOTIFY) == ERROR_SUCCESS;
}
HANDLE watch_event() const {
return key_.watch_event();
}
private:
base::win::RegKey key_;
base::win::ObjectWatcher watcher_;
};
ProxyConfigServiceWin::ProxyConfigServiceWin()
: PollingProxyConfigService(
base::TimeDelta::FromSeconds(kPollIntervalSec),
......@@ -125,35 +98,38 @@ void ProxyConfigServiceWin::StartWatchingRegistryForChanges() {
bool ProxyConfigServiceWin::AddKeyToWatchList(HKEY rootkey,
const wchar_t* subkey) {
scoped_ptr<KeyEntry> entry(new KeyEntry);
if (!entry->CreateRegKey(rootkey, subkey))
scoped_ptr<base::win::RegKey> key(new base::win::RegKey);
if (key->Create(rootkey, subkey, KEY_NOTIFY) != ERROR_SUCCESS)
return false;
if (!entry->StartWatching(this))
if (!key->StartWatching(base::Bind(&ProxyConfigServiceWin::OnObjectSignaled,
base::Unretained(this),
base::Unretained(key.get())))) {
return false;
}
keys_to_watch_.push_back(entry.release());
keys_to_watch_.push_back(key.release());
return true;
}
void ProxyConfigServiceWin::OnObjectSignaled(HANDLE object) {
void ProxyConfigServiceWin::OnObjectSignaled(base::win::RegKey* key) {
// TODO(vadimt): Remove ScopedProfile below once crbug.com/418183 is fixed.
tracked_objects::ScopedProfile tracking_profile(
FROM_HERE_WITH_EXPLICIT_FUNCTION(
"ProxyConfigServiceWin_OnObjectSignaled"));
// Figure out which registry key signalled this change.
KeyEntryList::iterator it;
for (it = keys_to_watch_.begin(); it != keys_to_watch_.end(); ++it) {
if ((*it)->watch_event() == object)
break;
}
RegKeyList::iterator it =
std::find(keys_to_watch_.begin(), keys_to_watch_.end(), key);
DCHECK(it != keys_to_watch_.end());
// Keep watching the registry key.
if (!(*it)->StartWatching(this))
if (!key->StartWatching(base::Bind(&ProxyConfigServiceWin::OnObjectSignaled,
base::Unretained(this),
base::Unretained(key)))) {
delete *it;
keys_to_watch_.erase(it);
}
// Have the PollingProxyConfigService test for changes.
CheckForChangesNow();
......
......@@ -12,9 +12,14 @@
#include "base/compiler_specific.h"
#include "base/gtest_prod_util.h"
#include "base/win/object_watcher.h"
#include "net/proxy/polling_proxy_config_service.h"
namespace base {
namespace win {
class RegKey;
}
} // namespace base.
namespace net {
// Implementation of ProxyConfigService that retrieves the system proxy
......@@ -40,8 +45,7 @@ namespace net {
// change, or in case we got it wrong (and are not checking all possible
// registry dependencies).
class NET_EXPORT_PRIVATE ProxyConfigServiceWin
: public PollingProxyConfigService,
public base::win::ObjectWatcher::Delegate {
: public PollingProxyConfigService {
public:
ProxyConfigServiceWin();
virtual ~ProxyConfigServiceWin();
......@@ -51,19 +55,17 @@ class NET_EXPORT_PRIVATE ProxyConfigServiceWin
private:
FRIEND_TEST_ALL_PREFIXES(ProxyConfigServiceWinTest, SetFromIEConfig);
class KeyEntry;
typedef std::vector<KeyEntry*> KeyEntryList;
typedef std::vector<base::win::RegKey*> RegKeyList;
// Registers change observers on the registry keys relating to proxy settings.
void StartWatchingRegistryForChanges();
// Creates a new KeyEntry and appends it to |keys_to_watch_|. If the key
// fails to be created, it is not appended to the list and we return false.
// Creates a new key and appends it to |keys_to_watch_|. If the key fails to
// be created, it is not appended to the list and we return false.
bool AddKeyToWatchList(HKEY rootkey, const wchar_t* subkey);
// ObjectWatcher::Delegate methods:
// This is called whenever one of the registry keys we are watching change.
virtual void OnObjectSignaled(HANDLE object) override;
void OnObjectSignaled(base::win::RegKey* key);
static void GetCurrentProxyConfig(ProxyConfig* config);
......@@ -72,7 +74,7 @@ class NET_EXPORT_PRIVATE ProxyConfigServiceWin
ProxyConfig* config,
const WINHTTP_CURRENT_USER_IE_PROXY_CONFIG& ie_config);
KeyEntryList keys_to_watch_;
RegKeyList keys_to_watch_;
};
} // namespace net
......
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