Initialize PowerMonitor on_power_battery intitial value for newly created processes.

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

[bajones, 2014-07-18 22:35:51]

This looks good, just one question about IPC behavior: If the child process
hasn't finished initializing before PowerMonitorBroadcaster::OnPowerStateChange
is called the resulting message will be queued until the child process is ready,
correct? Assuming that's not a race condition I think this will work great.

[fmeawad, 2014-07-18 23:13:56]

PTAL.

[brianderson, 2014-07-18 23:15:06]

Nice.

https://codereview.chromium.org/401083002/diff/1/content/browser/power_monito...
File content/browser/power_monitor_message_broadcaster_unittest.cc (right):

https://codereview.chromium.org/401083002/diff/1/content/browser/power_monito...
content/browser/power_monitor_message_broadcaster_unittest.cc:74:
EXPECT_EQ(sender.power_state_changes(), 1);
Does this check fail without your patch?

[fmeawad, 2014-07-18 23:19:55]

https://codereview.chromium.org/401083002/diff/1/content/browser/power_monito...
File content/browser/power_monitor_message_broadcaster_unittest.cc (right):

https://codereview.chromium.org/401083002/diff/1/content/browser/power_monito...
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 this check fail without your patch?
Without calling Init, no power_state_changes are propagated, so the count stays
at 0. Short answer yes.

[cpu_(ooo_6.6-7.5), 2014-07-20 01:33:47]

https://codereview.chromium.org/401083002/diff/1/content/browser/power_monito...
File content/browser/power_monitor_message_broadcaster.cc (right):

https://codereview.chromium.org/401083002/diff/1/content/browser/power_monito...
content/browser/power_monitor_message_broadcaster.cc:29: if (power_monitor)
we don't care if we manage to call IsOnBatteryPower() or not? in what conditions
would it fail?

If it should never fail to obtain a |power_monitor| then we want to CHECK()
here, if we could fail then an explanation of when or why is should be here.

[jam, 2014-07-21 17:08:23]

rubberstamp lgtm

fix the misspellings in the description (intitial & broacasts)

[fmeawad, 2014-07-21 19:47:47]

On 2014/07/18 22:35:51, bajones wrote:
> This looks good, just one question about IPC behavior: If the child process
> hasn't finished initializing before
PowerMonitorBroadcaster::OnPowerStateChange
> is called the resulting message will be queued until the child process is
ready,
> correct? Assuming that's not a race condition I think this will work great.

From my understanding, The channel is always initialized before the call,
otherwise it would not work at all. So no race conditions here.

[fmeawad, 2014-07-21 19:54:29]

Thanks jam@.

cpu@, can you take another look?

https://codereview.chromium.org/401083002/diff/1/content/browser/power_monito...
File content/browser/power_monitor_message_broadcaster.cc (right):

https://codereview.chromium.org/401083002/diff/1/content/browser/power_monito...
content/browser/power_monitor_message_broadcaster.cc:29: if (power_monitor)
On 2014/07/20 01:33:46, cpu wrote:
> we don't care if we manage to call IsOnBatteryPower() or not? in what
conditions
> would it fail?
> 
> If it should never fail to obtain a |power_monitor| then we want to CHECK()
> here, if we could fail then an explanation of when or why is should be here.

The CHECK will not be useful here, since there is no code  between the use and
the check, removed the condition.

I looked at where PowerMonitor is initialized, and everywhere the
MessageBroadcaster is currently used, the PowerMonitor is already initialized.

[fmeawad, 2014-07-21 21:48:25]

cpu@, ptal.

https://codereview.chromium.org/401083002/diff/1/content/browser/power_monito...
File content/browser/power_monitor_message_broadcaster.cc (right):

https://codereview.chromium.org/401083002/diff/1/content/browser/power_monito...
content/browser/power_monitor_message_broadcaster.cc:29: if (power_monitor)
On 2014/07/21 19:54:28, fmeawad-cr wrote:
> On 2014/07/20 01:33:46, cpu wrote:
> > we don't care if we manage to call IsOnBatteryPower() or not? in what
> conditions
> > would it fail?
> > 
> > If it should never fail to obtain a |power_monitor| then we want to CHECK()
> > here, if we could fail then an explanation of when or why is should be here.
> 
> The CHECK will not be useful here, since there is no code  between the use and
> the check, removed the condition.
> 
> I looked at where PowerMonitor is initialized, and everywhere the
> MessageBroadcaster is currently used, the PowerMonitor is already initialized.

Actually, I was wrong, unit_tests does not initialize the PowerMonitor, and many
of them run on an IO thread. I restored the condition and added a comment.

[cpu_(ooo_6.6-7.5), 2014-07-22 01:31:36]

lgtm

[cpu_(ooo_6.6-7.5), 2014-07-22 01:31:38]

The CQ bit was checked by cpu@chromium.org

[commit-bot: I haz the power, 2014-07-22 01:34:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fmeawad@chromium.org/401083002/60001

[commit-bot: I haz the power, 2014-07-22 02:34:07]

Change committed as 284599