Move socket API from experimental to dev channel and remove some dead code

Move socket API from experimental to dev channel and remove some dead code.


BUG=130379
TEST=browser_tests --gtest_filter=SocketApiTest.*

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

[Peng, 2012-06-01 15:40:09]

I think current socket API is OK for moving out of experimental. Please review
this CL.

[miket_OOO, 2012-06-01 16:42:41]

On 2012/06/01 15:40:09, Peng wrote:
> I think current socket API is OK for moving out of experimental. Please review
> this CL.

Before I even review this, we have a few more safeties we'd like to add in
before making a change like this. Please don't commit until we've done that
work. I'll discuss with you in email.

[Peng, 2012-07-27 15:05:19]

Hi Mike, I moved socket API from experimental to dev channel. Could you please
review it? Thanks.

[miket_OOO, 2012-07-27 16:37:53]

On 2012/07/27 15:05:19, Peng wrote:
> Hi Mike, I moved socket API from experimental to dev channel. Could you please
> review it? Thanks.

Exciting to reach this milestone! A few things:

1. You'll have to run DumpRenderTree to update the generated documentation. The
resulting changes should be included in this changelist.

2. I didn't see any changes to tests. I'm quite sure all the affected tests will
break with this change. Did you run your unit and browser tests locally before
sending out this CL?

3. Please ping mkearney and let her know about this change so that she can
update any remaining affected documentation.

4. Please update samples in https://github.com/GoogleChrome/chrome-app-samples
as this change lands.

5. Ping jeremya to get ircv updated as this lands.

6. Send out a PSA to chromium-apps-announce@, and to announce on #chromium-apps
IRC, to let people know about this breaking change.

There might be more logistics I'm not thinking of, but as for the CL itself,
please see inline comments and let me know about the tests.

[miket_OOO, 2012-07-27 16:37:59]

http://codereview.chromium.org/10440097/diff/4001/chrome/common/extensions/ap...
File chrome/common/extensions/api/socket.idl (right):

http://codereview.chromium.org/10440097/diff/4001/chrome/common/extensions/ap...
chrome/common/extensions/api/socket.idl:17: [nodoc] long? dummyValue;
Would you please see whether we can get rid of this nodoc field? I can't
remember whether we fixed this bug.

[miket_OOO, 2012-07-27 16:44:05]

One more thing I forgot. When you run the tests you'll discover that all apps
using this API now need an explicit "socket" permission (formerly the blanket
experimental permission took care of it).

[Peng, 2012-07-27 19:08:42]

Fixed issues and updated document. Please review it. Thanks.

http://codereview.chromium.org/10440097/diff/4001/chrome/common/extensions/ap...
File chrome/common/extensions/api/socket.idl (right):

http://codereview.chromium.org/10440097/diff/4001/chrome/common/extensions/ap...
chrome/common/extensions/api/socket.idl:17: [nodoc] long? dummyValue;
On 2012/07/27 16:37:59, miket wrote:
> Would you please see whether we can get rid of this nodoc field? I can't
> remember whether we fixed this bug.

Done.

[Peng, 2012-07-27 19:09:52]

http://codereview.chromium.org/10440097/diff/9001/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/socket/api/manifest.json (left):

http://codereview.chromium.org/10440097/diff/9001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/socket/api/manifest.json:13: "socket"
I run browser test, no error happened. It because the old manifest file already
includes socket permission.

[miket_OOO, 2012-07-27 19:21:24]

LGTM after removing the #if 0.

http://codereview.chromium.org/10440097/diff/9001/chrome/browser/extensions/a...
File chrome/browser/extensions/api/api_resource_event_notifier.h (right):

http://codereview.chromium.org/10440097/diff/9001/chrome/browser/extensions/a...
chrome/browser/extensions/api/api_resource_event_notifier.h:41: #if 0
Just remove the code. No #if 0 in non-test code.

http://codereview.chromium.org/10440097/diff/9001/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/socket/api/manifest.json (left):

http://codereview.chromium.org/10440097/diff/9001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/socket/api/manifest.json:13: "socket"
On 2012/07/27 19:09:52, Peng wrote:
> I run browser test, no error happened. It because the old manifest file
already
> includes socket permission. 

Great. Just for the sake of sanity, try removing the socket permission and
making sure that the test *does* fail. If not, then something's broken with the
permission framework. I'm slightly paranoid about this because it's the first
app API we've taken out of experimental, and I don't want anything to slip by
accidentally.

[Peng, 2012-07-27 19:31:41]

http://codereview.chromium.org/10440097/diff/9001/chrome/browser/extensions/a...
File chrome/browser/extensions/api/api_resource_event_notifier.h (right):

http://codereview.chromium.org/10440097/diff/9001/chrome/browser/extensions/a...
chrome/browser/extensions/api/api_resource_event_notifier.h:41: #if 0
On 2012/07/27 19:21:24, miket wrote:
> Just remove the code. No #if 0 in non-test code.

Done.

http://codereview.chromium.org/10440097/diff/9001/chrome/test/data/extensions...
File chrome/test/data/extensions/api_test/socket/api/manifest.json (left):

http://codereview.chromium.org/10440097/diff/9001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/socket/api/manifest.json:13: "socket"
On 2012/07/27 19:21:24, miket wrote:
> On 2012/07/27 19:09:52, Peng wrote:
> > I run browser test, no error happened. It because the old manifest file
> already
> > includes socket permission. 
> 
> Great. Just for the sake of sanity, try removing the socket permission and
> making sure that the test *does* fail. If not, then something's broken with
the
> permission framework. I'm slightly paranoid about this because it's the first
> app API we've taken out of experimental, and I don't want anything to slip by
> accidentally.

Done. Cases failed without socket permission.

[commit-bot: I haz the power, 2012-07-27 19:32:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/penghuang@chromium.org/10440097/5010

[commit-bot: I haz the power, 2012-07-27 21:38:14]

Change committed as 148819