[Extensions] Move remaining notifications out of ExtensionSettingsHandler

[Extensions] Move remaining notifications out of ExtensionSettingsHandler

Move the remaining notifications from ExtensionSettingsHandler to the
DeveloperPrivate API.
Also adjust the WarningService observer method to include a set of modified
extensions.

BUG=461039
Committed: https://crrev.com/7dd0529036b94a81b1812ea7f98afec89528975b
Cr-Commit-Position: refs/heads/master@{#330633}

[Devlin, 2015-05-19 20:09:01]

rdevlin.cronin@chromium.org changed reviewers:
+ kalman@chromium.org

[Devlin, 2015-05-19 20:09:01]

ExtensionSettingsHandler is now quite small.  Woohoo!

[not at google - send to devlin, 2015-05-19 20:35:05]

lgtm

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:166:
!info->is_supervised &&
I wonder why supervised users are called out here. Marc, why can't they use
developer mode?

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:327:
args->Append(dict.release());
I'd prefer inlining |dict| as just CreateProfileInfo(profile_)->ToValue(), not
because I'm pedantic, but because after this line |dict| is null and if anybody
tries to use it then bad things will happen. Inlining is both cleaner and avoid
this problem.

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:336:
BroadcastItemStateChanged(developer::EVENT_TYPE_WARNINGS_CHANGED, id);
These event broadcasts in loops make me sad. We should be coalescing them. Oh
well.

https://codereview.chromium.org/1146693004/diff/1/extensions/browser/warning_...
File extensions/browser/warning_service.h (right):

https://codereview.chromium.org/1146693004/diff/1/extensions/browser/warning_...
extensions/browser/warning_service.h:42: const std::set<std::string>&
affected_extensions) = 0;
I'd like to pass around an ExtensionSet, but I see we're pigeonholed here by the
way the warning service is factored. Not worth the changes required to use real
Extensions rather than their IDs. In lieu of that, we can pass around an
ExtensionIdSet?

[Devlin, 2015-05-19 22:01:08]

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:166:
!info->is_supervised &&
On 2015/05/19 20:35:04, kalman wrote:
> I wonder why supervised users are called out here. Marc, why can't they use
> developer mode?

(Since this isn't a behavior change, I'll commit now, but still good to hear
what Marc says.)

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:327:
args->Append(dict.release());
On 2015/05/19 20:35:04, kalman wrote:
> I'd prefer inlining |dict| as just CreateProfileInfo(profile_)->ToValue(), not
> because I'm pedantic, but because after this line |dict| is null and if
anybody
> tries to use it then bad things will happen. Inlining is both cleaner and
avoid
> this problem.

I'm not sure I buy your safety argument, but I like inlining things anyway, so
done. ;)

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:336:
BroadcastItemStateChanged(developer::EVENT_TYPE_WARNINGS_CHANGED, id);
On 2015/05/19 20:35:04, kalman wrote:
> These event broadcasts in loops make me sad. We should be coalescing them. Oh
> well.

We should be, but that's a much larger change.  And, more importantly, the
chance of warnings being added for multiple extensions is pretty low.  Lower yet
for it to happen when the user just so happens to have chrome://extensions open.
 So we need to do it, because of The Law of a Billion Users, but don't agonize.
:)

https://codereview.chromium.org/1146693004/diff/1/extensions/browser/warning_...
File extensions/browser/warning_service.h (right):

https://codereview.chromium.org/1146693004/diff/1/extensions/browser/warning_...
extensions/browser/warning_service.h:42: const std::set<std::string>&
affected_extensions) = 0;
On 2015/05/19 20:35:05, kalman wrote:
> I'd like to pass around an ExtensionSet, but I see we're pigeonholed here by
the
> way the warning service is factored. Not worth the changes required to use
real
> Extensions rather than their IDs. In lieu of that, we can pass around an
> ExtensionIdSet?

Sure, why not.

[Devlin, 2015-05-19 22:01:13]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2015-05-19 22:01:13]

The patchset sent to the CQ was uploaded after l-g-t-m from kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1146693004/#ps20001
(title: "Ben's")

[commit-bot: I haz the power, 2015-05-19 22:02:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1146693004/20001

[Devlin, 2015-05-19 22:08:53]

The CQ bit was unchecked by rdevlin.cronin@chromium.org

[Devlin, 2015-05-19 22:09:20]

rdevlin.cronin@chromium.org changed reviewers:
+ dbeam@chromium.org

[Devlin, 2015-05-19 22:09:21]

+Dan for third_party/closure_compiler/

[Dan Beam, 2015-05-19 22:13:40]

lgtm <3 auto generated externs

[Devlin, 2015-05-19 22:14:32]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2015-05-19 22:16:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1146693004/20001

[commit-bot: I haz the power, 2015-05-19 22:23:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-19 22:24:22]

Patchset 2 (id:??) landed as
https://crrev.com/7dd0529036b94a81b1812ea7f98afec89528975b
Cr-Commit-Position: refs/heads/master@{#330633}

[Marc Treib, 2015-05-20 08:07:26]

treib@chromium.org changed reviewers:
+ treib@chromium.org

[Marc Treib, 2015-05-20 08:07:27]

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:166:
!info->is_supervised &&
On 2015/05/19 22:01:08, Devlin wrote:
> On 2015/05/19 20:35:04, kalman wrote:
> > I wonder why supervised users are called out here. Marc, why can't they use
> > developer mode?
> 
> (Since this isn't a behavior change, I'll commit now, but still good to hear
> what Marc says.)

This is from before my time, so I might be missing things here :)

Supervised users aren't allowed to install extensions, and developer mode would
allow them to sideload extensions. In fact, until quite recently, they couldn't
use extensions at all, so I guess there was just no point to developer mode. We
might want to revisit this decision now...

[not at google - send to devlin, 2015-05-20 21:32:32]

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/developer_private/developer_private_api.cc
(right):

https://codereview.chromium.org/1146693004/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/developer_private/developer_private_api.cc:166:
!info->is_supervised &&
On 2015/05/20 08:07:27, Marc Treib wrote:
> On 2015/05/19 22:01:08, Devlin wrote:
> > On 2015/05/19 20:35:04, kalman wrote:
> > > I wonder why supervised users are called out here. Marc, why can't they
use
> > > developer mode?
> > 
> > (Since this isn't a behavior change, I'll commit now, but still good to hear
> > what Marc says.)
> 
> This is from before my time, so I might be missing things here :)
> 
> Supervised users aren't allowed to install extensions, and developer mode
would
> allow them to sideload extensions. In fact, until quite recently, they
couldn't
> use extensions at all, so I guess there was just no point to developer mode.
We
> might want to revisit this decision now...

Yeah seems like a perfectly reasonable thing for supervised users to do... write
extensions :-)