Move sockets APIs out of src/chrome

Move sockets APIs out of src/chrome

This moves sockets APIs out of src/chrome and into
src/extensions. The IDLs and function
implementations are moved, and the groundwork is
set for moving additional APIs.

The API tests remain in src/chrome, and several
references to src/chrome still remain for
permissions/features/manifest stuff.

BUG=349787
TBR=erg@chromium.org for profiles change
TBR=jar@chromium.org for +net to //extensions/browser/DEPS

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

[Ken Rockot(use gerrit already), 2014-03-06 18:01:26]

Here we go. FWIW I'm not totally sold on my approach of just stashing
src/extensions generated stuff into extensions::core_api.

I considered two alternatives:

1. Use extensions::api for src/extensions and move all src/chrome generated APIs
into a chrome_api:: or chrome_extensions_api:: namespace. This would be a large
albeit purely mechanical change.

2. Parameterize the generated class names in the schema compiler tools, so we
can generated e.g. GeneratedFunctionRegistry, GeneratedSchemas,
ChromeGeneratedFunctionRegistry, ChromeGeneratedSchemas, all living in
extensions::api.

Also open to other suggestions.

Note that the API implementations haven't changed; they're all namespace and
auto-formatting changes.

[Ken Rockot(use gerrit already), 2014-03-06 18:03:09]

Also note that this CL breaks the doc server and so is dependent on
https://codereview.chromium.org/176973009/ landing first.

[James Cook, 2014-03-06 18:30:59]

I'll let Yoyo make the call on the namespace issue.

(My two cents: I think it's confusing if the chrome-specific APIs are just
"api". They really want to be "chrome_api" or something like it, regardless of
what we call the core api.)

https://codereview.chromium.org/183893041/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/api_resource_manager.h (left):

https://codereview.chromium.org/183893041/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/api_resource_manager.h:162: //
BrowserContextKeyedAPI implementation.
nit: I would keep this comment since these statics actually are the BCKA
implementation details.

https://codereview.chromium.org/183893041/diff/1/chrome/browser/extensions/ap...
File
chrome/browser/extensions/api/sockets_tcp_server/sockets_tcp_server_apitest.cc
(right):

https://codereview.chromium.org/183893041/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/sockets_tcp_server/sockets_tcp_server_apitest.cc:102:
LOG(ERROR) << "sucks";
remove this line

https://codereview.chromium.org/183893041/diff/1/extensions/browser/DEPS
File extensions/browser/DEPS (right):

https://codereview.chromium.org/183893041/diff/1/extensions/browser/DEPS#newc...
extensions/browser/DEPS:17:
"+chrome/browser/extensions/api/api_resource_manager.h",
Both of these seem like they could move to //extensions. What do you think of
either moving them now, or moving them first in a separate CL?

Overall this feels like a relatively large number of DEPS additions to move a
single API.

[Ken Rockot(use gerrit already), 2014-03-06 18:46:32]

I agree with your opinion on the namespace issue. benwells@ also expressed a
desire to see extensions:: properly contain only things in src/extensions - this
makes sense to me and  further reinforces approach #1.

https://codereview.chromium.org/183893041/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/api_resource_manager.h (left):

https://codereview.chromium.org/183893041/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/api_resource_manager.h:162: //
BrowserContextKeyedAPI implementation.
On 2014/03/06 18:31:00, James Cook wrote:
> nit: I would keep this comment since these statics actually are the BCKA
> implementation details.

Yeah, that was a manual merge mistake. Fixed.

https://codereview.chromium.org/183893041/diff/1/chrome/browser/extensions/ap...
File
chrome/browser/extensions/api/sockets_tcp_server/sockets_tcp_server_apitest.cc
(right):

https://codereview.chromium.org/183893041/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/sockets_tcp_server/sockets_tcp_server_apitest.cc:102:
LOG(ERROR) << "sucks";
On 2014/03/06 18:31:00, James Cook wrote:
> remove this line

Done.

https://codereview.chromium.org/183893041/diff/1/extensions/browser/DEPS
File extensions/browser/DEPS (right):

https://codereview.chromium.org/183893041/diff/1/extensions/browser/DEPS#newc...
extensions/browser/DEPS:17:
"+chrome/browser/extensions/api/api_resource_manager.h",
On 2014/03/06 18:31:00, James Cook wrote:
> Both of these seem like they could move to //extensions. What do you think of
> either moving them now, or moving them first in a separate CL?
> 
> Overall this feels like a relatively large number of DEPS additions to move a
> single API.

