chromeos: call GetIdleTime from power manager client

chromeos: call GetIdleTime from power manager client

No longer using libcros to call the method.  Part of libcros removal.
Instead chrome will call the method directly.

BUG=chromium-os:16558
TEST=none

Signed-off-by: Simon Que <sque@chromium.org>;

R=satorux@chromium.org,stevenjb@chromium.org,tbarzic@chromium.org


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110188

[satorux1, 2011-11-15 00:57:42]

http://codereview.chromium.org/8566024/diff/1/chrome/browser/chromeos/cros/mo...
File chrome/browser/chromeos/cros/mock_power_library.h (left):

http://codereview.chromium.org/8566024/diff/1/chrome/browser/chromeos/cros/mo...
chrome/browser/chromeos/cros/mock_power_library.h:27:
MOCK_METHOD1(EnableScreenLock, void(bool));
One more function to go! :)

http://codereview.chromium.org/8566024/diff/1/chrome/browser/chromeos/dbus/po...
File chrome/browser/chromeos/dbus/power_manager_client.h (right):

http://codereview.chromium.org/8566024/diff/1/chrome/browser/chromeos/dbus/po...
chrome/browser/chromeos/dbus/power_manager_client.h:40: typedef
base::Callback<void(int64_t)> CalculateIdleTimeCallback;
Please add some comment about the parameter: int64_t.

http://codereview.chromium.org/8566024/diff/1/chrome/browser/chromeos/dbus/po...
chrome/browser/chromeos/dbus/power_manager_client.h:79: // it returns -1.
The comment is misleading. It's a void function. You can say something like

If it encounters some error, -1 is passed to |callback|.

BTW, on success, what value is passed? Please also document.

http://codereview.chromium.org/8566024/diff/1/chrome/browser/chromeos/dbus/po...
chrome/browser/chromeos/dbus/power_manager_client.h:80: virtual void
CalculateIdleTime(CalculateIdleTimeCallback* callback) = 0;
This shouldn't have to be a pointer. Please remove *, and fix the caller.

http://codereview.chromium.org/8566024/diff/1/chrome/browser/idle_chromeos.cc
File chrome/browser/idle_chromeos.cc (right):

http://codereview.chromium.org/8566024/diff/1/chrome/browser/idle_chromeos.cc...
chrome/browser/idle_chromeos.cc:37: CalculateIdleTime(callback);
It no longer needs to be a new'ed object. Please change it to be something like

chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->
    CalculateIdleTime(base::Bind((&CalculateIdleStateNotifier, idle_threshold,
notify)));

[Simon Que, 2011-11-15 01:29:38]

PTAL

http://codereview.chromium.org/8566024/diff/1/chrome/browser/chromeos/dbus/po...
File chrome/browser/chromeos/dbus/power_manager_client.h (right):

http://codereview.chromium.org/8566024/diff/1/chrome/browser/chromeos/dbus/po...
chrome/browser/chromeos/dbus/power_manager_client.h:40: typedef
base::Callback<void(int64_t)> CalculateIdleTimeCallback;
On 2011/11/15 00:57:42, satorux1 wrote:
> Please add some comment about the parameter: int64_t.

Done.

http://codereview.chromium.org/8566024/diff/1/chrome/browser/chromeos/dbus/po...
chrome/browser/chromeos/dbus/power_manager_client.h:79: // it returns -1.
On 2011/11/15 00:57:42, satorux1 wrote:
> The comment is misleading. It's a void function. You can say something like
> 
> If it encounters some error, -1 is passed to |callback|.
> 
> BTW, on success, what value is passed? Please also document.

Done.

http://codereview.chromium.org/8566024/diff/1/chrome/browser/chromeos/dbus/po...
chrome/browser/chromeos/dbus/power_manager_client.h:80: virtual void
CalculateIdleTime(CalculateIdleTimeCallback* callback) = 0;
On 2011/11/15 00:57:42, satorux1 wrote:
> This shouldn't have to be a pointer. Please remove *, and fix the caller.

