Add 'web_accessible_resource" keyword for version 2 extension manifests. This makes extension res...

Add 'web_accessible_resource" keyword for version 2 extension manifests. This makes extension resources web accessible only when explicitly intended. Behavior is unaffected unless an extension specifies a manifest_version of 2 or more. 

BUG=46002
TEST=ExtensionResourceRequestPolicyTest.WebAccessibleResources
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=115401

[Cris Neckar, 2011-12-07 22:47:29]

I have no idea if I did the unit test anywhere near correctly. Aaron, any advice
you can give me on this would be greatly appreciated. 

The other major question is whether the keyword 'web_accessible_resources' is
reasonable. I am happy to change it if either of you has suggestions.

[abarth-chromium, 2011-12-07 22:54:53]

Another approach is to have the web_accessible_resources manifest key work for
all extensions but then to change the default based on the mainfest_version. 
For manifest_version < 2, the default would be "all are accessible".  For
manifest_version >= 2, the default would be "none are accessible".

[abarth-chromium, 2011-12-07 22:55:34]

http://codereview.chromium.org/8849010/diff/13001/chrome/renderer/extensions/...
File chrome/renderer/extensions/extension_resource_request_policy.cc (right):

http://codereview.chromium.org/8849010/diff/13001/chrome/renderer/extensions/...
chrome/renderer/extensions/extension_resource_request_policy.cc:43: if
(extension->manifest_version() < 2 &&
Do you mean >= 2?

[Cris Neckar, 2011-12-07 23:04:33]

On 2011/12/07 22:54:53, abarth wrote:
> Another approach is to have the web_accessible_resources manifest key work for
> all extensions but then to change the default based on the mainfest_version. 
> For manifest_version < 2, the default would be "all are accessible".  For
> manifest_version >= 2, the default would be "none are accessible".

Opps I actually screwed up the check. I changed it so that it also does the
check if there are supplied web_accessible_resources to match the behavior you
suggest.

Aaron, any reason not to allow this in earlier manifest versions if it is
explicitly supplied?

[Cris Neckar, 2011-12-07 23:16:59]

That's what I get for shooting from the hip, ignore that last one. If we decide
we want it to work for versions < 2 I'll add something like
HasWebAccessibleResources() off of Extension.

Sorry for the spam

[Aaron Boodman, 2011-12-08 00:37:38]

I agree with Adam about making this work if the developer specifies it in v1.

Also, unit testing Extension::IsResourceWebAccessible() would be good (see
extension_manifests_unittest.cc).

http://codereview.chromium.org/8849010/diff/18004/chrome/common/extensions/ex...
File chrome/common/extensions/extension.h (right):

http://codereview.chromium.org/8849010/diff/18004/chrome/common/extensions/ex...
chrome/common/extensions/extension.h:324: bool CanWebAccessResource(const
std::string& relative_path) const;
I think the name IsResourceWebAccessible() would be more clear.

http://codereview.chromium.org/8849010/diff/18004/chrome/test/data/extensions...
File
chrome/test/data/extensions/api_test/extension_resource_request_policy/web_accessible/manifest.json
(right):

http://codereview.chromium.org/8849010/diff/18004/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/extension_resource_request_policy/web_accessible/manifest.json:2:
"key":
"MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC2Amx0Ngou+Or8umJXhdpsvQ1jVJvGLD8th6cemE5JL5h9y+5Yj8MMG2CfSiO7MfyZ6aTjB9YiMjVfhngSAufQY0eZnlNsGcBScwMmU/tTbDZEptJegW0XevECgs0ze92n3VoHkDUuuVf6wgW8WHgBA/ERMjJa+nqDVPjJjqUGwwIDAQAB",
The "key" property is not required.

[Aaron Boodman, 2011-12-08 00:37:57]

(and btw, your api test is perfect - thanks for taking the time to make one!)

[Cris Neckar, 2011-12-08 00:54:14]

On 2011/12/08 00:37:57, Aaron Boodman wrote:
> (and btw, your api test is perfect - thanks for taking the time to make one!)

So if I don't specify a key will the chrome-extension: uris in the html files be
correct? It seems like I caused a ton of tests to fail on the try bots and I
have no clue how to fix them :(

[Aaron Boodman, 2011-12-08 01:04:42]

On Wed, Dec 7, 2011 at 4:54 PM,  <cdn@chromium.org> wrote:
> So if I don't specify a key will the chrome-extension: uris in the html
> files be
> correct? It seems like I caused a ton of tests to fail on the try bots and I
> have no clue how to fix them :(

Oh I didn't notice that - just use relative paths (eg "test.png" alone).

- a

[Cris Neckar, 2011-12-08 01:13:54]

On 2011/12/08 01:04:42, Aaron Boodman wrote:
> On Wed, Dec 7, 2011 at 4:54 PM,  <mailto:cdn@chromium.org> wrote:
> > So if I don't specify a key will the chrome-extension: uris in the html
> > files be
> > correct? It seems like I caused a ton of tests to fail on the try bots and I
> > have no clue how to fix them :(
> 
> Oh I didn't notice that - just use relative paths (eg "test.png" alone).
> 
> - a

cool I'll try that. 

It looks like the failing tests are mostly just this working as intended. I'll
go through and add the directive to the manifests and make the other changes
discussed tomorrow.

Thanks guys

[Cris Neckar, 2011-12-08 23:45:21]

On 2011/12/08 01:13:54, Cris Neckar wrote:
> On 2011/12/08 01:04:42, Aaron Boodman wrote:
> > On Wed, Dec 7, 2011 at 4:54 PM,  <mailto:cdn@chromium.org> wrote:
> > > So if I don't specify a key will the chrome-extension: uris in the html
> > > files be
> > > correct? It seems like I caused a ton of tests to fail on the try bots and
I
> > > have no clue how to fix them :(
> > 
> > Oh I didn't notice that - just use relative paths (eg "test.png" alone).
> > 
> > - a
> 
> cool I'll try that. 
> 
> It looks like the failing tests are mostly just this working as intended. I'll
> go through and add the directive to the manifests and make the other changes
> discussed tomorrow.
> 
> Thanks guys

Ok I think this one should be correct. I'll let try jobs finish to confirm.

[abarth-chromium, 2011-12-08 23:52:44]

http://codereview.chromium.org/8849010/diff/21008/chrome/common/extensions/ma...
File chrome/common/extensions/manifest.cc (right):

http://codereview.chromium.org/8849010/diff/21008/chrome/common/extensions/ma...
chrome/common/extensions/manifest.cc:63: map[keys::kWebAccessibleResources] =
all_but_themes;
Do hosted apps use this key too?

http://codereview.chromium.org/8849010/diff/21008/chrome/renderer/extensions/...
File chrome/renderer/extensions/extension_resource_request_policy.cc (right):

http://codereview.chromium.org/8849010/diff/21008/chrome/renderer/extensions/...
chrome/renderer/extensions/extension_resource_request_policy.cc:45: // - empty
origin (needed for some edge cases when we have empty origins)
Empty origins no longer exist.

http://codereview.chromium.org/8849010/diff/21008/chrome/renderer/extensions/...
chrome/renderer/extensions/extension_resource_request_policy.cc:47: // - data:
(basic HTML notifications use data URLs internally)
Doesn't this cause a big security hole?  Any web site can use a frame with a
data URL to make a frame to a non-public extension resource.  It seems like we
need a better way to whitelist notifications.

[Cris Neckar, 2011-12-09 00:06:22]

http://codereview.chromium.org/8849010/diff/21008/chrome/common/extensions/ma...
File chrome/common/extensions/manifest.cc (right):

http://codereview.chromium.org/8849010/diff/21008/chrome/common/extensions/ma...
chrome/common/extensions/manifest.cc:63: map[keys::kWebAccessibleResources] =
all_but_themes;
On 2011/12/08 23:52:45, abarth wrote:
> Do hosted apps use this key too?

I'm not sure.. Aaron should be able to answer this.

http://codereview.chromium.org/8849010/diff/21008/chrome/renderer/extensions/...
File chrome/renderer/extensions/extension_resource_request_policy.cc (right):

http://codereview.chromium.org/8849010/diff/21008/chrome/renderer/extensions/...
chrome/renderer/extensions/extension_resource_request_policy.cc:45: // - empty
origin (needed for some edge cases when we have empty origins)
On 2011/12/08 23:52:45, abarth wrote:
> Empty origins no longer exist.

Extension background pages still do. This is what was causing all the unit test
failures.

http://codereview.chromium.org/8849010/diff/21008/chrome/renderer/extensions/...
chrome/renderer/extensions/extension_resource_request_policy.cc:47: // - data:
(basic HTML notifications use data URLs internally)
On 2011/12/08 23:52:45, abarth wrote:
> Doesn't this cause a big security hole?  Any web site can use a frame with a
> data URL to make a frame to a non-public extension resource.  It seems like we
> need a better way to whitelist notifications.

Yeah good point. I included this because Aaron had it in the original version
which blocked resource loads from pages which the extension doesn't request
access to.

I'm not sure what the reasoning was. I can take it out and see if anything still
breaks.

[Cris Neckar, 2011-12-09 00:13:06]

http://codereview.chromium.org/8849010/diff/13001/chrome/renderer/extensions/...
File chrome/renderer/extensions/extension_resource_request_policy.cc (right):

http://codereview.chromium.org/8849010/diff/13001/chrome/renderer/extensions/...
chrome/renderer/extensions/extension_resource_request_policy.cc:43: if
(extension->manifest_version() < 2 &&
On 2011/12/07 22:55:34, abarth wrote:
> Do you mean >= 2?

Done.

http://codereview.chromium.org/8849010/diff/18004/chrome/common/extensions/ex...
File chrome/common/extensions/extension.h (right):

http://codereview.chromium.org/8849010/diff/18004/chrome/common/extensions/ex...
chrome/common/extensions/extension.h:324: bool CanWebAccessResource(const
std::string& relative_path) const;
On 2011/12/08 00:37:38, Aaron Boodman wrote:
> I think the name IsResourceWebAccessible() would be more clear.

Done.

http://codereview.chromium.org/8849010/diff/18004/chrome/test/data/extensions...
File
chrome/test/data/extensions/api_test/extension_resource_request_policy/web_accessible/manifest.json
(right):

http://codereview.chromium.org/8849010/diff/18004/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/extension_resource_request_policy/web_accessible/manifest.json:2:
"key":
"MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC2Amx0Ngou+Or8umJXhdpsvQ1jVJvGLD8th6cemE5JL5h9y+5Yj8MMG2CfSiO7MfyZ6aTjB9YiMjVfhngSAufQY0eZnlNsGcBScwMmU/tTbDZEptJegW0XevECgs0ze92n3VoHkDUuuVf6wgW8WHgBA/ERMjJa+nqDVPjJjqUGwwIDAQAB",
On 2011/12/08 00:37:38, Aaron Boodman wrote:
> The "key" property is not required.

Done.

[Aaron Boodman, 2011-12-09 15:17:21]

LGTM

One last idea for improvement. No need to wait for another review from me.

http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/...
File chrome/renderer/extensions/extension_resource_request_policy.cc (right):

http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/...
chrome/renderer/extensions/extension_resource_request_policy.cc:49:
(extension->manifest_version() >= 2 ||
Why not move the manifest-version part of this into Extension, so that it can be
unit tested?

[Cris Neckar, 2011-12-16 01:15:28]

If either of you has any idea why this is failing on the trybots I could really
use the help. I have tried everything that I can think of and have no idea what
is going on. The test passes locally and I've verified that the extension id
doesn't change on the trybots (by adding an expect() for the literal string.
I've also verified it manually in the browser and opening the test file works as
expected. 

Any other thoughts on what to try?


On 2011/12/09 15:17:21, Aaron Boodman wrote:
> LGTM
> 
> One last idea for improvement. No need to wait for another review from me.
> 
>
http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/...
> File chrome/renderer/extensions/extension_resource_request_policy.cc (right):
> 
>
http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/...
> chrome/renderer/extensions/extension_resource_request_policy.cc:49:
> (extension->manifest_version() >= 2 ||
> Why not move the manifest-version part of this into Extension, so that it can
be
> unit tested?

[mihaip_google.com, 2011-12-16 01:50:07]

I believe that trybots don't handle patches with binary changes well. You
should try TBR-ing the images first and then using a tryjob at that
revision.

Mihai

On Thu, Dec 15, 2011 at 5:15 PM, <cdn@chromium.org> wrote:

> If either of you has any idea why this is failing on the trybots I could
> really
> use the help. I have tried everything that I can think of and have no idea
> what
> is going on. The test passes locally and I've verified that the extension
> id
> doesn't change on the trybots (by adding an expect() for the literal
> string.
> I've also verified it manually in the browser and opening the test file
> works as
> expected.
>
> Any other thoughts on what to try?
>
>
>
> On 2011/12/09 15:17:21, Aaron Boodman wrote:
>
>> LGTM
>>
>
>  One last idea for improvement. No need to wait for another review from me.
>>
>
>
> http://codereview.chromium.**org/8849010/diff/22024/chrome/**
>
renderer/extensions/extension_**resource_request_policy.cc<http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/extension_resource_request_policy.cc>
>
>> File chrome/renderer/extensions/**extension_resource_request_**policy.cc
>> (right):
>>
>
>
> http://codereview.chromium.**org/8849010/diff/22024/chrome/**
>
renderer/extensions/extension_**resource_request_policy.cc#**newcode49<http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/extension_resource_request_policy.cc#newcode49>
>
>> chrome/renderer/extensions/**extension_resource_request_**policy.cc:49:
>> (extension->manifest_version() >= 2 ||
>> Why not move the manifest-version part of this into Extension, so that it
>> can
>>
> be
>
>> unit tested?
>>
>
>
>
>
http://codereview.chromium.**org/8849010/<http://codereview.chromium.org/8849...
>

[Aaron Boodman, 2011-12-16 04:11:59]

Mihai's right. I thought I told you about that the other day.

- a

On Thu, Dec 15, 2011 at 5:50 PM, Mihai Parparita <mihaip@google.com> wrote:
> I believe that trybots don't handle patches with binary changes well. You
> should try TBR-ing the images first and then using a tryjob at that
> revision.
>
> Mihai
>
>
> On Thu, Dec 15, 2011 at 5:15 PM, <cdn@chromium.org> wrote:
>>
>> If either of you has any idea why this is failing on the trybots I could
>> really
>> use the help. I have tried everything that I can think of and have no idea
>> what
>> is going on. The test passes locally and I've verified that the extension
>> id
>> doesn't change on the trybots (by adding an expect() for the literal
>> string.
>> I've also verified it manually in the browser and opening the test file
>> works as
>> expected.
>>
>> Any other thoughts on what to try?
>>
>>
>>
>> On 2011/12/09 15:17:21, Aaron Boodman wrote:
>>>
>>> LGTM
>>
>>
>>> One last idea for improvement. No need to wait for another review from
>>> me.
>>
>>
>>
>>
>>
http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/...
>>>
>>> File chrome/renderer/extensions/extension_resource_request_policy.cc
>>> (right):
>>
>>
>>
>>
>>
http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/...
>>>
>>> chrome/renderer/extensions/extension_resource_request_policy.cc:49:
>>> (extension->manifest_version() >= 2 ||
>>> Why not move the manifest-version part of this into Extension, so that it
>>> can
>>
>> be
>>>
>>> unit tested?
>>
>>
>>
>>
>> http://codereview.chromium.org/8849010/
>
>

[Cris Neckar, 2011-12-16 23:07:34]

Thanks guys, that did the trick.. Unfortunately there is another issue with the
nacl tests which we still need to solve. I'll keep you posted.

On 2011/12/16 04:11:59, Aaron Boodman wrote:
> Mihai's right. I thought I told you about that the other day.
> 
> - a
> 
> On Thu, Dec 15, 2011 at 5:50 PM, Mihai Parparita <mailto:mihaip@google.com>
wrote:
> > I believe that trybots don't handle patches with binary changes well. You
> > should try TBR-ing the images first and then using a tryjob at that
> > revision.
> >
> > Mihai
> >
> >
> > On Thu, Dec 15, 2011 at 5:15 PM, <mailto:cdn@chromium.org> wrote:
> >>
> >> If either of you has any idea why this is failing on the trybots I could
> >> really
> >> use the help. I have tried everything that I can think of and have no idea
> >> what
> >> is going on. The test passes locally and I've verified that the extension
> >> id
> >> doesn't change on the trybots (by adding an expect() for the literal
> >> string.
> >> I've also verified it manually in the browser and opening the test file
> >> works as
> >> expected.
> >>
> >> Any other thoughts on what to try?
> >>
> >>
> >>
> >> On 2011/12/09 15:17:21, Aaron Boodman wrote:
> >>>
> >>> LGTM
> >>
> >>
> >>> One last idea for improvement. No need to wait for another review from
> >>> me.
> >>
> >>
> >>
> >>
> >>
>
http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/...
> >>>
> >>> File chrome/renderer/extensions/extension_resource_request_policy.cc
> >>> (right):
> >>
> >>
> >>
> >>
> >>
>
http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/...
> >>>
> >>> chrome/renderer/extensions/extension_resource_request_policy.cc:49:
> >>> (extension->manifest_version() >= 2 ||
> >>> Why not move the manifest-version part of this into Extension, so that it
> >>> can
> >>
> >> be
> >>>
> >>> unit tested?
> >>
> >>
> >>
> >>
> >> http://codereview.chromium.org/8849010/
> >
> >

[Cris Neckar, 2011-12-20 19:48:45]

Ok I think this is now ready to land. Could I get another sanity check from you
guys as a few things have changed since aa's lgtm. Specifically I added a switch
to turn this off.

On 2011/12/16 23:07:34, Cris Neckar wrote:
> Thanks guys, that did the trick.. Unfortunately there is another issue with
the
> nacl tests which we still need to solve. I'll keep you posted.
> 
> On 2011/12/16 04:11:59, Aaron Boodman wrote:
> > Mihai's right. I thought I told you about that the other day.
> > 
> > - a
> > 
> > On Thu, Dec 15, 2011 at 5:50 PM, Mihai Parparita <mailto:mihaip@google.com>
> wrote:
> > > I believe that trybots don't handle patches with binary changes well. You
> > > should try TBR-ing the images first and then using a tryjob at that
> > > revision.
> > >
> > > Mihai
> > >
> > >
> > > On Thu, Dec 15, 2011 at 5:15 PM, <mailto:cdn@chromium.org> wrote:
> > >>
> > >> If either of you has any idea why this is failing on the trybots I could
> > >> really
> > >> use the help. I have tried everything that I can think of and have no
idea
> > >> what
> > >> is going on. The test passes locally and I've verified that the extension
> > >> id
> > >> doesn't change on the trybots (by adding an expect() for the literal
> > >> string.
> > >> I've also verified it manually in the browser and opening the test file
> > >> works as
> > >> expected.
> > >>
> > >> Any other thoughts on what to try?
> > >>
> > >>
> > >>
> > >> On 2011/12/09 15:17:21, Aaron Boodman wrote:
> > >>>
> > >>> LGTM
> > >>
> > >>
> > >>> One last idea for improvement. No need to wait for another review from
> > >>> me.
> > >>
> > >>
> > >>
> > >>
> > >>
> >
>
http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/...
> > >>>
> > >>> File chrome/renderer/extensions/extension_resource_request_policy.cc
> > >>> (right):
> > >>
> > >>
> > >>
> > >>
> > >>
> >
>
http://codereview.chromium.org/8849010/diff/22024/chrome/renderer/extensions/...
> > >>>
> > >>> chrome/renderer/extensions/extension_resource_request_policy.cc:49:
> > >>> (extension->manifest_version() >= 2 ||
> > >>> Why not move the manifest-version part of this into Extension, so that
it
> > >>> can
> > >>
> > >> be
> > >>>
> > >>> unit tested?
> > >>
> > >>
> > >>
> > >>
> > >> http://codereview.chromium.org/8849010/
> > >
> > >

[Aaron Boodman, 2011-12-20 21:12:58]

LGTM

some nits tho

http://codereview.chromium.org/8849010/diff/66002/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/8849010/diff/66002/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:276: const char
kDisableExtensionsResourceWhitelist[] =
Perhaps a brief description of what the 'extension resource whitelist' feature
is in the comments?

http://codereview.chromium.org/8849010/diff/66002/chrome/test/data/extensions...
File
chrome/test/data/extensions/api_test/content_scripts/extension_iframe/manifest.json
(right):

http://codereview.chromium.org/8849010/diff/66002/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/content_scripts/extension_iframe/manifest.json:16:
"iframe.js"
weird indent

http://codereview.chromium.org/8849010/diff/66002/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/tabs/on_updated/manifest.json (right):

http://codereview.chromium.org/8849010/diff/66002/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/tabs/on_updated/manifest.json:10:
"iframeNavigated/iframe1.js",
tab characters here

http://codereview.chromium.org/8849010/diff/66002/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/webrequest/manifest.json (right):

http://codereview.chromium.org/8849010/diff/66002/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/webrequest/manifest.json:9:
"complexLoad/a.html",
tab characters here

http://codereview.chromium.org/8849010/diff/66002/chrome/test/nacl_test_injec...
File chrome/test/nacl_test_injection/buildbot_nacl_integration.py (right):

http://codereview.chromium.org/8849010/diff/66002/chrome/test/nacl_test_injec...
chrome/test/nacl_test_injection/buildbot_nacl_integration.py:47:
tests_to_disable.append('run_ppapi_extension_mime_handler_browser_test')
I have no idea about this. Did you talk to the nacl people?

http://codereview.chromium.org/8849010/diff/66002/ppapi/native_client/tests/p...
File
ppapi/native_client/tests/ppapi_browser/extension_mime_handler/manifest.json
(right):

http://codereview.chromium.org/8849010/diff/66002/ppapi/native_client/tests/p...
ppapi/native_client/tests/ppapi_browser/extension_mime_handler/manifest.json:16:
"ppapi_extension_mime_handler_x86-64.nexe"
tab

[Cris Neckar, 2011-12-20 22:00:06]

http://codereview.chromium.org/8849010/diff/66002/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/8849010/diff/66002/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:276: const char
kDisableExtensionsResourceWhitelist[] =
On 2011/12/20 21:12:58, Aaron Boodman wrote:
> Perhaps a brief description of what the 'extension resource whitelist' feature
> is in the comments?

Done.

http://codereview.chromium.org/8849010/diff/66002/chrome/test/data/extensions...
File
chrome/test/data/extensions/api_test/content_scripts/extension_iframe/manifest.json
(right):

http://codereview.chromium.org/8849010/diff/66002/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/content_scripts/extension_iframe/manifest.json:16:
"iframe.js"
On 2011/12/20 21:12:58, Aaron Boodman wrote:
> weird indent

Done.

http://codereview.chromium.org/8849010/diff/66002/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/tabs/on_updated/manifest.json (right):

http://codereview.chromium.org/8849010/diff/66002/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/tabs/on_updated/manifest.json:10:
"iframeNavigated/iframe1.js",
On 2011/12/20 21:12:58, Aaron Boodman wrote:
> tab characters here

Done.

http://codereview.chromium.org/8849010/diff/66002/chrome/test/nacl_test_injec...
File chrome/test/nacl_test_injection/buildbot_nacl_integration.py (right):

http://codereview.chromium.org/8849010/diff/66002/chrome/test/nacl_test_injec...
chrome/test/nacl_test_injection/buildbot_nacl_integration.py:47:
tests_to_disable.append('run_ppapi_extension_mime_handler_browser_test')
On 2011/12/20 21:12:58, Aaron Boodman wrote:
> I have no idea about this. Did you talk to the nacl people?

Yeah, I should have noted this in my comment. I discussed this with the nacl
team and we are working on getting it re-enabled. basically we will just need to
plumb the command line switch that I added through to nacl.

http://codereview.chromium.org/8849010/diff/66002/ppapi/native_client/tests/p...
File
ppapi/native_client/tests/ppapi_browser/extension_mime_handler/manifest.json
(right):

http://codereview.chromium.org/8849010/diff/66002/ppapi/native_client/tests/p...
ppapi/native_client/tests/ppapi_browser/extension_mime_handler/manifest.json:16:
"ppapi_extension_mime_handler_x86-64.nexe"
On 2011/12/20 21:12:58, Aaron Boodman wrote:
> tab

Done.

[Aaron Boodman, 2011-12-20 22:18:16]

OK, LGTM

[Cris Neckar, 2011-12-20 22:54:09]

On 2011/12/20 22:18:16, Aaron Boodman wrote:
> OK, LGTM

Thanks I want to wait for the trybots one last time and then I'll land it.