Sanity tweaks to the extension blacklist: check all extensions at once on

Sanity tweaks to the extension blacklist:
- check all extensions at once on startup (checking one at a time was
  thrashing the message loop),
- check before installing rather than after (prevent blacklisted extensions
  from ever being installed), and
- show an infobar ala the policy blacklist warning when an extension is
  blacklisted.


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181859

[Matt Perry, 2013-02-07 01:05:35]

https://codereview.chromium.org/12211029/diff/2003/chrome/browser/extensions/...
File chrome/browser/extensions/extension_service.cc (right):

https://codereview.chromium.org/12211029/diff/2003/chrome/browser/extensions/...
chrome/browser/extensions/extension_service.cc:2054: void
ExtensionService::AddExtensions(const ExtensionList& extensions) {
Rather than adding a new API, couldn't you instead defer the blacklist check to
OnLoadedInstalledExtensions() if ready_ is false?

https://codereview.chromium.org/12211029/diff/2003/chrome/browser/extensions/...
File chrome/browser/extensions/extension_service.h (left):

https://codereview.chromium.org/12211029/diff/2003/chrome/browser/extensions/...
chrome/browser/extensions/extension_service.h:443: void OnExtensionInstalled(
After thinking about it, I can see the reasoning behind this name. The extension
has already been unpacked onto disk into the installation dir. We're just
informing the ExtensionService to add it to the install list.

How about HandleExtensionInstall and HandleExtensionInstallAsync?

[Matt Perry, 2013-02-07 01:10:23]

https://codereview.chromium.org/12211029/diff/2003/chrome/browser/extensions/...
File chrome/browser/extensions/extension_service.h (left):

https://codereview.chromium.org/12211029/diff/2003/chrome/browser/extensions/...
chrome/browser/extensions/extension_service.h:443: void OnExtensionInstalled(
On 2013/02/07 01:05:35, Matt Perry wrote:
> After thinking about it, I can see the reasoning behind this name. The
extension
> has already been unpacked onto disk into the installation dir. We're just
> informing the ExtensionService to add it to the install list.
> 
> How about HandleExtensionInstall and HandleExtensionInstallAsync?

Actually, lets just make it explicit. Keep OnExtensionInstalled, and add a
CheckExtensionBlacklist method with success/failure callbacks.

[not at google - send to devlin, 2013-02-07 02:11:37]

ok, lots of changes. You might want to just re-review since the patch is much
smaller now anyway.

https://codereview.chromium.org/12211029/diff/2003/chrome/browser/extensions/...
File chrome/browser/extensions/extension_service.cc (right):

https://codereview.chromium.org/12211029/diff/2003/chrome/browser/extensions/...
chrome/browser/extensions/extension_service.cc:2054: void
ExtensionService::AddExtensions(const ExtensionList& extensions) {
On 2013/02/07 01:05:35, Matt Perry wrote:
> Rather than adding a new API, couldn't you instead defer the blacklist check
to
> OnLoadedInstalledExtensions() if ready_ is false?

Done.

https://codereview.chromium.org/12211029/diff/2003/chrome/browser/extensions/...
File chrome/browser/extensions/extension_service.h (left):

https://codereview.chromium.org/12211029/diff/2003/chrome/browser/extensions/...
chrome/browser/extensions/extension_service.h:443: void OnExtensionInstalled(
On 2013/02/07 01:10:23, Matt Perry wrote:
> On 2013/02/07 01:05:35, Matt Perry wrote:
> > After thinking about it, I can see the reasoning behind this name. The
> extension
> > has already been unpacked onto disk into the installation dir. We're just
> > informing the ExtensionService to add it to the install list.
> > 
> > How about HandleExtensionInstall and HandleExtensionInstallAsync?
> 
> Actually, lets just make it explicit. Keep OnExtensionInstalled, and add a
> CheckExtensionBlacklist method with success/failure callbacks.

Done.

[Matt Perry, 2013-02-07 02:16:33]

Yay, much simpler. LGTM

https://codereview.chromium.org/12211029/diff/13/chrome/browser/extensions/bl...
File chrome/browser/extensions/blacklist.cc (right):

https://codereview.chromium.org/12211029/diff/13/chrome/browser/extensions/bl...
chrome/browser/extensions/blacklist.cc:189: }  // namespace
nit: I think this might be against the style guide. I would move it up to the
top of the file with the rest of the anon namespace stuff.

[not at google - send to devlin, 2013-02-07 02:26:58]

https://codereview.chromium.org/12211029/diff/13/chrome/browser/extensions/bl...
File chrome/browser/extensions/blacklist.cc (right):

https://codereview.chromium.org/12211029/diff/13/chrome/browser/extensions/bl...
chrome/browser/extensions/blacklist.cc:189: }  // namespace
On 2013/02/07 02:16:33, Matt Perry wrote:
> nit: I think this might be against the style guide. I would move it up to the
> top of the file with the rest of the anon namespace stuff.

Done.

[commit-bot: I haz the power, 2013-02-07 02:40:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/12211029/7006

[commit-bot: I haz the power, 2013-02-07 03:21:24]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-02-11 17:00:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/12211029/3020

[commit-bot: I haz the power, 2013-02-11 17:27:04]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-02-11 17:45:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/12211029/10028

[commit-bot: I haz the power, 2013-02-11 18:18:37]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-02-11 20:01:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/12211029/10029

[commit-bot: I haz the power, 2013-02-11 20:46:56]

Retried try job too often on linux_aura for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-02-11 21:02:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/12211029/12014

[commit-bot: I haz the power, 2013-02-12 00:34:53]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-02-12 01:08:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kalman@chromium.org/12211029/12014

[commit-bot: I haz the power, 2013-02-12 04:57:30]

Change committed as 181859