I think it might cascade quickly (I remember seeing a dependency on
ExtensionHost or something like that). I'll take another look though.

If I move them it'll be a separate precursor CL. Bug already filed btw
(http://crbug.com/349930)

[Yoyo Zhou, 2014-03-06 23:50:06]

temporary names LGTM

https://codereview.chromium.org/183893041/diff/1/extensions/browser/DEPS
File extensions/browser/DEPS (right):

https://codereview.chromium.org/183893041/diff/1/extensions/browser/DEPS#newc...
extensions/browser/DEPS:17:
"+chrome/browser/extensions/api/api_resource_manager.h",
On 2014/03/06 18:46:32, Ken Rockot wrote:
> On 2014/03/06 18:31:00, James Cook wrote:
> > Both of these seem like they could move to //extensions. What do you think
of
> > either moving them now, or moving them first in a separate CL?
> > 
> > Overall this feels like a relatively large number of DEPS additions to move
a
> > single API.
> 
> I think it might cascade quickly (I remember seeing a dependency on
> ExtensionHost or something like that). I'll take another look though.
> 
> If I move them it'll be a separate precursor CL. Bug already filed btw
> (http://crbug.com/349930)

extension_host.h is already in here, but chrome_notification_types are something
we don't yet DEPend on in src/extensions (though src/apps already DEPends on
it).

There's a bug for getting rid of notifications (I can't find it at the moment),
but that's a long-term effort and in the short term, we should pull in
chrome_notification_types, and then start extension_notification_types.

https://codereview.chromium.org/183893041/diff/40001/apps/shell/browser/shell...
File apps/shell/browser/shell_extensions_browser_client.cc (right):

https://codereview.chromium.org/183893041/diff/40001/apps/shell/browser/shell...
apps/shell/browser/shell_extensions_browser_client.cc:161:
extensions::core_api::GeneratedFunctionRegistry::RegisterAll(registry);
I agree, it's not ideally named right now. But I don't expect renaming
extensions::api to be a fun chore.

extensions::api and chrome_api, I think, would be my preferred final names (for
extensions::core_api and extensions::api). Then each GeneratedFunctionRegistry
belongs to a particular namespace.

[Ken Rockot(use gerrit already), 2014-03-07 00:06:35]

Great, thanks. api_resource/_manager are moving in a separate CL. Going to sit
on this one until that and the doc server change can be landed.

https://codereview.chromium.org/183893041/diff/40001/apps/shell/browser/shell...
File apps/shell/browser/shell_extensions_browser_client.cc (right):

https://codereview.chromium.org/183893041/diff/40001/apps/shell/browser/shell...
apps/shell/browser/shell_extensions_browser_client.cc:161:
extensions::core_api::GeneratedFunctionRegistry::RegisterAll(registry);
On 2014/03/06 23:50:08, Yoyo Zhou wrote:
> I agree, it's not ideally named right now. But I don't expect renaming
> extensions::api to be a fun chore.
> 
> extensions::api and chrome_api, I think, would be my preferred final names
(for
> extensions::core_api and extensions::api). Then each GeneratedFunctionRegistry
> belongs to a particular namespace.

Great, I think we have a quorum on this. Bug filed http://crbug.com/350169

[James Cook, 2014-03-07 01:05:27]

Ping me if you want me to look at this again.

[James Cook, 2014-03-07 19:36:21]

LGTM

[Ken Rockot(use gerrit already), 2014-03-07 20:34:08]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2014-03-07 20:36:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rockot@chromium.org/183893041/80001

[Ken Rockot(use gerrit already), 2014-03-08 04:45:10]

The CQ bit was unchecked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2014-03-08 04:48:26]

This is on hold for now because the doc server support had to be rolled back.

[Ken Rockot(use gerrit already), 2014-03-11 17:21:33]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2014-03-11 17:26:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rockot@chromium.org/183893041/100001

[commit-bot: I haz the power, 2014-03-11 17:57:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-11 17:57:04]

Try jobs failed on following builders: mac_chromium_rel

[Ken Rockot(use gerrit already), 2014-03-11 20:58:56]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2014-03-11 21:57:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rockot@chromium.org/183893041/120001

[commit-bot: I haz the power, 2014-03-12 06:41:33]

Change committed as 256456