Move Mime type handling to streamsPrivate API, so that it works on Desktop Chrome.

Move Mime type handling to streamsPrivate API, so that it works on Desktop Chrome.

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

[Zachary Kuznia, 2013-03-01 07:21:33]

Hi Matt,

Could you have a look at this change?

It's basically a move of the mime type handling API from fileBrowserHandler to
streamsPrivate.  The manifest entry from fileBrowserHandler.mimeTypes is also
moved to a top level entry.

The only other change to logic is to enable it on non-ChromeOS platforms.

Thanks,
-Zach

[Matt Perry, 2013-03-01 22:15:16]

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/_manifest_features.json (right):

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/_manifest_features.json:186: "extension_types": [
"extension", "packaged_app", "platform_app" ]
whitelist here too

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/_permission_features.json:326: "extension_types":
["extension", "packaged_app", "platform_app"]
I think you can move the whitelist here instead of hardcoded now. IIRC we didn't
do that before because only part of the API was whitelisted.

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/file_browser_handler.json (right):

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/file_browser_handler.json:20: "tab_id" : {
oops - this should use a javaScriptStyle name.

what about this: (in a future CL) Change this to tabId, and write a custom
argument massager[1] to add an alias to tab_id for backwards compat.

[1] See
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/re...
for an example

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/file_browser_handler.json:58: "name":
"original_url",
these should all be javaScriptStyle as well. Since they're event handler args, I
think these can change without breaking anything.

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/streams_private.json (right):

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/streams_private.json:18: "name": "mime_type",
use javaScriptStyle names for these.

[Zachary Kuznia, 2013-03-04 06:09:43]

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/_manifest_features.json (right):

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/_manifest_features.json:186: "extension_types": [
"extension", "packaged_app", "platform_app" ]
On 2013/03/01 22:15:16, Matt Perry wrote:
> whitelist here too

Done.

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/_permission_features.json (right):

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/_permission_features.json:326: "extension_types":
["extension", "packaged_app", "platform_app"]
On 2013/03/01 22:15:16, Matt Perry wrote:
> I think you can move the whitelist here instead of hardcoded now. IIRC we
didn't
> do that before because only part of the API was whitelisted.

Done.

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/file_browser_handler.json (right):

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/file_browser_handler.json:20: "tab_id" : {
On 2013/03/01 22:15:16, Matt Perry wrote:
> oops - this should use a javaScriptStyle name.
> 
> what about this: (in a future CL) Change this to tabId, and write a custom
> argument massager[1] to add an alias to tab_id for backwards compat.
> 
> [1] See
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/re...
> for an example

Bug filed: crbug.com/179767

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/file_browser_handler.json:58: "name":
"original_url",
On 2013/03/01 22:15:16, Matt Perry wrote:
> these should all be javaScriptStyle as well. Since they're event handler args,
I
> think these can change without breaking anything.

Actually, I meant to remove this API, since it's being move to streamsPrivate.

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
File chrome/common/extensions/api/streams_private.json (right):

https://codereview.chromium.org/12381035/diff/2001/chrome/common/extensions/a...
chrome/common/extensions/api/streams_private.json:18: "name": "mime_type",
On 2013/03/01 22:15:16, Matt Perry wrote:
> use javaScriptStyle names for these.

Done.

[Matt Perry, 2013-03-04 21:11:19]

lgtm

[Zachary Kuznia, 2013-03-05 05:52:49]

sky: Could you do an OWNERS review of:
chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc

erg: Could you do an OWNERS review of:
chrome/browser/profiles/profile_dependency_manager.cc

Thanks,
-Zach

[sky, 2013-03-05 16:50:22]

On Mon, Mar 4, 2013 at 9:52 PM,  <zork@chromium.org> wrote:
> sky: Could you do an OWNERS review of:
> chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc

I'm not a good reviewer for that file. Could you get another reviewer?

> erg: Could you do an OWNERS review of:
> chrome/browser/profiles/profile_dependency_manager.cc
>
> Thanks,
> -Zach
>
> https://codereview.chromium.org/12381035/

[Elliot Glaysher, 2013-03-05 19:02:06]

PDM lgtm

[Zachary Kuznia, 2013-03-05 23:29:36]

brettw:  Could you do an OWNERS review of:
chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc

Thanks,
-Zach

[brettw, 2013-03-07 22:49:27]

chrome/browser/renderer_host/chrome_resource_dispatcher_host_delegate.cc LGTM

[commit-bot: I haz the power, 2013-03-11 00:53:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zork@chromium.org/12381035/25001

[commit-bot: I haz the power, 2013-03-11 01:22:29]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_clang_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-03-11 04:42:49]

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

[commit-bot: I haz the power, 2013-03-11 08:41:30]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-03-12 02:40:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zork@chromium.org/12381035/41043

[commit-bot: I haz the power, 2013-03-12 07:56:00]

Change committed as 187536