Add tests for CWS restriction of Nacl plugin invocation.

Add tests for CWS restriction of NaCl plugin.
BUG=none
TEST=this

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

[bbudge, 2011-08-05 18:40:10]

This patch tests the CWS restriction for NaCl.

http://codereview.chromium.org/7488060/diff/8007/chrome/test/data/extensions/...
File chrome/test/data/extensions/native_client/manifest.json (right):

http://codereview.chromium.org/7488060/diff/8007/chrome/test/data/extensions/...
chrome/test/data/extensions/native_client/manifest.json:2: "key":
"MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCp40PYAXfSDlzCW1f5MDzRW64h0YhgV7MX8Frem0vO1ZAlq/mlUO4KxwkF2AZliFScO4Cc3CYpO6jpHXwz27tUwaN46C/LzYO7u/kb2piOep8gClvZ64EMnDv5PIoIeZlOJhkpnfs/5FbQt5sqT9avXX7YfLCLBJBql0U/V5615wIDAQAB",
I copied this key from the only other 'component' extension test I could find.
Is there a better way to generate the key?

files/extensions/api_test/chrome_auth_private

[asargent_no_longer_on_chrome, 2011-08-05 22:39:46]

LGTM

http://codereview.chromium.org/7488060/diff/8007/chrome/test/data/extensions/...
File chrome/test/data/extensions/native_client/manifest.json (right):

http://codereview.chromium.org/7488060/diff/8007/chrome/test/data/extensions/...
chrome/test/data/extensions/native_client/manifest.json:2: "key":
"MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCp40PYAXfSDlzCW1f5MDzRW64h0YhgV7MX8Frem0vO1ZAlq/mlUO4KxwkF2AZliFScO4Cc3CYpO6jpHXwz27tUwaN46C/LzYO7u/kb2piOep8gClvZ64EMnDv5PIoIeZlOJhkpnfs/5FbQt5sqT9avXX7YfLCLBJBql0U/V5615wIDAQAB",
On 2011/08/05 18:40:10, bbudge1 wrote:
> I copied this key from the only other 'component' extension test I could find.
> Is there a better way to generate the key?
> 
> files/extensions/api_test/chrome_auth_private

You should manually install your native_client.crx file, go open the Preferences
file in your profile directory and find the corresponding key entry for the
extension, and put that here.

The reason you sometimes have a key in the manifest and sometimes don't is a bit
complicated and was probably a design mistake, but I can explain in person why
it's that way if you are interested. :)

[Paweł Hajdan Jr., 2011-08-05 23:18:36]

Drive-by with an up-to-you testing comment.

http://codereview.chromium.org/7488060/diff/8007/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_nacl_browsertest.cc (right):

http://codereview.chromium.org/7488060/diff/8007/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_nacl_browsertest.cc:63:
CHECK(ui_test_utils::ExecuteJavaScriptAndExtractBool(
Do you like your tests to be DISABLED? If not, please don't use CHECK but
convert things to use ASSERT_TRUE. Up to you.

http://codereview.chromium.org/7488060/diff/8007/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_nacl_browsertest.cc:72:
CHECK(ui_test_utils::ExecuteJavaScriptAndExtractBool(
Same here.

[bbudge, 2011-08-09 05:26:50]

Had to add a method to ExtensionBrowserTest to use CRX install machinery instead
of rolling my own. Reworked tests a little. Tries are green now. PTAL.

http://codereview.chromium.org/7488060/diff/8007/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_nacl_browsertest.cc (right):

http://codereview.chromium.org/7488060/diff/8007/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_nacl_browsertest.cc:63:
CHECK(ui_test_utils::ExecuteJavaScriptAndExtractBool(
On 2011/08/05 23:18:36, Paweł Hajdan Jr. wrote:
> Do you like your tests to be DISABLED? If not, please don't use CHECK but
> convert things to use ASSERT_TRUE. Up to you.

Done.

http://codereview.chromium.org/7488060/diff/8007/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_nacl_browsertest.cc:72:
CHECK(ui_test_utils::ExecuteJavaScriptAndExtractBool(
On 2011/08/05 23:18:36, Paweł Hajdan Jr. wrote:
> Same here.

Done.

[Paweł Hajdan Jr., 2011-08-09 16:39:07]

Code I commented in the drive-by LGTM, thank you.

[asargent_no_longer_on_chrome, 2011-08-09 19:23:33]

LGTM with one optional suggestion.

http://codereview.chromium.org/7488060/diff/16006/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_nacl_browsertest.cc (right):

http://codereview.chromium.org/7488060/diff/16006/chrome/browser/extensions/e...
chrome/browser/extensions/extension_nacl_browsertest.cc:109:
EXPECT_TRUE(embedded_plugin && content_handler_plugin);
nit: It looks like the body of these 3 tests are identical except for how the
extension is loaded, and what the expected values of embedded_plugin &&
content_handler_plugin is. Consider refactoring the common parts into a member
function in the NaClExtensionTest class - one idea would be to have it be
something like this:

enum TestExtensionType {
  FROM_WEBSTORE,
  NOT_FROM_WEBSTORE,
  COMPONENT
}
void RunTest(TestExtensionType type, bool nacl_allowed) { ... }

[bbudge, 2011-08-11 18:56:02]

http://codereview.chromium.org/7488060/diff/16006/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_nacl_browsertest.cc (right):

http://codereview.chromium.org/7488060/diff/16006/chrome/browser/extensions/e...
chrome/browser/extensions/extension_nacl_browsertest.cc:109:
EXPECT_TRUE(embedded_plugin && content_handler_plugin);
On 2011/08/09 19:23:34, Antony Sargent wrote:
> nit: It looks like the body of these 3 tests are identical except for how the
> extension is loaded, and what the expected values of embedded_plugin &&
> content_handler_plugin is. Consider refactoring the common parts into a member
> function in the NaClExtensionTest class - one idea would be to have it be
> something like this:
> 
> enum TestExtensionType {
>   FROM_WEBSTORE,
>   NOT_FROM_WEBSTORE,
>   COMPONENT
> }
> void RunTest(TestExtensionType type, bool nacl_allowed) { ... }

Done.

[commit-bot: I haz the power, 2011-08-11 22:21:51]

Change committed as 96467