extensions: Introduce a fake ExtensionsClient implementation.

extensions: Introduce a fake ExtensionsClient implementation.

And hook it up in our test suite.

The following tests still fail or crash, but for other reasons other than g_client being NULL. Now they are more about FATAL:api_permission_set.cc(125)] Check failed: permission_info.

8 tests failed:
    ExtensionURLPatternTest.ConvertToExplicitSchemes
    ExtensionURLPatternTest.GetAsString
    ExtensionURLPatternTest.Match13
    ExtensionURLPatternTest.Match19
    ExtensionURLPatternTest.Match9
    ExtensionURLPatternTest.Ports
    FileUtilTest.ExtensionResourceURLToFilePath
    FileUtilTest.ExtensionURLToRelativeFilePath
9 tests crashed:
    APIPermissionSetTest.CreateDifference
    APIPermissionSetTest.CreateIntersection
    APIPermissionSetTest.CreateUnion
    APIPermissionSetTest.General
    APIPermissionSetTest.IPC
    APIPermissionSetTest.ImplicitPermissions
    ManifestHandlerTest.DependentHandlers
    ManifestHandlerTest.FailingHandlers
    ManifestHandlerTest.Validate

BUG=348066
TEST=extensions_unittests
R=jamescook@chromium.org,yoz@chromium.org
NOTRY=true

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

[tfarina, 2014-04-11 05:00:26]

This is ready for review now. 1:59 AM here, heh :/.

PTAL.

Thanks,

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
File extensions/test/test_permissions_provider.cc (right):

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
extensions/test/test_permissions_provider.cc:15: std::vector<APIPermissionInfo*>
permissions;
I will look into this next.

[James Cook, 2014-04-11 16:26:49]

Looking good, one question.

Yoz, we need to think again about how to provide either all or a subset of
permissions to the extensions module, as we're going to need a solution for both
these tests and shell_extensions_client.cc / http://crbug.com/339301

https://codereview.chromium.org/234563003/diff/20001/extensions/test/extensio...
File extensions/test/extensions_unittests_main.cc (right):

https://codereview.chromium.org/234563003/diff/20001/extensions/test/extensio...
extensions/test/extensions_unittests_main.cc:38: base::TestSuite::Shutdown();
Question: Do you want to explicitly client_.reset() and Set(NULL) above this
line?  I'm not sure when Shutdown() gets called relative to this object being
destroyed and that would make sure that the client can do cleanup before the
test framework starts tearing itself down.

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_ext...
File extensions/test/test_extensions_client.cc (right):

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_ext...
extensions/test/test_extensions_client.cc:67: return hosts;
nit: Can this be "return URLPatternSet();"?

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_ext...
File extensions/test/test_extensions_client.h (right):

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_ext...
extensions/test/test_extensions_client.h:19: virtual void Initialize() OVERRIDE;
nit: maybe "ExtensionsClient implementation" above this line?

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
File extensions/test/test_permission_message_provider.h (right):

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
extensions/test/test_permission_message_provider.h:19: virtual
PermissionMessages GetPermissionMessages(
nit: "PermissionMessageProvider implementation" above?

[Yoyo Zhou, 2014-04-11 21:10:29]

LGTM, I'll look into the design for permissions.

https://codereview.chromium.org/234563003/diff/20001/extensions/test/extensio...
File extensions/test/extensions_unittests_main.cc (right):

https://codereview.chromium.org/234563003/diff/20001/extensions/test/extensio...
extensions/test/extensions_unittests_main.cc:38: base::TestSuite::Shutdown();
On 2014/04/11 16:26:49, James Cook wrote:
> Question: Do you want to explicitly client_.reset() and Set(NULL) above this
> line?  I'm not sure when Shutdown() gets called relative to this object being
> destroyed and that would make sure that the client can do cleanup before the
> test framework starts tearing itself down.

I'm not sure it's necessary - for example, ChromeUnitTestSuite doesn't do
especial cleanup.

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
File extensions/test/test_permission_message_provider.cc (right):

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
extensions/test/test_permission_message_provider.cc:19: return messages;
nit: return PermissionMessages();

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
File extensions/test/test_permissions_provider.cc (right):

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
extensions/test/test_permissions_provider.cc:15: std::vector<APIPermissionInfo*>
permissions;
On 2014/04/11 05:00:26, tfarina wrote:
> I will look into this next.

Add a TODO here. That helps distinguish between intentionally-left-blank and
blank-but-working-on-it.

[tfarina, 2014-04-13 21:40:52]

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
File extensions/test/test_permission_message_provider.cc (right):

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
extensions/test/test_permission_message_provider.cc:19: return messages;
On 2014/04/11 21:10:29, Yoyo Zhou wrote:
> nit: return PermissionMessages();

Done.

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
File extensions/test/test_permissions_provider.cc (right):

https://codereview.chromium.org/234563003/diff/20001/extensions/test/test_per...
extensions/test/test_permissions_provider.cc:15: std::vector<APIPermissionInfo*>
permissions;
On 2014/04/11 21:10:29, Yoyo Zhou wrote:
> On 2014/04/11 05:00:26, tfarina wrote:
> > I will look into this next.
> 
> Add a TODO here. That helps distinguish between intentionally-left-blank and
> blank-but-working-on-it.

Done.

[tfarina, 2014-04-13 21:40:57]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2014-04-13 21:41:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/234563003/40001

[commit-bot: I haz the power, 2014-04-14 00:50:38]

Change committed as 263573