Fix crash in test.

Fix crash in test.

BUG=107246
TEST=fixed


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114111

[Matt Perry, 2011-12-12 20:00:22]

http://codereview.chromium.org/8921016/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_service_unittest.cc (right):

http://codereview.chromium.org/8921016/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_service_unittest.cc:3606:
PluginService::GetInstance()->SetPluginListForTesting(&plugin_list);
Seems like this would fix the crash you saw, but I'm curious why
PluginService::RefreshPlugins is even getting called. We should only do that if
there is an extension installed with an NPAPI plugin. This test looks like it
doesn't even load any extensions.

Do we have a new component extension with an NPAPI plugin?

[miket_OOO, 2011-12-12 21:18:21]

http://codereview.chromium.org/8921016/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_service_unittest.cc (right):

http://codereview.chromium.org/8921016/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_service_unittest.cc:3606:
PluginService::GetInstance()->SetPluginListForTesting(&plugin_list);
On 2011/12/12 20:00:22, Matt Perry wrote:
> Seems like this would fix the crash you saw, but I'm curious why
> PluginService::RefreshPlugins is even getting called. We should only do that
if
> there is an extension installed with an NPAPI plugin. This test looks like it
> doesn't even load any extensions.
> 
> Do we have a new component extension with an NPAPI plugin?

I know why this is happening. I have
out/Debug/default_apps/external_extensions.json referring to a not-so-weird
extension (an internal Google thing), and it looks like this test is picking it
up. So unit tests are not as hermetic as we'd like. I think my fix is the right
fix, because a developer could legitimately have this JSON file sitting around
while running tests.

[Matt Perry, 2011-12-12 21:34:32]

LGTM, I'm OK with this fix. I think the following solutions would be even
better:
1. more generic way to init the plugin service for tests, since this could
happen on other tests too (I assume), or even better:
2. ensure unit tests like this are more hermetic, so they aren't affected by the
presence of an external_extensions.json

but this looks good for fixing the crash.

[commit-bot: I haz the power, 2011-12-12 21:37:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miket@chromium.org/8921016/1

[miket_OOO, 2011-12-12 21:37:41]

I'll file an issue about the more general solution.

[miket_OOO, 2011-12-12 21:46:54]

On 2011/12/12 21:37:41, miket wrote:
> I'll file an issue about the more general solution.

http://code.google.com/p/chromium/issues/detail?id=107263

[commit-bot: I haz the power, 2011-12-12 23:12:22]

Change committed as 114111