Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(164)

Issue 10024013: chromeos: Add support for the battery status API on chromeos.

Created:
7 years, 7 months ago by sadrul
Modified:
7 years, 7 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org
Visibility:
Public.

Description

chromeos: Add support for the battery status API on chromeos. BUG=122593 TEST=none

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : DEPS #

Total comments: 1

Patch Set 4 : . #

Patch Set 5 : comment #

Patch Set 6 : . #

Total comments: 2

Patch Set 7 : file-move #

Total comments: 8

Patch Set 8 : . #

Total comments: 12

Patch Set 9 : . #

Total comments: 2

Patch Set 10 : threadsafe-observers #

Patch Set 11 : . #

Patch Set 12 : . #

Total comments: 1

Patch Set 13 : tot-merge #

Patch Set 14 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+514 lines, -0 lines) Patch
A chrome/browser/chromeos/battery_status_provider_chromeos.h View 1 2 3 4 5 6 1 chunk +30 lines, -0 lines 0 comments Download
A chrome/browser/chromeos/battery_status_provider_chromeos.cc View 1 2 3 4 5 6 7 1 chunk +38 lines, -0 lines 0 comments Download
M chrome/browser/ui/views/ash/chrome_browser_main_extra_parts_ash.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +2 lines, -0 lines 0 comments Download
A content/browser/battery_status/battery_status_message_filter.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +41 lines, -0 lines 0 comments Download
A content/browser/battery_status/battery_status_message_filter.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +94 lines, -0 lines 0 comments Download
A content/browser/battery_status/battery_status_provider_impl.h View 1 2 3 4 5 6 7 8 10 1 chunk +42 lines, -0 lines 0 comments Download
A content/browser/battery_status/battery_status_provider_impl.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +44 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +2 lines, -0 lines 0 comments Download
M content/common/content_message_generator.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M content/content_browser.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +6 lines, -0 lines 0 comments Download
M content/content_common.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download
M content/content_renderer.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -0 lines 0 comments Download
A content/public/browser/battery_status_provider.h View 1 2 3 4 5 6 7 8 1 chunk +27 lines, -0 lines 0 comments Download
A content/public/browser/power_supply_status.h View 1 2 3 4 5 6 7 1 chunk +30 lines, -0 lines 0 comments Download
A content/renderer/battery_status_dispatcher.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +48 lines, -0 lines 0 comments Download
A content/renderer/battery_status_dispatcher.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +66 lines, -0 lines 0 comments Download
M content/renderer/render_thread_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +6 lines, -0 lines 0 comments Download
M content/renderer/render_thread_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +14 lines, -0 lines 0 comments Download
M content/renderer/renderer_webkitplatformsupport_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/renderer_webkitplatformsupport_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +12 lines, -0 lines 0 comments Download
M ipc/ipc_message_utils.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 24 (0 generated)
sadrul
ben@: Mostly for content/browser/battery_status/DEPS. The chromeos dependency is added with the tightest scope, only for ...
7 years, 7 months ago (2012-04-09 01:41:06 UTC) #1
Ben Goodger (Google)
+jam, who might have more time to review this week.
7 years, 7 months ago (2012-04-09 19:26:22 UTC) #2
jam
(just saw this). i stopped after seeing the chromeos include http://codereview.chromium.org/10024013/diff/1002/content/browser/battery_status/DEPS File content/browser/battery_status/DEPS (right): http://codereview.chromium.org/10024013/diff/1002/content/browser/battery_status/DEPS#newcode2 ...
7 years, 7 months ago (2012-04-10 16:09:22 UTC) #3
sadrul
On 2012/04/10 16:09:22, John Abd-El-Malek wrote: > (just saw this). i stopped after seeing the ...
7 years, 7 months ago (2012-04-10 16:13:04 UTC) #4
sadrul
Updated patch to remove chromeos dependency. PTAL.
7 years, 7 months ago (2012-04-10 20:00:58 UTC) #5
jam
having these chromeos ifdefs in content is best avoided. chromeos should build on content, and ...
7 years, 7 months ago (2012-04-10 22:58:37 UTC) #6
sadrul
On 2012/04/10 22:58:37, John Abd-El-Malek wrote: > having these chromeos ifdefs in content is best ...
7 years, 7 months ago (2012-04-10 23:08:08 UTC) #7
jam
ok, since it's not just chromeos specific (even if we only have cros impl for ...
7 years, 7 months ago (2012-04-11 00:50:49 UTC) #8
sadrul
On 2012/04/11 00:50:49, John Abd-El-Malek wrote: > ok, since it's not just chromeos specific (even ...
7 years, 7 months ago (2012-04-11 07:06:50 UTC) #9
sadrul
On 2012/04/11 07:06:50, sadrul wrote: > On 2012/04/11 00:50:49, John Abd-El-Malek wrote: > > ok, ...
7 years, 7 months ago (2012-04-11 20:45:47 UTC) #10
Ben Goodger (Google)
http://codereview.chromium.org/10024013/diff/12001/chrome/browser/battery_status_provider_chromeos.h File chrome/browser/battery_status_provider_chromeos.h (right): http://codereview.chromium.org/10024013/diff/12001/chrome/browser/battery_status_provider_chromeos.h#newcode5 chrome/browser/battery_status_provider_chromeos.h:5: #ifndef CHROME_BROWSER_BATTERY_STATUS_PROVIDER_CHROMEOS_H_ -> chrome/browser/chromeos at least.
7 years, 7 months ago (2012-04-11 21:09:11 UTC) #11
sadrul
http://codereview.chromium.org/10024013/diff/12001/chrome/browser/battery_status_provider_chromeos.h File chrome/browser/battery_status_provider_chromeos.h (right): http://codereview.chromium.org/10024013/diff/12001/chrome/browser/battery_status_provider_chromeos.h#newcode5 chrome/browser/battery_status_provider_chromeos.h:5: #ifndef CHROME_BROWSER_BATTERY_STATUS_PROVIDER_CHROMEOS_H_ On 2012/04/11 21:09:11, Ben Goodger (Google) wrote: ...
7 years, 7 months ago (2012-04-11 21:50:54 UTC) #12
jam
(looked through most of this, but getting off shuttle so I figured I'd send these ...
7 years, 7 months ago (2012-04-12 01:55:09 UTC) #13
sadrul
Thanks for the excellent advice. I have made the suggested changes. PTAL. http://codereview.chromium.org/10024013/diff/15002/content/common/battery_status_messages.h File content/common/battery_status_messages.h ...
7 years, 7 months ago (2012-04-12 05:20:09 UTC) #14
jam
http://codereview.chromium.org/10024013/diff/24007/content/browser/battery_status/battery_status_message_filter.h File content/browser/battery_status/battery_status_message_filter.h (right): http://codereview.chromium.org/10024013/diff/24007/content/browser/battery_status/battery_status_message_filter.h#newcode13 content/browser/battery_status/battery_status_message_filter.h:13: class BatteryStatusMessageFilter : public content::BrowserMessageFilter { nit: for this ...
7 years, 7 months ago (2012-04-12 19:34:55 UTC) #15
sadrul
http://codereview.chromium.org/10024013/diff/24007/content/browser/battery_status/battery_status_message_filter.h File content/browser/battery_status/battery_status_message_filter.h (right): http://codereview.chromium.org/10024013/diff/24007/content/browser/battery_status/battery_status_message_filter.h#newcode13 content/browser/battery_status/battery_status_message_filter.h:13: class BatteryStatusMessageFilter : public content::BrowserMessageFilter { On 2012/04/12 19:34:55, ...
7 years, 7 months ago (2012-04-12 21:53:24 UTC) #16
jam
http://codereview.chromium.org/10024013/diff/26004/content/browser/battery_status/battery_status_provider_impl.cc File content/browser/battery_status/battery_status_provider_impl.cc (right): http://codereview.chromium.org/10024013/diff/26004/content/browser/battery_status/battery_status_provider_impl.cc#newcode35 content/browser/battery_status/battery_status_provider_impl.cc:35: FOR_EACH_OBSERVER(Observer, observers_, PowerChanged(status)); so observers_ is written on IO ...
7 years, 7 months ago (2012-04-13 00:46:42 UTC) #17
sadrul
http://codereview.chromium.org/10024013/diff/26004/content/browser/battery_status/battery_status_provider_impl.cc File content/browser/battery_status/battery_status_provider_impl.cc (right): http://codereview.chromium.org/10024013/diff/26004/content/browser/battery_status/battery_status_provider_impl.cc#newcode35 content/browser/battery_status/battery_status_provider_impl.cc:35: FOR_EACH_OBSERVER(Observer, observers_, PowerChanged(status)); On 2012/04/13 00:46:42, John Abd-El-Malek wrote: ...
7 years, 7 months ago (2012-04-13 11:28:44 UTC) #18
jam
On 2012/04/13 11:28:44, sadrul wrote: > http://codereview.chromium.org/10024013/diff/26004/content/browser/battery_status/battery_status_provider_impl.cc > File content/browser/battery_status/battery_status_provider_impl.cc (right): > > http://codereview.chromium.org/10024013/diff/26004/content/browser/battery_status/battery_status_provider_impl.cc#newcode35 > ...
7 years, 7 months ago (2012-04-13 18:30:07 UTC) #19
sadrul
[snip] > > content/browser/battery_status/battery_status_provider_impl.cc:35: > > FOR_EACH_OBSERVER(Observer, observers_, PowerChanged(status)); > > On 2012/04/13 00:46:42, John ...
7 years, 7 months ago (2012-04-13 20:26:24 UTC) #20
sadrul
ping!
7 years, 7 months ago (2012-04-17 16:32:03 UTC) #21
jam
lgtm
7 years, 7 months ago (2012-04-17 21:54:46 UTC) #22
sadrul
On 2012/04/17 21:54:46, John Abd-El-Malek wrote: > lgtm Thanks! Ben, Darin: Did you guys want ...
7 years, 7 months ago (2012-04-17 22:09:39 UTC) #23
Ben Goodger (Google)
7 years, 7 months ago (2012-04-17 22:20:31 UTC) #24
LGTM other than nit:

http://codereview.chromium.org/10024013/diff/39002/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/10024013/diff/39002/chrome/chrome_browser.gypi...
chrome/chrome_browser.gypi:283:
'browser/chromeos/battery_status_provider_chromeos.cc',
wrong place in gyp file

Powered by Google App Engine
This is Rietveld 408576698