Chrome changes to fix the idle extension API

This change list reimplements the idle extension API to fix the following issues:
. Query is now throttled only to 1 second
. Calls to calculate the idle time are async (this is needed for chromium-os)
. OnStateChanged now fires within a reasonable time of the machine going active (1 second)
. Fixed a memory leak of the polling class

This is just the Chrome change, there will be an additional CL to integrate the ChromiumOS code changes with the API once those are in.

BUG=chromium-os:17167
TEST=Tested with the idle sample extension to confirm that idle query and idle state changed event work correctly.

Review URL: http://codereview.chromium.org/7519008

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

8 files changed, 165 insertions, 88 deletions

diff --git a/chrome/browser/extensions/extension_idle_api.cc b/chrome/browser/extensions/extension_idle_api.cc
index bf22ecc..f52f4b7 100644
--- a/chrome/browser/extensions/extension_idle_api.cc
+++ b/chrome/browser/extensions/extension_idle_api.cc
@@ -2,13 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// This implementation supposes a single extension thread and synchronized
-// method invokation.
-
 #include "chrome/browser/extensions/extension_idle_api.h"
 
 #include <string>
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/json/json_writer.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
@@ -25,9 +24,11 @@
 namespace keys = extension_idle_api_constants;
 
 namespace {
-
-const int kIdlePollInterval = 15;  // Number of seconds between status checks
-                                   // when polling for active.
+const int kIdlePollInterval = 1;  // Number of seconds between status checks
+                                  // when polling for active.
+const int kThrottleInterval = 1;  // Number of seconds to throttle idle checks
+                                  // for. Return the previously checked idle
+                                  // state if the next check is faster than this
 const int kMinThreshold = 15;  // In seconds.  Set >1 sec for security concerns.
 const int kMaxThreshold = 60*60;  // One hours, in seconds.  Not set arbitrarily
                                   // high for security concerns.
@@ -37,32 +38,74 @@ struct ExtensionIdlePollingData {
   double timestamp;
 };
 
-// Static variables shared between instances of polling.
-static ExtensionIdlePollingData polling_data;
-
-// Forward declaration of utility methods.
-static const char* IdleStateToDescription(IdleState state);
-static StringValue* CreateIdleValue(IdleState idle_state);
-static int CheckThresholdBounds(int timeout);
-static IdleState CalculateIdleStateAndUpdateTimestamp(int threshold);
-static void CreateNewPollTask(Profile* profile);
-static IdleState ThrottledCalculateIdleState(int threshold, Profile* profile);
+// Used to throttle excessive calls to query for idle state
+ExtensionIdlePollingData polling_data = {IDLE_STATE_UNKNOWN, 0};
 
-// Internal object which watches for changes in the system idle state.
-class ExtensionIdlePollingTask : public Task {
+// Internal class which is used to poll for changes in the system idle state.
+class ExtensionIdlePollingTask {
  public:
-  explicit ExtensionIdlePollingTask(Profile* profile) : profile_(profile) {}
-  virtual ~ExtensionIdlePollingTask() {}
+  explicit ExtensionIdlePollingTask(int threshold, IdleState last_state,
+      Profile* profile) : threshold_(threshold), last_state_(last_state),
+                          profile_(profile) {}
 
-  // Overridden from Task.
-  virtual void Run();
+  // Check if we're active; then restart the polling task. Do this till we are
+  // are in active state.
+  void CheckIdleState();
+  void IdleStateCallback(IdleState state);
+
+  // Create a poll task to check for Idle state
+  static void CreateNewPollTask(int threshold, IdleState state,
+                                Profile* profile);
 
  private:
+  int threshold_;
+  IdleState last_state_;
   Profile* profile_;
 
+  static bool poll_task_running_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionIdlePollingTask);
 };
 
+// Implementation of ExtensionIdlePollingTask.
+bool ExtensionIdlePollingTask::poll_task_running_ = false;
+
+void ExtensionIdlePollingTask::IdleStateCallback(IdleState current_state) {
+  // If we just came into an active state, notify the extension.
+  if (IDLE_STATE_ACTIVE == current_state && last_state_ != current_state)
+    ExtensionIdleEventRouter::OnIdleStateChange(profile_, current_state);
+
+  ExtensionIdlePollingTask::poll_task_running_ = false;
+
+  // Startup another polling task as we exit.
+  if (current_state != IDLE_STATE_ACTIVE)
+    ExtensionIdlePollingTask::CreateNewPollTask(threshold_, current_state,
+                                                profile_);
+
+  // This instance won't be needed anymore.
+  delete this;
+}
+
+void ExtensionIdlePollingTask::CheckIdleState() {
+  CalculateIdleState(threshold_,
+      base::Bind(&ExtensionIdlePollingTask::IdleStateCallback,
+                 base::Unretained(this)));
+}
+
+// static
+void ExtensionIdlePollingTask::CreateNewPollTask(int threshold, IdleState state,
+                                                 Profile* profile) {
+  if (ExtensionIdlePollingTask::poll_task_running_) return;
+
+  ExtensionIdlePollingTask::poll_task_running_ = true;
+  MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&ExtensionIdlePollingTask::CheckIdleState, base::Unretained(
+          new ExtensionIdlePollingTask(threshold, state, profile))),
+          kIdlePollInterval * 1000);
+}
+
+
 const char* IdleStateToDescription(IdleState state) {
   if (IDLE_STATE_ACTIVE == state)
     return keys::kStateActive;
@@ -84,62 +127,51 @@ int CheckThresholdBounds(int timeout) {
   return timeout;
 }
 
-IdleState CalculateIdleStateAndUpdateTimestamp(int threshold) {
-  polling_data.timestamp = base::Time::Now().ToDoubleT();
-  return CalculateIdleState(threshold);
+bool ShouldThrottle() {
+  double now = base::Time::Now().ToDoubleT();
+  double delta = now - polling_data.timestamp;
+  polling_data.timestamp = now;
+  if (delta < kThrottleInterval)
+    return false;
+  else
+    return true;
 }
+};  // namespace
 
-void CreateNewPollTask(Profile* profile) {
-  MessageLoop::current()->PostDelayedTask(
-      FROM_HERE,
-      new ExtensionIdlePollingTask(profile),
-      kIdlePollInterval * 1000);
-}
-
-IdleState ThrottledCalculateIdleState(int threshold, Profile* profile) {
-  // If we are not active we should be polling.
-  if (IDLE_STATE_ACTIVE != polling_data.state)
-    return polling_data.state;
-
-  // Only allow one check per threshold.
-  double time_now = base::Time::Now().ToDoubleT();
-  double delta = time_now - polling_data.timestamp;
-  if (delta < threshold)
-    return polling_data.state;
-
-  // Update the new state with a poll.  Note this updates time of last check.
-  polling_data.state = CalculateIdleStateAndUpdateTimestamp(threshold);
-
-  if (IDLE_STATE_ACTIVE != polling_data.state)
-    CreateNewPollTask(profile);
-
-  return polling_data.state;
-}
-
-void ExtensionIdlePollingTask::Run() {
-  IdleState state = CalculateIdleStateAndUpdateTimestamp(
-      kIdlePollInterval);
-  if (state != polling_data.state) {
-    polling_data.state = state;
-
-    // Inform of change if the current state is IDLE_STATE_ACTIVE.
-    if (IDLE_STATE_ACTIVE == polling_data.state)
-      ExtensionIdleEventRouter::OnIdleStateChange(profile_, state);
+void ExtensionIdleQueryStateFunction::IdleStateCallback(int threshold,
+                                                        IdleState state) {
+  // If our state is not active, make sure we're running a polling task to check
+  // for active state and report it when it changes to active.
+  if (state != IDLE_STATE_ACTIVE) {
+    ExtensionIdlePollingTask::CreateNewPollTask(threshold, state, profile_);
   }
 
-  // Create a secondary polling task until an active state is reached.
-  if (IDLE_STATE_ACTIVE != polling_data.state)
-    CreateNewPollTask(profile_);
+  result_.reset(CreateIdleValue(state));
+  polling_data.state = state;
+  SendResponse(true);
 }
 
-};  // namespace
-
 bool ExtensionIdleQueryStateFunction::RunImpl() {
   int threshold;
   EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &threshold));
   threshold = CheckThresholdBounds(threshold);
-  IdleState state = ThrottledCalculateIdleState(threshold, profile());
-  result_.reset(CreateIdleValue(state));
+
+  if (ShouldThrottle()) {
+    if (polling_data.state != IDLE_STATE_UNKNOWN) {
+      result_.reset(CreateIdleValue(polling_data.state));
+      SendResponse(true);
+      return true;
+    }
+    // We cannot get the idle state right now, we're already checking for idle
+    // from a previous call, so continue with normal idle check instead.
+  }
+
+  polling_data.state = IDLE_STATE_UNKNOWN;
+  CalculateIdleState(threshold,
+      base::Bind(&ExtensionIdleQueryStateFunction::IdleStateCallback,
+                 base::Unretained(this), threshold));
+
+  // Don't send the response, it'll be sent by our callback
   return true;
 }
 
