Skip to content

Commit

Permalink
Bug 1863035 - Remove unused watching functions in nsIWindowsRegKey. r…
Browse files Browse the repository at this point in the history
…=win-reviewers,rkraesig

They were added in bug 292981 but this interface is not great,
as it is poll-based. Instead, we want a registry key change
notification of sorts.

Removing this simplifies upcoming patches.

Differential Revision: https://phabricator.services.mozilla.com/D192755
  • Loading branch information
emilio committed Nov 3, 2023
1 parent f137cf6 commit 0a0d1b0
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 132 deletions.
29 changes: 0 additions & 29 deletions xpcom/ds/nsIWindowsRegKey.idl
Original file line number Diff line number Diff line change
Expand Up @@ -304,33 +304,4 @@ interface nsIWindowsRegKey : nsISupports
* The data for the value to modify.
*/
void writeBinaryValue(in AString name, in ACString data);

/**
* This method starts watching the key to see if any of its values have
* changed. The key must have been opened with mode including ACCESS_NOTIFY.
* If recurse is true, then this key and any of its descendant keys are
* watched. Otherwise, only this key is watched.
*
* @param recurse
* Indicates whether or not to also watch child keys.
*/
void startWatching(in boolean recurse);

/**
* This method stops any watching of the key initiated by a call to
* startWatching. This method does nothing if the key is not being watched.
*/
void stopWatching();

/**
* This method returns true if the key is being watched for changes (i.e.,
* if startWatching() was called).
*/
boolean isWatching();

/**
* This method returns true if the key has changed and false otherwise.
* This method will always return false if startWatching was not called.
*/
boolean hasChanged();
};
73 changes: 3 additions & 70 deletions xpcom/ds/nsWindowsRegKey.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,12 @@ class nsWindowsRegKey final : public nsIWindowsRegKey {
NS_DECL_ISUPPORTS
NS_DECL_NSIWINDOWSREGKEY

nsWindowsRegKey()
: mKey(nullptr), mWatchEvent(nullptr), mWatchRecursive(FALSE) {}
nsWindowsRegKey() : mKey(nullptr) {}

private:
~nsWindowsRegKey() { Close(); }

HKEY mKey;
HANDLE mWatchEvent;
BOOL mWatchRecursive;
};

