AU: DBus support.

AU: DBus support.

A few changes to support dbus in the Update Engine daemon:

- SConstruct: build marshaller for the dbus signal.

- Update Attempter: respond to dbus calls and broadcast status on dbus
  signal.

- Update Engine Client: flag to listen for status updates.

- Also, cleanup outdated code in Omaha Response Handler.

BUG=None
TEST=attached unittests/on device tests

[adlr, 2010-05-07 04:24:22]

Thanks for volunteering for code reviewing. I'll try not to bug you too much.

[adlr, 2010-05-10 15:46:34]

[-sam, +dan]

[Daniel Erat, 2010-05-10 16:29:49]

http://codereview.chromium.org/2037002/diff/1/2
File src/platform/update_engine/SConstruct (right):

http://codereview.chromium.org/2037002/diff/1/2#newcode82
src/platform/update_engine/SConstruct:82: """ Inputs:
follow python style here (and update the rest of the file too): docstrings
should appear after the initial function signature.  see 
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Comments

http://codereview.chromium.org/2037002/diff/1/4
File src/platform/update_engine/dbus_service.cc (right):

http://codereview.chromium.org/2037002/diff/1/4#newcode79
src/platform/update_engine/dbus_service.cc:79: int64_t last_checked_time,
nit: you should probably use gint64 and gdouble here since this is glib code

http://codereview.chromium.org/2037002/diff/1/7
File src/platform/update_engine/download_action.h (right):

http://codereview.chromium.org/2037002/diff/1/7#newcode42
src/platform/update_engine/download_action.h:42: virtual void
BytesReceived(uint64 bytes_received, uint64 total) = 0;
s/uint64/uint64_t/

http://codereview.chromium.org/2037002/diff/1/11
File src/platform/update_engine/main.cc (right):

http://codereview.chromium.org/2037002/diff/1/11#newcode45
src/platform/update_engine/main.cc:45: 123,
what are these magic numbers?

http://codereview.chromium.org/2037002/diff/1/11#newcode100
src/platform/update_engine/main.cc:100: logging::LOG_ONLY_TO_SYSTEM_DEBUG_LOG :
do you mind sticking with the old indentation, actually (or using parentheses)? 
i think that it makes it easier to read

http://codereview.chromium.org/2037002/diff/1/11#newcode122
src/platform/update_engine/main.cc:122: //g_timeout_add(0,
&chromeos_update_engine::SetupInMainLoop, &update_attempter);
delete this?

http://codereview.chromium.org/2037002/diff/1/16
File src/platform/update_engine/update_attempter.cc (right):

http://codereview.chromium.org/2037002/diff/1/16#newcode7
src/platform/update_engine/update_attempter.cc:7: #define _POSIX_C_SOURCE
199309L  // For clock_*()
what's this do?  add a comment

http://codereview.chromium.org/2037002/diff/1/16#newcode41
src/platform/update_engine/update_attempter.cc:41: ret.tv_sec = (stop.tv_sec +
1000000000L) - start.tv_sec;
this looks wrong.

but why don't you just store the usec in an int64_t instead?  it'll make things
much easier.

http://codereview.chromium.org/2037002/diff/1/16#newcode191
src/platform/update_engine/update_attempter.cc:191:
reinterpret_cast<OmahaResponseHandlerAction*>(action)->install_plan();
this should be dynamic_cast (i think that reinterpret_cast will do the wrong
thing if you start using multiple inheritance later)

http://codereview.chromium.org/2037002/diff/1/16#newcode214
src/platform/update_engine/update_attempter.cc:214: void
UpdateAttempter::BytesReceived(uint64 bytes_received, uint64 total) {
uint64_t

http://codereview.chromium.org/2037002/diff/1/17
File src/platform/update_engine/update_attempter.h (right):

http://codereview.chromium.org/2037002/diff/1/17#newcode22
src/platform/update_engine/update_attempter.h:22: UPDATE_STATUS_IDLE,
can you add an " = 0" at the end of this line?

http://codereview.chromium.org/2037002/diff/1/17#newcode76
src/platform/update_engine/update_attempter.h:76: void BytesReceived(uint64
bytes_received, uint64 total);
uint64_t

http://codereview.chromium.org/2037002/diff/1/19
File src/platform/update_engine/update_engine_client.cc (right):

http://codereview.chromium.org/2037002/diff/1/19#newcode53
src/platform/update_engine/update_engine_client.cc:53: static void
StatusUpdateSignalHandler(DBusGProxy *proxy,
fix '*' placement here (and many other places in this file)

[adlr, 2010-05-10 17:30:04]

Dan, thanks for the quick review. Please have another look.

Thanks,

http://codereview.chromium.org/2037002/diff/1/2
File src/platform/update_engine/SConstruct (right):

http://codereview.chromium.org/2037002/diff/1/2#newcode82
src/platform/update_engine/SConstruct:82: """ Inputs:
On 2010/05/10 16:29:49, Daniel Erat wrote:
> follow python style here (and update the rest of the file too): docstrings
> should appear after the initial function signature.  see 
> http://google-styleguide.googlecode.com/svn/trunk/pyguide.html#Comments

Done.

http://codereview.chromium.org/2037002/diff/1/4
File src/platform/update_engine/dbus_service.cc (right):

