Don't fire the chrome.idle.onStateChanged event when a listener is re-added

after the idle state has changed.

BUG=366580
R=courage@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267043 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/api/idle/idle_api_unittest.cc

@@ -550,4 +550,30 @@ TEST_F(IdleTest, UnloadWhileListening) {
       content::Details<UnloadedExtensionInfo>(&details));
 }
 
+// Verifies that re-adding a listener after a state change doesn't immediately
+// fire a change event. Regression test for http://crbug.com/366580.
+TEST_F(IdleTest, ReAddListener) {
+  idle_provider_->set_locked(false);
+
+  {
+    // Fire idle event.
+    ScopedListen listen(idle_manager_, "test");
+    idle_provider_->set_idle_time(60);
+    EXPECT_CALL(*event_delegate_, OnStateChanged("test", IDLE_STATE_IDLE));
+    idle_manager_->UpdateIdleState();
+    testing::Mock::VerifyAndClearExpectations(event_delegate_);
+  }
+
+  // Trigger active.
+  idle_provider_->set_idle_time(0);
+  idle_manager_->UpdateIdleState();
+
+  {
+    // Nothing should have fired, the listener wasn't added until afterward.
+    ScopedListen listen(idle_manager_, "test");
+    idle_manager_->UpdateIdleState();
+    testing::Mock::VerifyAndClearExpectations(event_delegate_);
+  }
+}
+
 }  // namespace extensions

chrome/browser/extensions/api/idle/idle_manager.cc

@@ -171,6 +171,9 @@ void IdleManager::OnListenerRemoved(const EventListenerInfo& details) {
   MonitorMap::iterator it = monitors_.find(details.extension_id);
   if (it != monitors_.end()) {
     DCHECK_GT(it->second.listeners, 0);
+    // Note: Deliberately leave the listener count as 0 rather than erase()ing
+    // this record so that the threshold doesn't get reset when all listeners
+    // are removed.
     --it->second.listeners;
   }
 }
@@ -259,24 +262,18 @@ void IdleManager::UpdateIdleStateCallback(int idle_time) {
 
   for (MonitorMap::iterator it = monitors_.begin();
        it != monitors_.end(); ++it) {
-    if (it->second.listeners < 1)
-      continue;
-
-    ++listener_count;
-
-    IdleState new_state = IdleTimeToIdleState(locked,
-                                              idle_time,
-                                              it->second.threshold);
-
-    if (new_state != it->second.last_state) {
-      it->second.last_state = new_state;
+    IdleMonitor& monitor = it->second;
+    IdleState new_state =
+        IdleTimeToIdleState(locked, idle_time, monitor.threshold);
+    // TODO(kalman): Use EventRouter::HasListeners for these sorts of checks.
+    if (monitor.listeners > 0 && monitor.last_state != new_state)
       event_delegate_->OnStateChanged(it->first, new_state);
-    }
+    monitor.last_state = new_state;
+    listener_count += monitor.listeners;
   }
 
-  if (listener_count == 0) {
+  if (listener_count == 0)
     StopPolling();
-  }
 }
 
 }  // namespace extensions

chrome/browser/extensions/api/idle/idle_manager.h

@@ -103,6 +103,7 @@ class IdleManager : public content::NotificationObserver,
   FRIEND_TEST_ALL_PREFIXES(IdleTest, LockedToActive);
   FRIEND_TEST_ALL_PREFIXES(IdleTest, LockedToIdle);
   FRIEND_TEST_ALL_PREFIXES(IdleTest, MultipleExtensions);
+  FRIEND_TEST_ALL_PREFIXES(IdleTest, ReAddListener);
   FRIEND_TEST_ALL_PREFIXES(IdleTest, SetDetectionInterval);
   FRIEND_TEST_ALL_PREFIXES(IdleTest, SetDetectionIntervalBeforeListener);
   FRIEND_TEST_ALL_PREFIXES(IdleTest, SetDetectionIntervalMaximum);