NS_IMPL_ISUPPORTS(nsWindowsRegKey, nsIWindowsRegKey)
Expand All @@ -45,17 +42,12 @@ nsWindowsRegKey::GetKey(HKEY* aKey) {

NS_IMETHODIMP
nsWindowsRegKey::SetKey(HKEY aKey) {
// We do not close the older aKey!
StopWatching();

mKey = aKey;
return NS_OK;
}

NS_IMETHODIMP
nsWindowsRegKey::Close() {
StopWatching();

if (mKey) {
RegCloseKey(mKey);
mKey = nullptr;
Expand Down Expand Up @@ -304,7 +296,7 @@ nsWindowsRegKey::ReadStringValue(const nsAString& aName, nsAString& aResult) {
return NS_ERROR_OUT_OF_MEMORY;
}

auto begin = aResult.BeginWriting();
auto* begin = aResult.BeginWriting();

rv = RegQueryValueExW(mKey, flatName.get(), 0, &type, (LPBYTE)begin, &size);

Expand Down Expand Up @@ -390,7 +382,7 @@ nsWindowsRegKey::ReadBinaryValue(const nsAString& aName, nsACString& aResult) {
return NS_ERROR_OUT_OF_MEMORY;
}

auto begin = aResult.BeginWriting();
auto* begin = aResult.BeginWriting();

rv = RegQueryValueExW(mKey, PromiseFlatString(aName).get(), 0, nullptr,
(LPBYTE)begin, &size);
Expand Down Expand Up @@ -448,65 +440,6 @@ nsWindowsRegKey::WriteBinaryValue(const nsAString& aName,
return (rv == ERROR_SUCCESS) ? NS_OK : NS_ERROR_FAILURE;
}

NS_IMETHODIMP
nsWindowsRegKey::StartWatching(bool aRecurse) {
if (NS_WARN_IF(!mKey)) {
return NS_ERROR_NOT_INITIALIZED;
}

if (mWatchEvent) {
return NS_OK;
}

mWatchEvent = CreateEventW(nullptr, TRUE, FALSE, nullptr);
if (!mWatchEvent) {
return NS_ERROR_OUT_OF_MEMORY;
}

DWORD filter = REG_NOTIFY_CHANGE_NAME | REG_NOTIFY_CHANGE_ATTRIBUTES |
REG_NOTIFY_CHANGE_LAST_SET | REG_NOTIFY_CHANGE_SECURITY;

LONG rv = RegNotifyChangeKeyValue(mKey, aRecurse, filter, mWatchEvent, TRUE);
if (rv != ERROR_SUCCESS) {
StopWatching();
// On older versions of Windows, this call is not implemented, so simply
// return NS_OK in those cases and pretend that the watching is happening.
return (rv == ERROR_CALL_NOT_IMPLEMENTED) ? NS_OK : NS_ERROR_FAILURE;
}

mWatchRecursive = aRecurse;
return NS_OK;
}

NS_IMETHODIMP
nsWindowsRegKey::StopWatching() {
if (mWatchEvent) {
CloseHandle(mWatchEvent);
mWatchEvent = nullptr;
}
return NS_OK;
}

NS_IMETHODIMP
nsWindowsRegKey::HasChanged(bool* aResult) {
if (mWatchEvent && WaitForSingleObject(mWatchEvent, 0) == WAIT_OBJECT_0) {
// An event only gets signaled once, then it's done, so we have to set up
// another event to watch.
StopWatching();
StartWatching(mWatchRecursive);
*aResult = true;
} else {
*aResult = false;
}
return NS_OK;
}

NS_IMETHODIMP
nsWindowsRegKey::IsWatching(bool* aResult) {
*aResult = (mWatchEvent != nullptr);
return NS_OK;
}

//-----------------------------------------------------------------------------

void NS_NewWindowsRegKey(nsIWindowsRegKey** aResult) {
Expand Down
33 changes: 0 additions & 33 deletions xpcom/tests/unit/test_windows_registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ function run_test() {
//* check that creating/enumerating/deleting child keys works
test_childkey_functions(testKey);

test_watching_functions(testKey);

//* clean up
cleanup_test_run(testKey, keyName);
}
Expand Down Expand Up @@ -174,37 +172,6 @@ function test_childkey_functions(testKey) {
strictEqual(testKey.hasChild(TESTDATA_CHILD_KEY), false);
}

function test_watching_functions(testKey) {
strictEqual(testKey.isWatching(), false);
strictEqual(testKey.hasChanged(), false);

testKey.startWatching(true);
strictEqual(testKey.isWatching(), true);

testKey.stopWatching();
strictEqual(testKey.isWatching(), false);

// Create a child key, and update a value
let childKey = testKey.createChild(
TESTDATA_CHILD_KEY,
nsIWindowsRegKey.ACCESS_ALL
);
childKey.writeIntValue(TESTDATA_INTNAME, TESTDATA_INTVALUE);

// Start a recursive watch, and update the child's value
testKey.startWatching(true);
strictEqual(testKey.isWatching(), true);

childKey.writeIntValue(TESTDATA_INTNAME, 0);
strictEqual(testKey.hasChanged(), true);
testKey.stopWatching();
strictEqual(testKey.isWatching(), false);

childKey.removeValue(TESTDATA_INTNAME);
childKey.close();
testKey.removeChild(TESTDATA_CHILD_KEY);
}

function cleanup_test_run(testKey, keyName) {
info("Cleaning up test.");

Expand Down

0 comments on commit 0a0d1b0

Please sign in to comment.