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

Issue 401083002: Initialize PowerMonitor on_power_battery intitial value for newly created processes. (Closed)

Created:
5 years, 3 months ago by fmeawad
Modified:
5 years, 2 months ago
CC:
chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, nasko+codewatch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Initialize PowerMonitor on_power_battery initial value for newly created processes. Adding an Init method to the Broadcaster, that gets invoked while initializing the host. The Init method broadcasts the original on_power_battery value to the new processes. BUG=153139 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284599

Patch Set 1 #

Total comments: 5

Patch Set 2 : Remove the power_monitor check. #

Patch Set 3 : Restore power_monitor checks #

Patch Set 4 : Typo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -4 lines) Patch
M content/browser/browser_child_process_host_impl.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/power_monitor_message_broadcaster.h View 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/power_monitor_message_broadcaster.cc View 1 2 3 1 chunk +7 lines, -0 lines 0 comments Download
M content/browser/power_monitor_message_broadcaster_unittest.cc View 2 chunks +8 lines, -4 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
bajones
This looks good, just one question about IPC behavior: If the child process hasn't finished ...
5 years, 3 months ago (2014-07-18 22:35:51 UTC) #1
fmeawad
PTAL.
5 years, 3 months ago (2014-07-18 23:13:56 UTC) #2
brianderson
Nice. https://codereview.chromium.org/401083002/diff/1/content/browser/power_monitor_message_broadcaster_unittest.cc File content/browser/power_monitor_message_broadcaster_unittest.cc (right): https://codereview.chromium.org/401083002/diff/1/content/browser/power_monitor_message_broadcaster_unittest.cc#newcode74 content/browser/power_monitor_message_broadcaster_unittest.cc:74: EXPECT_EQ(sender.power_state_changes(), 1); Does this check fail without your ...
5 years, 3 months ago (2014-07-18 23:15:06 UTC) #3
fmeawad
https://codereview.chromium.org/401083002/diff/1/content/browser/power_monitor_message_broadcaster_unittest.cc File content/browser/power_monitor_message_broadcaster_unittest.cc (right): https://codereview.chromium.org/401083002/diff/1/content/browser/power_monitor_message_broadcaster_unittest.cc#newcode74 content/browser/power_monitor_message_broadcaster_unittest.cc:74: EXPECT_EQ(sender.power_state_changes(), 1); On 2014/07/18 23:15:06, brianderson wrote: > Does ...
5 years, 3 months ago (2014-07-18 23:19:55 UTC) #4
cpu_(ooo_6.6-7.5)
https://codereview.chromium.org/401083002/diff/1/content/browser/power_monitor_message_broadcaster.cc File content/browser/power_monitor_message_broadcaster.cc (right): https://codereview.chromium.org/401083002/diff/1/content/browser/power_monitor_message_broadcaster.cc#newcode29 content/browser/power_monitor_message_broadcaster.cc:29: if (power_monitor) we don't care if we manage to ...
5 years, 2 months ago (2014-07-20 01:33:47 UTC) #5
jam
rubberstamp lgtm fix the misspellings in the description (intitial & broacasts)
5 years, 2 months ago (2014-07-21 17:08:23 UTC) #6
fmeawad
On 2014/07/18 22:35:51, bajones wrote: > This looks good, just one question about IPC behavior: ...
5 years, 2 months ago (2014-07-21 19:47:47 UTC) #7
fmeawad
Thanks jam@. cpu@, can you take another look? https://codereview.chromium.org/401083002/diff/1/content/browser/power_monitor_message_broadcaster.cc File content/browser/power_monitor_message_broadcaster.cc (right): https://codereview.chromium.org/401083002/diff/1/content/browser/power_monitor_message_broadcaster.cc#newcode29 content/browser/power_monitor_message_broadcaster.cc:29: if ...
5 years, 2 months ago (2014-07-21 19:54:29 UTC) #8
fmeawad
cpu@, ptal. https://codereview.chromium.org/401083002/diff/1/content/browser/power_monitor_message_broadcaster.cc File content/browser/power_monitor_message_broadcaster.cc (right): https://codereview.chromium.org/401083002/diff/1/content/browser/power_monitor_message_broadcaster.cc#newcode29 content/browser/power_monitor_message_broadcaster.cc:29: if (power_monitor) On 2014/07/21 19:54:28, fmeawad-cr wrote: ...
5 years, 2 months ago (2014-07-21 21:48:25 UTC) #9
cpu_(ooo_6.6-7.5)
lgtm
5 years, 2 months ago (2014-07-22 01:31:36 UTC) #10
cpu_(ooo_6.6-7.5)
The CQ bit was checked by cpu@chromium.org
5 years, 2 months ago (2014-07-22 01:31:38 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fmeawad@chromium.org/401083002/60001
5 years, 2 months ago (2014-07-22 01:34:17 UTC) #12
commit-bot: I haz the power
5 years, 2 months ago (2014-07-22 02:34:07 UTC) #13
Message was sent while issue was closed.
Change committed as 284599

Powered by Google App Engine
This is Rietveld 408576698