Add more test cases to enterprise.platformKeys API test.

Add more test cases to enterprise.platformKeys API test.

BUG=NONE

Committed: https://crrev.com/f2a63b4103928bdc637068e70ec7921f033f4465
Cr-Commit-Position: refs/heads/master@{#295503}

[pneubeck (no reviews), 2014-09-18 14:07:05]

pneubeck@chromium.org changed reviewers:
+ joaodasilva@chromium.org

[pneubeck (no reviews), 2014-09-18 14:07:05]

ptal

[pneubeck (no reviews), 2014-09-18 14:17:54]

fixed a small error

[Joao da Silva, 2014-09-18 14:31:21]

The crx is changed but the source files in
	chrome/test/data/extensions/api_test/enterprise_platform_keys aren't. Why is
that?

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
File
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc
(right):

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:137:
enum SYSTEM_TOKEN { SYSTEM_TOKEN_EXISTS, SYSTEM_TOKEN_NOT_EXISTS };
SystemToken

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:138:
enum DEVICE_STATUS { DEVICE_STATUS_ENROLLED, DEVICE_STATUS_NOT_ENROLLED  };
Break each constants into its own line

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:138:
enum DEVICE_STATUS { DEVICE_STATUS_ENROLLED, DEVICE_STATUS_NOT_ENROLLED  };
DeviceStatus

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:139:
enum USER_AFFILIATION {
UserAffiliation

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:172:
chromeos::login::kStubUser);
I don't understand how this switch affects USER_AFFILIATION_ENROLLED_DOMAIN.
Maybe this is worth having a comment?

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:184:
device_policy_test_helper_.MarkAsEnterpriseOwned();
OK maybe it's because of this part... but don't we always need a kLoginUser,
even if not enrolled?

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:229:
ExtensionApiTest::SetUpOnMainThread();
SetUp again? :-)

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:346:
USER_AFFILIATION_ENROLLED_DOMAIN)));
I'm not sure about this but I think structs can be initialized with values for
their fields in { }. If that works then you don't need a constructor for Params,
and you can just pass values like this here:

::testing::Values( { SYSTEM_TOKEN_EXISTS,
                     DEVICE_STATUS_ENROLLED,
                     USER_AFFILIATION_ENROLLED_DOMAIN },
                           { ... });

[pneubeck (no reviews), 2014-09-18 15:00:40]

I don't know why the crx changes when I re-package, but I'd like to keep it
instead of letting it potentially being out of sync.
I should really remove it from the repository and add it to the build scripts.

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
File
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc
(right):

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:137:
enum SYSTEM_TOKEN { SYSTEM_TOKEN_EXISTS, SYSTEM_TOKEN_NOT_EXISTS };
On 2014/09/18 14:31:21, Joao da Silva wrote:
> SystemToken

Done.

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:138:
enum DEVICE_STATUS { DEVICE_STATUS_ENROLLED, DEVICE_STATUS_NOT_ENROLLED  };
On 2014/09/18 14:31:20, Joao da Silva wrote:
> Break each constants into its own line

That's 'git cl format'

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:138:
enum DEVICE_STATUS { DEVICE_STATUS_ENROLLED, DEVICE_STATUS_NOT_ENROLLED  };
On 2014/09/18 14:31:20, Joao da Silva wrote:
> DeviceStatus

Done.

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:139:
enum USER_AFFILIATION {
On 2014/09/18 14:31:21, Joao da Silva wrote:
> UserAffiliation

Done.

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:172:
chromeos::login::kStubUser);
On 2014/09/18 14:31:20, Joao da Silva wrote:
> I don't understand how this switch affects USER_AFFILIATION_ENROLLED_DOMAIN.
> Maybe this is worth having a comment?


clarified

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:184:
device_policy_test_helper_.MarkAsEnterpriseOwned();
On 2014/09/18 14:31:21, Joao da Silva wrote:
> OK maybe it's because of this part... but don't we always need a kLoginUser,
> even if not enrolled?

there's a default. I changed it to always set it for clarity and not to depend
on that default.

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:229:
ExtensionApiTest::SetUpOnMainThread();
On 2014/09/18 14:31:20, Joao da Silva wrote:
> SetUp again? :-)

Done.

https://codereview.chromium.org/582723002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:346:
USER_AFFILIATION_ENROLLED_DOMAIN)));
On 2014/09/18 14:31:20, Joao da Silva wrote:
> I'm not sure about this but I think structs can be initialized with values for
> their fields in { }. If that works then you don't need a constructor for
Params,
> and you can just pass values like this here:
> 
> ::testing::Values( { SYSTEM_TOKEN_EXISTS,
>                      DEVICE_STATUS_ENROLLED,
>                      USER_AFFILIATION_ENROLLED_DOMAIN },
>                            { ... });

not without C++11 :-/

[Joao da Silva, 2014-09-18 15:26:33]

lgtm

Why did you repackage if the files weren't modified? If there are no modified
files then the CRX shouldn't need to be repackaged. I may be missing something
here.

https://codereview.chromium.org/582723002/diff/60001/chrome/browser/extension...
File
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc
(right):

https://codereview.chromium.org/582723002/diff/60001/chrome/browser/extension...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:153:
Params(SystemToken SystemToken,
arg name should be system_token, etc

[pneubeck (no reviews), 2014-09-18 15:33:55]

I temporarily had changes in the test extension.
After removing these changes and repackaging git showed still binary
differences.
Not sure why, maybe there is some counter/date OR there were changes before and
the .crx was not repackaged.

I can also do that in a separate CL. removed the .crx.

https://codereview.chromium.org/582723002/diff/60001/chrome/browser/extension...
File
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc
(right):

https://codereview.chromium.org/582723002/diff/60001/chrome/browser/extension...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc:153:
Params(SystemToken SystemToken,
On 2014/09/18 15:26:32, Joao da Silva wrote:
> arg name should be system_token, etc

hmpf. 'set ignorecase' :/

[pneubeck (no reviews), 2014-09-18 18:06:52]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2014-09-18 18:07:27]

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

[commit-bot: I haz the power, 2014-09-18 18:35:47]

Committed patchset #5 (id:80001) as 9972ff45466af59db40b4dbfcf0b939bccd8b006

[commit-bot: I haz the power, 2014-09-18 18:36:23]

Patchset 5 (id:??) landed as
https://crrev.com/f2a63b4103928bdc637068e70ec7921f033f4465
Cr-Commit-Position: refs/heads/master@{#295503}