Chromium Code Reviews
Help | Chromium Project | Sign in
(678)

Issue 9318007: Adding using protobuf based communication for the power information transfer. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 2 months ago by rharrison
Modified:
2 years, 1 month ago
Reviewers:
satorux1
CC:
chromium-reviews_chromium.org, stevenjb+watch_chromium.org, nkostylev+watch_chromium.org, davemoore+watch_chromium.org
Base URL:
http://git.chromium.org/chromium/src.git@master
Visibility:
Public.

Description

Adding using protobuf based communication for the power information transfer.

Added in gyp rule for building the needed protobuf. Added secondary code path that calls the PopArrayOfBytesAsProto command and then passes the value up to the rest of Chrome. Removed the old code path.

BUG=chromium-os:25489
TEST=Build image using new code path ChromeOS and Chrome. Then test that is
working on a device.


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

Patch Set 1 #

Patch Set 2 : Removed a change in the .gypi that should have been in there #

Patch Set 3 : Fixed a plethora of build issues #

Total comments: 6

Patch Set 4 : Removed old code path #

Patch Set 5 : Rebased CL since it became stale #

Patch Set 6 : Rewrote the gypi rule for the protobuf, so to not leak into non-cros builds #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -21 lines) Lint Patch
M chrome/browser/chromeos/dbus/power_manager_client.cc View 1 2 3 4 5 3 chunks +18 lines, -21 lines 0 comments 0 errors Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 2 chunks +19 lines, -0 lines 0 comments ? errors Download
Commit:

Messages

Total messages: 14
rharrison
I am intending to build a test image tomorrow for the purposes of verifying that ...
2 years, 2 months ago #1
rharrison
I have cleaned up the CLs leading up to this one and I am in ...
2 years, 2 months ago #2
satorux1
http://codereview.chromium.org/9318007/diff/4001/chrome/browser/chromeos/dbus/power_manager_client.cc File chrome/browser/chromeos/dbus/power_manager_client.cc (right): http://codereview.chromium.org/9318007/diff/4001/chrome/browser/chromeos/dbus/power_manager_client.cc#newcode171 chrome/browser/chromeos/dbus/power_manager_client.cc:171: dbus::MethodCall get_all_method_call(power_manager::kPowerManagerInterface, Do we still need to call the ...
2 years, 2 months ago #3
satorux1
ping?
2 years, 2 months ago #4
rharrison
On 2012/02/13 20:18:34, satorux1 wrote: > ping? I was dealing with other CLs that this ...
2 years, 2 months ago #5
satorux1
Thanks you for the update. sounds good to me.
2 years, 2 months ago #6
rharrison
I have removed the old code path from the build now. I don't have an ...
2 years, 2 months ago #7
satorux1
LGTM. There is no rush. Please check this in once you get back to work.
2 years, 2 months ago #8
rharrison
On 2012/02/14 17:11:41, satorux1 wrote: > LGTM. There is no rush. Please check this in ...
2 years, 2 months ago #9
rharrison
On 2012/02/14 19:55:11, rharrison wrote: > On 2012/02/14 17:11:41, satorux1 wrote: > > LGTM. There ...
2 years, 1 month ago #10
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rharrison@chromium.org/9318007/18001
2 years, 1 month ago #11
I haz the power (commit-bot)
Try job failure for 9318007-18001 (retry) on linux_rel for step "compile" (clobber build). It's a ...
2 years, 1 month ago #12
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rharrison@chromium.org/9318007/27001
2 years, 1 month ago #13
I haz the power (commit-bot)
2 years, 1 month ago #14
Change committed as 125005
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6