http://codereview.chromium.org/2037002/diff/1/4#newcode79
src/platform/update_engine/dbus_service.cc:79: int64_t last_checked_time,
On 2010/05/10 16:29:49, Daniel Erat wrote:
> nit: you should probably use gint64 and gdouble here since this is glib code

Done.

http://codereview.chromium.org/2037002/diff/1/7
File src/platform/update_engine/download_action.h (right):

http://codereview.chromium.org/2037002/diff/1/7#newcode42
src/platform/update_engine/download_action.h:42: virtual void
BytesReceived(uint64 bytes_received, uint64 total) = 0;
On 2010/05/10 16:29:49, Daniel Erat wrote:
> s/uint64/uint64_t/

oh, should have caught that. Fixed.

http://codereview.chromium.org/2037002/diff/1/11
File src/platform/update_engine/main.cc (right):

http://codereview.chromium.org/2037002/diff/1/11#newcode45
src/platform/update_engine/main.cc:45: 123,
On 2010/05/10 16:29:49, Daniel Erat wrote:
> what are these magic numbers?

sorry, this file was embarrassingly messy. deleted this function.

http://codereview.chromium.org/2037002/diff/1/11#newcode100
src/platform/update_engine/main.cc:100: logging::LOG_ONLY_TO_SYSTEM_DEBUG_LOG :
On 2010/05/10 16:29:49, Daniel Erat wrote:
> do you mind sticking with the old indentation, actually (or using
parentheses)? 
> i think that it makes it easier to read

I put in parens and according indent (which is 1 space).

http://codereview.chromium.org/2037002/diff/1/11#newcode122
src/platform/update_engine/main.cc:122: //g_timeout_add(0,
&chromeos_update_engine::SetupInMainLoop, &update_attempter);
On 2010/05/10 16:29:49, Daniel Erat wrote:
> delete this?

Done.

http://codereview.chromium.org/2037002/diff/1/16
File src/platform/update_engine/update_attempter.cc (right):

http://codereview.chromium.org/2037002/diff/1/16#newcode7
src/platform/update_engine/update_attempter.cc:7: #define _POSIX_C_SOURCE
199309L  // For clock_*()
On 2010/05/10 16:29:49, Daniel Erat wrote:
> what's this do?  add a comment

Hopefully explained? Some functions require macros to be set before #include-ing
a .h file. In this case, we need _POSIX_C_SOURCE to be set to 199309 before
#include-ing time.h

http://codereview.chromium.org/2037002/diff/1/16#newcode41
src/platform/update_engine/update_attempter.cc:41: ret.tv_sec = (stop.tv_sec +
1000000000L) - start.tv_sec;
On 2010/05/10 16:29:49, Daniel Erat wrote:
> this looks wrong.
> 
> but why don't you just store the usec in an int64_t instead?  it'll make
things
> much easier.

Done.

http://codereview.chromium.org/2037002/diff/1/16#newcode191
src/platform/update_engine/update_attempter.cc:191:
reinterpret_cast<OmahaResponseHandlerAction*>(action)->install_plan();
On 2010/05/10 16:29:49, Daniel Erat wrote:
> this should be dynamic_cast (i think that reinterpret_cast will do the wrong
> thing if you start using multiple inheritance later)

Oh, good point. I suppose safety is more important than our "don't use multiple
inheritance (and therefore you don't use dynamic cast)" rule. Changed.

http://codereview.chromium.org/2037002/diff/1/16#newcode214
src/platform/update_engine/update_attempter.cc:214: void
UpdateAttempter::BytesReceived(uint64 bytes_received, uint64 total) {
On 2010/05/10 16:29:49, Daniel Erat wrote:
> uint64_t

Done.

http://codereview.chromium.org/2037002/diff/1/17
File src/platform/update_engine/update_attempter.h (right):

http://codereview.chromium.org/2037002/diff/1/17#newcode22
src/platform/update_engine/update_attempter.h:22: UPDATE_STATUS_IDLE,
On 2010/05/10 16:29:49, Daniel Erat wrote:
> can you add an " = 0" at the end of this line?

Done.

http://codereview.chromium.org/2037002/diff/1/17#newcode76
src/platform/update_engine/update_attempter.h:76: void BytesReceived(uint64
bytes_received, uint64 total);
On 2010/05/10 16:29:49, Daniel Erat wrote:
> uint64_t

Done.

http://codereview.chromium.org/2037002/diff/1/19
File src/platform/update_engine/update_engine_client.cc (right):

http://codereview.chromium.org/2037002/diff/1/19#newcode53
src/platform/update_engine/update_engine_client.cc:53: static void
StatusUpdateSignalHandler(DBusGProxy *proxy,
On 2010/05/10 16:29:49, Daniel Erat wrote:
> fix '*' placement here (and many other places in this file)

Done.

[Daniel Erat, 2010-05-10 19:11:42]

LGTM

http://codereview.chromium.org/2037002/diff/12001/13015
File src/platform/update_engine/update_attempter.cc (right):

http://codereview.chromium.org/2037002/diff/12001/13015#newcode46
src/platform/update_engine/update_attempter.cc:46: const int64 kOneBillion =
1000000000L;
nit: int64_t :-)