Don't try to reload blacklisted extensions.

Don't try to reload blacklisted extensions.

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

[Marijn Kruisselbrink, 2014-05-19 20:10:31]

https://codereview.chromium.org/291603002/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/extension_service.h (right):

https://codereview.chromium.org/291603002/diff/40001/chrome/browser/extension...
chrome/browser/extensions/extension_service.h:196: void ReloadExtension(const
std::string& extension_id);
Is this safe? Presumably the same comment as in UninstallExtension below applies
("We pass the |extension_id| by value to avoid having it deleted from under us
incase someone calls it with Extension::id() or another string that we are going
to delete in this function.")

Although it seems somebody recently (incorrectly?) changed the
UninstallExtension paramtere to be a reference in
https://src.chromium.org/viewvc/chrome?revision=267343&view=revision without
updating the comment.

[not at google - send to devlin, 2014-05-19 20:32:09]

https://codereview.chromium.org/291603002/diff/40001/chrome/browser/extension...
File chrome/browser/extensions/extension_service.h (right):

https://codereview.chromium.org/291603002/diff/40001/chrome/browser/extension...
chrome/browser/extensions/extension_service.h:196: void ReloadExtension(const
std::string& extension_id);
On 2014/05/19 20:10:32, Marijn Kruisselbrink wrote:
> Is this safe? Presumably the same comment as in UninstallExtension below
applies
> ("We pass the |extension_id| by value to avoid having it deleted from under us
> incase someone calls it with Extension::id() or another string that we are
going
> to delete in this function.")
> 
> Although it seems somebody recently (incorrectly?) changed the
> UninstallExtension paramtere to be a reference in
> https://src.chromium.org/viewvc/chrome?revision=267343&view=revision without
> updating the comment.

Ah. Good point, very subtle.

"const std::string" without the reference is nonsensical and confusing. I don't
think it's great as a concern of the interface to this class. Seems kind of
flaky. I'll update the implementations to be safe in this regard.

[Marijn Kruisselbrink, 2014-05-19 20:36:49]

lgtm

[not at google - send to devlin, 2014-05-19 20:46:50]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-05-19 20:48:19]

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

[commit-bot: I haz the power, 2014-05-20 02:50:06]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-05-20 03:22:24]

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

[commit-bot: I haz the power, 2014-05-20 03:22:24]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)

[not at google - send to devlin, 2014-05-20 16:10:32]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-05-20 16:10:58]

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

[commit-bot: I haz the power, 2014-05-20 20:20:11]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-20 20:41:06]

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

[commit-bot: I haz the power, 2014-05-20 20:41:06]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[not at google - send to devlin, 2014-05-22 15:06:50]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-05-22 15:08:50]

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

[commit-bot: I haz the power, 2014-05-22 15:14:23]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-22 15:19:04]

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

[commit-bot: I haz the power, 2014-05-22 15:19:05]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[not at google - send to devlin, 2014-05-22 15:40:07]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-05-22 15:42:54]

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

[commit-bot: I haz the power, 2014-05-22 15:49:31]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-22 15:53:39]

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

[commit-bot: I haz the power, 2014-05-22 15:53:39]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[not at google - send to devlin, 2014-05-22 19:18:56]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-05-22 19:20:46]

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

[commit-bot: I haz the power, 2014-05-22 19:51:06]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[commit-bot: I haz the power, 2014-05-22 19:56:33]

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

[commit-bot: I haz the power, 2014-05-22 19:56:34]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[not at google - send to devlin, 2014-05-22 20:55:21]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-05-22 20:55:48]

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

[not at google - send to devlin, 2014-05-22 21:29:49]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-05-22 21:31:43]

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

[commit-bot: I haz the power, 2014-05-23 00:11:47]

Change committed as 272378