Update the Website Settings page when the power consumption is updated.

Update the Website Settings page when the origin power map for the
profile is updated.

BUG=409093
Committed: https://crrev.com/828da409d7ffa5a837187eedf77af3f867f5ee31
Cr-Commit-Position: refs/heads/master@{#293177}

[Daniel Nishi, 2014-08-29 18:27:37]

dhnishi@chromium.org changed reviewers:
+ bauerb@chromium.org, derat@chromium.org

[Daniel Nishi, 2014-08-29 18:27:38]

derat: PTAL at c/b/power/process_power_collector.cc and components/power/*.

bauerb: PTAL at c/b/ui/webui/options/*.

[Bernhard Bauer, 2014-08-29 19:16:31]

LGTM w/ a nit and a suggestion:

https://codereview.chromium.org/519883002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/website_settings_handler.cc (left):

https://codereview.chromium.org/519883002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/website_settings_handler.cc:34: 
Instead of removing this blank line, I would rather add one before the end of
the namespace.

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
File components/power/origin_power_map_observer.h (right):

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
components/power/origin_power_map_observer.h:13: virtual void
OnPowerConsumptionUpdated() = 0;
An alternative to a one-method interface is to use base::SubscriptionList and
callbacks.

[Daniel Erat, 2014-08-29 21:14:02]

https://codereview.chromium.org/519883002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/website_settings_handler.cc (left):

https://codereview.chromium.org/519883002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/website_settings_handler.cc:34: 
On 2014/08/29 19:16:31, Bernhard Bauer wrote:
> Instead of removing this blank line, I would rather add one before the end of
> the namespace.

(i concur)

https://codereview.chromium.org/519883002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/website_settings_handler.cc (right):

https://codereview.chromium.org/519883002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/website_settings_handler.cc:50: :
observer_(this), power_observer_(this), weak_ptr_factory_(this) {
nit: chromium style is usually one member per line if they don't all fit on the
same line as the signature, i think

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
File components/power/origin_power_map.h (right):

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
components/power/origin_power_map.h:44: void OnPowerConsumptionUpdated();
mind giving this a different name from the OriginPowerMapObserver method to make
it less likely that people will get confused? maybe something like
"OnUpdatesDone" or "OnAllOriginsUpdated"? this also hopefully makes it clearer
that the entire round of updates is done, rather than e.g. this being called
after each AddPowerForOrigin call.

[Daniel Nishi, 2014-09-02 16:30:30]

https://codereview.chromium.org/519883002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/website_settings_handler.cc (left):

https://codereview.chromium.org/519883002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/website_settings_handler.cc:34: 
On 2014/08/29 19:16:31, Bernhard Bauer wrote:
> Instead of removing this blank line, I would rather add one before the end of
> the namespace.

Done.

https://codereview.chromium.org/519883002/diff/20001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/website_settings_handler.cc (right):

https://codereview.chromium.org/519883002/diff/20001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/website_settings_handler.cc:50: :
observer_(this), power_observer_(this), weak_ptr_factory_(this) {
On 2014/08/29 21:14:02, Daniel Erat wrote:
> nit: chromium style is usually one member per line if they don't all fit on
the
> same line as the signature, i think

Done.

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
File components/power/origin_power_map.h (right):

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
components/power/origin_power_map.h:44: void OnPowerConsumptionUpdated();
On 2014/08/29 21:14:02, Daniel Erat wrote:
> mind giving this a different name from the OriginPowerMapObserver method to
make
> it less likely that people will get confused? maybe something like
> "OnUpdatesDone" or "OnAllOriginsUpdated"? this also hopefully makes it clearer
> that the entire round of updates is done, rather than e.g. this being called
> after each AddPowerForOrigin call.

Done.

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
File components/power/origin_power_map_observer.h (right):

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
components/power/origin_power_map_observer.h:13: virtual void
OnPowerConsumptionUpdated() = 0;
On 2014/08/29 19:16:31, Bernhard Bauer wrote:
> An alternative to a one-method interface is to use base::SubscriptionList and
> callbacks.

Would this require weak ptr factories for everyone who would go onto the
SubscriberList, though?

[Bernhard Bauer, 2014-09-02 16:39:57]

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
File components/power/origin_power_map_observer.h (right):

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
components/power/origin_power_map_observer.h:13: virtual void
OnPowerConsumptionUpdated() = 0;
On 2014/09/02 16:30:30, Daniel Nishi wrote:
> On 2014/08/29 19:16:31, Bernhard Bauer wrote:
> > An alternative to a one-method interface is to use base::SubscriptionList
and
> > callbacks.
> 
> Would this require weak ptr factories for everyone who would go onto the
> SubscriberList, though?

No, because the observer would destroy its SubscriptionList::Subscription when
it is destroyed itself, which unregisters.

[Daniel Nishi, 2014-09-02 17:10:09]

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
File components/power/origin_power_map_observer.h (right):

https://codereview.chromium.org/519883002/diff/20001/components/power/origin_...
components/power/origin_power_map_observer.h:13: virtual void
OnPowerConsumptionUpdated() = 0;
On 2014/09/02 16:39:57, Bernhard Bauer wrote:
> On 2014/09/02 16:30:30, Daniel Nishi wrote:
> > On 2014/08/29 19:16:31, Bernhard Bauer wrote:
> > > An alternative to a one-method interface is to use base::SubscriptionList
> and
> > > callbacks.
> > 
> > Would this require weak ptr factories for everyone who would go onto the
> > SubscriberList, though?
> 
> No, because the observer would destroy its SubscriptionList::Subscription when
> it is destroyed itself, which unregisters.

Ah, I understand.

I've updated the implementation to use a Subscription instead.

[Bernhard Bauer, 2014-09-02 17:11:47]

Thank you!

[Daniel Erat, 2014-09-02 17:14:40]

lgtm

https://codereview.chromium.org/519883002/diff/80001/components/power/origin_...
File components/power/origin_power_map.h (right):

https://codereview.chromium.org/519883002/diff/80001/components/power/origin_...
components/power/origin_power_map.h:38: // Adds a callback for when the power
consumption for the map finishes
nit: maybe something like "Adds a callback for the completion of a round of
updates to |origin_map_|."

[Daniel Nishi, 2014-09-02 17:32:49]

https://codereview.chromium.org/519883002/diff/80001/components/power/origin_...
File components/power/origin_power_map.h (right):

https://codereview.chromium.org/519883002/diff/80001/components/power/origin_...
components/power/origin_power_map.h:38: // Adds a callback for when the power
consumption for the map finishes
On 2014/09/02 17:14:39, Daniel Erat wrote:
> nit: maybe something like "Adds a callback for the completion of a round of
> updates to |origin_map_|."

Done.

[Daniel Nishi, 2014-09-02 17:32:52]

The CQ bit was checked by dhnishi@chromium.org

[commit-bot: I haz the power, 2014-09-02 17:33:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dhnishi@chromium.org/519883002/100001

[commit-bot: I haz the power, 2014-09-02 18:32:21]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-09-02 19:01:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-02 19:01:07]

Try jobs failed on following builders:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Daniel Nishi, 2014-09-03 17:49:34]

The CQ bit was checked by dhnishi@chromium.org

[commit-bot: I haz the power, 2014-09-03 17:50:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dhnishi@chromium.org/519883002/140001

[commit-bot: I haz the power, 2014-09-03 18:44:23]

Committed patchset #6 (id:140001) as 47eafae382b66f5ec83eb1625fa03158cb309ccb

[commit-bot: I haz the power, 2014-09-10 03:27:06]

Patchset 6 (id:??) landed as
https://crrev.com/828da409d7ffa5a837187eedf77af3f867f5ee31
Cr-Commit-Position: refs/heads/master@{#293177}