Check for warnings when loading extensions in browser tests.

Check for warnings when loading extensions in browser tests.

Test extension should be well-formed. Having warnings is often an early
indicator that something else is wrong (see for example https://chromiumcodereview.appspot.com/10852016/).

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

[Mihai Parparita -not on Chrome, 2012-08-06 21:50:17]

Aaron: Please verify that this is a good idea, and the
extension_browsertest/apitest.cc changes.
Mike: Please review everything else.

[miket_OOO, 2012-08-06 22:33:28]

LGTM. Please confirm that manifest_format v1 still gets adequate test coverage.
Also, thanks for doing this!

http://codereview.chromium.org/10826157/diff/8001/chrome/browser/extensions/a...
File chrome/browser/extensions/api/web_request/web_request_apitest.cc (right):

http://codereview.chromium.org/10826157/diff/8001/chrome/browser/extensions/a...
chrome/browser/extensions/api/web_request/web_request_apitest.cc:159: int
load_extension_flags = kFlagNone;
To be hyper-correct, this is a ExtensionApiTest::Flags, not an int. But I see
lots and lots of existing int usage, so it's OK as-is.

http://codereview.chromium.org/10826157/diff/8001/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/dns/api/manifest.json (right):

http://codereview.chromium.org/10826157/diff/8001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/dns/api/manifest.json:12: "experimental"
Thanks.

http://codereview.chromium.org/10826157/diff/8001/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/webnavigation/test_failures.js
(right):

http://codereview.chromium.org/10826157/diff/8001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/webnavigation/test_failures.js:190: //
Navigates to a non-existant page (this test case must be last,
non-existent

http://codereview.chromium.org/10826157/diff/8001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/webnavigation/test_failures.js:191: //
otherwise the non-existed URL breaks tests that follow, since loading
non-existent (or maybe non-existing?)

http://codereview.chromium.org/10826157/diff/8001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/webnavigation/test_failures.js:195:
function nonExistant() {
nonExistent

http://codereview.chromium.org/10826157/diff/8001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/webnavigation/test_failures.js:203: url:
getURL('failures/nonexistant.html') }},
nonexistent.html

http://codereview.chromium.org/10826157/diff/8001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/webnavigation/test_failures.js:211: url:
getURL('failures/nonexistant.html') }}],
etc.

[Aaron Boodman, 2012-08-07 02:12:35]

lgtm

http://codereview.chromium.org/10826157/diff/8001/chrome/browser/extensions/a...
File chrome/browser/extensions/api/web_request/web_request_apitest.cc (right):

http://codereview.chromium.org/10826157/diff/8001/chrome/browser/extensions/a...
chrome/browser/extensions/api/web_request/web_request_apitest.cc:160: if
(load_extension_with_incognito_permission) {
Either way is allowed, but we tend to drop the braces in this case in extensions
code.

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

http://codereview.chromium.org/10826157/diff/8001/chrome/browser/extensions/e...
chrome/browser/extensions/extension_nacl_browsertest.cc:156: 
stray edit

[Mihai Parparita -not on Chrome, 2012-08-09 23:04:08]

> Please confirm that manifest_format v1 still gets adequate test coverage.

Yes, the tests that use RunExtensionTestIgnoreManifestWarnings end up
exercising the manifest_version 1 path (it also has unit tests, which aren't
affected by this change)

http://codereview.chromium.org/10826157/diff/8001/chrome/browser/extensions/a...
File chrome/browser/extensions/api/web_request/web_request_apitest.cc (right):

http://codereview.chromium.org/10826157/diff/8001/chrome/browser/extensions/a...
chrome/browser/extensions/api/web_request/web_request_apitest.cc:159: int
load_extension_flags = kFlagNone;
On 2012/08/06 22:33:28, miket wrote:
> To be hyper-correct, this is a ExtensionApiTest::Flags, not an int. But I see
> lots and lots of existing int usage, so it's OK as-is.

ExtensionApiTest::Flags isn't really the right type, because once we start to or
things together, the resulting value is no longer a valid enum value.

http://codereview.chromium.org/10826157/diff/8001/chrome/browser/extensions/a...
chrome/browser/extensions/api/web_request/web_request_apitest.cc:160: if
(load_extension_with_incognito_permission) {
On 2012/08/07 02:12:35, Aaron Boodman wrote:
> Either way is allowed, but we tend to drop the braces in this case in
extensions
> code.

Done.

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

http://codereview.chromium.org/10826157/diff/8001/chrome/browser/extensions/e...
chrome/browser/extensions/extension_nacl_browsertest.cc:156: 
On 2012/08/07 02:12:35, Aaron Boodman wrote:
> stray edit

Fixed.

http://codereview.chromium.org/10826157/diff/8001/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/webnavigation/test_failures.js
(right):

http://codereview.chromium.org/10826157/diff/8001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/webnavigation/test_failures.js:190: //
Navigates to a non-existant page (this test case must be last,
On 2012/08/06 22:33:28, miket wrote:
> non-existent

Fixed all.