[CommandUpdater] Make Command's enabled state unspecified by default
This CL fixes a discrepancy that a command can get silently enabled by a
call to AddCommandObserver(id). After this CL, a AddCommandObserver(id)
call only creates an instance for the command in the in-memory map,
without specifying its enabled state. As a result, it does _not_ change
the IsCommandEnabled(id) value.
As a side-effect, command observers are now notified in both cases after
the first call to UpdateCommandEnabled(id, state). Previously, they only
got notified if the provided |state| was false, which can be seen as a
minor bug this CL is fixing.
Bug: 1230601
Change-Id: I80ffd082af1c7559d8272bc6e7a5d922709fa860
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3039742
Commit-Queue: Jan Krcal <[email protected]>
Reviewed-by: David Roger <[email protected]>
Reviewed-by: Peter Kasting <[email protected]>
Cr-Commit-Position: refs/heads/master@{#903865}
diff --git a/chrome/browser/command_updater_impl.cc b/chrome/browser/command_updater_impl.cc
index f1014fc..80400d7 100644
--- a/chrome/browser/command_updater_impl.cc
+++ b/chrome/browser/command_updater_impl.cc
@@ -10,13 +10,12 @@
#include "base/observer_list.h"
#include "chrome/browser/command_observer.h"
#include "chrome/browser/command_updater_delegate.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
-class CommandUpdaterImpl::Command {
- public:
- bool enabled;
+struct CommandUpdaterImpl::Command {
+ // Empty optional means not specified yet and thus implicitly disabled.
+ absl::optional<bool> enabled;
base::ObserverList<CommandObserver>::Unchecked observers;
-
- Command() : enabled(true) {}
};
CommandUpdaterImpl::CommandUpdaterImpl(CommandUpdaterDelegate* delegate)
@@ -32,9 +31,9 @@
bool CommandUpdaterImpl::IsCommandEnabled(int id) const {
auto command = commands_.find(id);
- if (command == commands_.end())
+ if (command == commands_.end() || command->second->enabled == absl::nullopt)
return false;
- return command->second->enabled;
+ return *command->second->enabled;
}
bool CommandUpdaterImpl::ExecuteCommand(int id, base::TimeTicks time_stamp) {
@@ -72,7 +71,7 @@
bool CommandUpdaterImpl::UpdateCommandEnabled(int id, bool enabled) {
Command* command = GetCommand(id, true);
- if (command->enabled == enabled)
+ if (command->enabled.has_value() && *command->enabled == enabled)
return true; // Nothing to do.
command->enabled = enabled;
for (auto& observer : command->observers)