diff --git a/chrome/browser/extensions/extension_idle_api.h b/chrome/browser/extensions/extension_idle_api.h
index 9f40481..927c68c 100644
--- a/chrome/browser/extensions/extension_idle_api.h
+++ b/chrome/browser/extensions/extension_idle_api.h
@@ -21,10 +21,13 @@ class ExtensionIdleEventRouter {
 };
 
 // Implementation of the chrome.idle.queryState API.
-class ExtensionIdleQueryStateFunction : public SyncExtensionFunction {
+class ExtensionIdleQueryStateFunction : public AsyncExtensionFunction {
  public:
   virtual bool RunImpl();
   DECLARE_EXTENSION_FUNCTION_NAME("idle.queryState")
+
+ private:
+  void IdleStateCallback(int threshold, IdleState state);
 };
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_IDLE_API_H_
diff --git a/chrome/browser/idle.cc b/chrome/browser/idle.cc
new file mode 100644
index 0000000..73b5613
--- /dev/null
+++ b/chrome/browser/idle.cc
@@ -0,0 +1,25 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/idle.h"
+
+#include "base/bind.h"
+#include "base/synchronization/waitable_event.h"
+
+
+
+void IdleStateCallback(IdleState* return_state, base::WaitableEvent* done,
+                       IdleState state) {
+  *return_state = state;
+  done->Signal();
+}
+
+IdleState CalculateIdleStateSync(unsigned int idle_threshold) {
+  IdleState return_state;
+  base::WaitableEvent done(true, false);
+  CalculateIdleState(idle_threshold, base::Bind(&IdleStateCallback,
+                                                &return_state, &done));
+  done.Wait();
+  return return_state;
+}
diff --git a/chrome/browser/idle.h b/chrome/browser/idle.h
index 4f99bef..05b5066 100644
--- a/chrome/browser/idle.h
+++ b/chrome/browser/idle.h
@@ -6,10 +6,14 @@
 #define CHROME_BROWSER_IDLE_H_
 #pragma once
 
+#include "base/callback.h"
+
 enum IdleState {
   IDLE_STATE_ACTIVE = 0,
   IDLE_STATE_IDLE = 1,   // No activity within threshold.
-  IDLE_STATE_LOCKED = 2  // Only available on supported systems.
+  IDLE_STATE_LOCKED = 2,  // Only available on supported systems.
+  IDLE_STATE_UNKNOWN = 3  // Used when waiting for the Idle state or in error
+                          // conditions
 };
 
 // For MacOSX, InitIdleMonitor needs to be called first to setup the monitor.
@@ -19,6 +23,11 @@ void InitIdleMonitor();
 void StopIdleMonitor();
 #endif
 
-IdleState CalculateIdleState(unsigned int idle_threshold);
+typedef base::Callback<void(IdleState)> IdleCallback;
+
+// Calculate the Idle state and notify the callback.
+void CalculateIdleState(unsigned int idle_threshold, IdleCallback notify);
+// Calculate the Idle state synchronously and return the state.
+IdleState CalculateIdleStateSync(unsigned int idle_threshold);
 
 #endif  // CHROME_BROWSER_IDLE_H_
diff --git a/chrome/browser/idle_linux.cc b/chrome/browser/idle_linux.cc
index 2689808..eba7958 100644
--- a/chrome/browser/idle_linux.cc
+++ b/chrome/browser/idle_linux.cc
@@ -63,18 +63,21 @@ bool ScreensaverWindowExists() {
 
 }
 
-IdleState CalculateIdleState(unsigned int idle_threshold) {
+void CalculateIdleState(unsigned int idle_threshold, IdleCallback notify) {
   // Usually the screensaver is used to lock the screen, so we do not need to
   // check if the workstation is locked.
   gdk_error_trap_push();
   bool result = ScreensaverWindowExists();
   bool got_error = gdk_error_trap_pop();
-  if (result && !got_error)
-    return IDLE_STATE_LOCKED;
+  if (result && !got_error) {
+    notify.Run(IDLE_STATE_LOCKED);
+    return;
+  }
 
   browser::IdleQueryLinux idle_query;
   unsigned int idle_time = idle_query.IdleTime();
   if (idle_time >= idle_threshold)
-    return IDLE_STATE_IDLE;
-  return IDLE_STATE_ACTIVE;
+    notify.Run(IDLE_STATE_IDLE);
+  else
+    notify.Run(IDLE_STATE_ACTIVE);
 }
diff --git a/chrome/browser/idle_mac.mm b/chrome/browser/idle_mac.mm
index 4032408..e1ab476 100644
--- a/chrome/browser/idle_mac.mm
+++ b/chrome/browser/idle_mac.mm
@@ -84,16 +84,18 @@ void StopIdleMonitor() {
   g_screenMonitor = nil;
 }
 
-IdleState CalculateIdleState(unsigned int idle_threshold) {
+void CalculateIdleState(unsigned int idle_threshold, IdleCallback notify) {
   if ([g_screenMonitor isScreensaverRunning] ||
-      [g_screenMonitor isScreenLocked])
-    return IDLE_STATE_LOCKED;
+      [g_screenMonitor isScreenLocked]) {
+    notify.Run(IDLE_STATE_LOCKED);
+    return;
+  }
 
   CFTimeInterval idle_time = CGEventSourceSecondsSinceLastEventType(
       kCGEventSourceStateCombinedSessionState,
       kCGAnyInputEventType);
   if (idle_time >= idle_threshold)
-    return IDLE_STATE_IDLE;
-
-  return IDLE_STATE_ACTIVE;
+    notify.Run(IDLE_STATE_IDLE);
+  else
+    notify.Run(IDLE_STATE_ACTIVE);
 }
diff --git a/chrome/browser/idle_win.cc b/chrome/browser/idle_win.cc
index ed45da2..c745ca9 100644
--- a/chrome/browser/idle_win.cc
+++ b/chrome/browser/idle_win.cc
@@ -10,9 +10,11 @@
 static bool IsScreensaverRunning();
 static bool IsWorkstationLocked();
 
-IdleState CalculateIdleState(unsigned int idle_threshold) {
-  if (IsScreensaverRunning() || IsWorkstationLocked())
-    return IDLE_STATE_LOCKED;
+void CalculateIdleState(unsigned int idle_threshold, IdleCallback notify) {
+  if (IsScreensaverRunning() || IsWorkstationLocked()) {
+    notify.Run(IDLE_STATE_LOCKED);
+    return;
+  }
 
   LASTINPUTINFO last_input_info = {0};
   last_input_info.cbSize = sizeof(LASTINPUTINFO);
@@ -36,8 +38,9 @@ IdleState CalculateIdleState(unsigned int idle_threshold) {
   }
 
   if (current_idle_time >= idle_threshold)
-    return IDLE_STATE_IDLE;
-  return IDLE_STATE_ACTIVE;
+    notify.Run(IDLE_STATE_IDLE);
+  else
+    notify.Run(IDLE_STATE_ACTIVE);
 }
 
 bool IsScreensaverRunning() {
diff --git a/chrome/browser/notifications/notification_ui_manager.cc b/chrome/browser/notifications/notification_ui_manager.cc
index 2d66214..a2fdf24 100644
--- a/chrome/browser/notifications/notification_ui_manager.cc
+++ b/chrome/browser/notifications/notification_ui_manager.cc
@@ -152,7 +152,7 @@ void NotificationUIManager::CheckAndShowNotifications() {
 
 void NotificationUIManager::CheckUserState() {
   bool is_user_active_previously = is_user_active_;
-  is_user_active_ = CalculateIdleState(0) != IDLE_STATE_LOCKED &&
+  is_user_active_ = CalculateIdleStateSync(0) != IDLE_STATE_LOCKED &&
                     !IsFullScreenMode();
   if (is_user_active_ == is_user_active_previously)
     return;