Done.

http://codereview.chromium.org/8566024/diff/1/chrome/browser/idle_chromeos.cc
File chrome/browser/idle_chromeos.cc (right):

http://codereview.chromium.org/8566024/diff/1/chrome/browser/idle_chromeos.cc...
chrome/browser/idle_chromeos.cc:37: CalculateIdleTime(callback);
On 2011/11/15 00:57:42, satorux1 wrote:
> It no longer needs to be a new'ed object. Please change it to be something
like
> 
> chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->
>     CalculateIdleTime(base::Bind((&CalculateIdleStateNotifier, idle_threshold,
> notify)));

Done.

[satorux1, 2011-11-15 02:07:12]

http://codereview.chromium.org/8566024/diff/1009/chrome/browser/chromeos/dbus...
File chrome/browser/chromeos/dbus/power_manager_client.cc (right):

http://codereview.chromium.org/8566024/diff/1009/chrome/browser/chromeos/dbus...
chrome/browser/chromeos/dbus/power_manager_client.cc:247: dbus::Response*
response) {
Please add something like:

if (!response) {
  LOG(ERROR) << "Error calling " << power_manager::kGetIdleTime;
  callback.Run(-1);
  return;
}

http://codereview.chromium.org/8566024/diff/1009/chrome/browser/chromeos/dbus...
chrome/browser/chromeos/dbus/power_manager_client.cc:319: OVERRIDE {
Please call callback.Run(0) to keep compatibility with the behavior in
PowerLibrary.

http://codereview.chromium.org/8566024/diff/1009/chrome/browser/chromeos/dbus...
File chrome/browser/chromeos/dbus/power_manager_client.h (right):

http://codereview.chromium.org/8566024/diff/1009/chrome/browser/chromeos/dbus...
chrome/browser/chromeos/dbus/power_manager_client.h:82: // replied.  It passes
the idle time in seconds to |callback|.  If it
seconds -> milliseconds? The comment above says it's milliseconds.

[Simon Que, 2011-11-15 02:47:43]

PTAL

http://codereview.chromium.org/8566024/diff/1009/chrome/browser/chromeos/dbus...
File chrome/browser/chromeos/dbus/power_manager_client.cc (right):

http://codereview.chromium.org/8566024/diff/1009/chrome/browser/chromeos/dbus...
chrome/browser/chromeos/dbus/power_manager_client.cc:247: dbus::Response*
response) {
On 2011/11/15 02:07:12, satorux1 wrote:
> Please add something like:
> 
> if (!response) {
>   LOG(ERROR) << "Error calling " << power_manager::kGetIdleTime;
>   callback.Run(-1);
>   return;
> }

Done.

http://codereview.chromium.org/8566024/diff/1009/chrome/browser/chromeos/dbus...
chrome/browser/chromeos/dbus/power_manager_client.cc:319: OVERRIDE {
On 2011/11/15 02:07:12, satorux1 wrote:
> Please call callback.Run(0) to keep compatibility with the behavior in
> PowerLibrary.

Done.

http://codereview.chromium.org/8566024/diff/1009/chrome/browser/chromeos/dbus...
File chrome/browser/chromeos/dbus/power_manager_client.h (right):

http://codereview.chromium.org/8566024/diff/1009/chrome/browser/chromeos/dbus...
chrome/browser/chromeos/dbus/power_manager_client.h:82: // replied.  It passes
the idle time in seconds to |callback|.  If it
On 2011/11/15 02:07:12, satorux1 wrote:
> seconds -> milliseconds? The comment above says it's milliseconds.

Done. Actually the other way around.

[satorux1, 2011-11-15 05:52:49]

LGTM

[commit-bot: I haz the power, 2011-11-15 21:21:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sque@chromium.org/8566024/10001

[satorux1, 2011-11-15 21:21:52]

Still LGTM

[commit-bot: I haz the power, 2011-11-15 22:24:53]

Change committed as 110188