Implement Command Line Private extension API

Implement Command Line Private extension API.

API proposal: https://docs.google.com/document/d/1joXXu06G3XtvJf8fSNVVvB7UCMadVO07E_v3EulPB00/edit

BUG=226915
TEST=browser_tests

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

[not at google - send to devlin, 2013-04-15 06:08:45]

cool, lg when there are tests

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
File
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc
(right):

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc:14:
namespace command_line_private = extensions::api::command_line_private;
can go inside the extensions { namespace below

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc:21:
EXTENSION_FUNCTION_VALIDATE(params.get());
don't need .get()

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc:24:
return false;
should also set error_ here

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc:28:
SetResult(base::Value::CreateBooleanValue(result));
typically I'd (a) inline most of this and (b) use
command_line_private::HasSwitch::Result::Create instead of CreateBooleanValue
(in which case you need to assign to result_ rather than use SetResult).

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/common/exten...
File chrome/common/extensions/api/command_line_private.json (right):

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/common/exten...
chrome/common/extensions/api/command_line_private.json:8: "nodoc": true,
nodoc isn't necessary since we're not adding a template for it.

[方觉(Fang Jue), 2013-04-15 06:25:03]

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
File
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc
(right):

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc:14:
namespace command_line_private = extensions::api::command_line_private;
On 2013/04/15 06:08:45, kalman wrote:
> can go inside the extensions { namespace below

Done.

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc:21:
EXTENSION_FUNCTION_VALIDATE(params.get());
On 2013/04/15 06:08:45, kalman wrote:
> don't need .get()

I looked at other API implementations (such as alarms) and they all use
EXTENSION_FUNCTION_VALIDATE(params.get());

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc:24:
return false;
On 2013/04/15 06:08:45, kalman wrote:
> should also set error_ here

Done.

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc:28:
SetResult(base::Value::CreateBooleanValue(result));
On 2013/04/15 06:08:45, kalman wrote:
> typically I'd (a) inline most of this and (b) use
> command_line_private::HasSwitch::Result::Create instead of CreateBooleanValue
> (in which case you need to assign to result_ rather than use SetResult).

Done.

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/common/exten...
File chrome/common/extensions/api/command_line_private.json (right):

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/common/exten...
chrome/common/extensions/api/command_line_private.json:8: "nodoc": true,
On 2013/04/15 06:08:45, kalman wrote:
> nodoc isn't necessary since we're not adding a template for it.

Done.
Actually, many private APIs use nodoc here but I looked at some newer APIs (like
developerPrivate) and they don't use it.

[not at google - send to devlin, 2013-04-15 06:26:52]

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
File
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc
(right):

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/browser/exte...
chrome/browser/extensions/api/command_line_private/command_line_private_api.cc:21:
EXTENSION_FUNCTION_VALIDATE(params.get());
On 2013/04/15 06:25:03, 方觉(Fang Jue) wrote:
> On 2013/04/15 06:08:45, kalman wrote:
> > don't need .get()
> 
> I looked at other API implementations (such as alarms) and they all use
> EXTENSION_FUNCTION_VALIDATE(params.get());

Nevertheless, you don't need .get().

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/common/exten...
File chrome/common/extensions/api/command_line_private.json (right):

https://chromiumcodereview.appspot.com/14027005/diff/3002/chrome/common/exten...
chrome/common/extensions/api/command_line_private.json:8: "nodoc": true,
On 2013/04/15 06:25:03, 方觉(Fang Jue) wrote:
> On 2013/04/15 06:08:45, kalman wrote:
> > nodoc isn't necessary since we're not adding a template for it.
> 
> Done.
> Actually, many private APIs use nodoc here but I looked at some newer APIs
(like
> developerPrivate) and they don't use it.

Yeah this is a relic.

[方觉(Fang Jue), 2013-04-15 07:55:51]

Added a missing file (extension_function_histogram_value.h). Do I need to update
tools/metrics/histograms/histograms.xml?

Test is added. Please take a look.

[方觉(Fang Jue), 2013-04-15 10:39:19]

+Ben Goodger for gyps.

[not at google - send to devlin, 2013-04-15 12:15:36]

> Added a missing file (extension_function_histogram_value.h). Do I need to
update
> tools/metrics/histograms/histograms.xml?

Nope, that's only needed for the the UMA_* call sites.

> 
> Test is added. Please take a look.

Looking.

[not at google - send to devlin, 2013-04-15 12:20:15]

lgtm

https://chromiumcodereview.appspot.com/14027005/diff/27015/chrome/test/data/e...
File chrome/test/data/extensions/api_test/command_line/basics/test.js (right):

https://chromiumcodereview.appspot.com/14027005/diff/27015/chrome/test/data/e...
chrome/test/data/extensions/api_test/command_line/basics/test.js:21:
assertEq(result, true);
assertTrue(result)?

https://chromiumcodereview.appspot.com/14027005/diff/27015/chrome/test/data/e...
chrome/test/data/extensions/api_test/command_line/basics/test.js:28:
assertEq(result, false);
assertFalse(result)?

[方觉(Fang Jue), 2013-04-15 23:25:03]

Also added "commandLinePrivate" permission in manifest.json.

https://chromiumcodereview.appspot.com/14027005/diff/27015/chrome/test/data/e...
File chrome/test/data/extensions/api_test/command_line/basics/test.js (right):

https://chromiumcodereview.appspot.com/14027005/diff/27015/chrome/test/data/e...
chrome/test/data/extensions/api_test/command_line/basics/test.js:21:
assertEq(result, true);
On 2013/04/15 12:20:15, kalman wrote:
> assertTrue(result)?

Done.

https://chromiumcodereview.appspot.com/14027005/diff/27015/chrome/test/data/e...
chrome/test/data/extensions/api_test/command_line/basics/test.js:28:
assertEq(result, false);
On 2013/04/15 12:20:15, kalman wrote:
> assertFalse(result)?

Done.

[Ben Goodger (Google), 2013-04-16 16:33:16]

gyp lgtm

[commit-bot: I haz the power, 2013-04-18 04:05:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fangjue23303@gmail.com/14027005/36001

[commit-bot: I haz the power, 2013-04-18 04:05:29]

Failed to apply patch for
chrome/browser/extensions/extension_function_histogram_value.h:
While running patch -p0 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/extensions/extension_function_histogram_value.h
  Hunk #1 FAILED at 502.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/extensions/extension_function_histogram_value.h.rej

Patch:       chrome/browser/extensions/extension_function_histogram_value.h
Index: chrome/browser/extensions/extension_function_histogram_value.h
===================================================================
--- chrome/browser/extensions/extension_function_histogram_value.h	(revision
194035)
+++ chrome/browser/extensions/extension_function_histogram_value.h	(working
copy)
@@ -502,6 +502,7 @@
   WEBSTOREPRIVATE_ENABLEAPPLAUNCHER,
   APP_CURRENTWINDOWINTERNAL_FULLSCREEN,
   DEVELOPERPRIVATE_LOADUNPACKEDCROS,
+  COMMANDLINEPRIVATE_HASSWITCH,
   ENUM_BOUNDARY // Last entry: Add new entries above.
 };

[commit-bot: I haz the power, 2013-04-18 04:09:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fangjue23303@gmail.com/14027005/46001

[方觉(Fang Jue), 2013-04-18 04:46:25]

On 2013/04/18 04:09:59, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/fangjue23303%40gmail.com/14027005/46001

unit_tests fails on all platforms. I added APIPermission::kCommandLinePrivate in
permission_set_unittest.cc. Please take a look. Thanks!

[not at google - send to devlin, 2013-04-18 04:58:57]

lgtm

[方觉(Fang Jue), 2013-04-18 05:06:02]

On 2013/04/18 04:58:57, kalman wrote:
> lgtm

browser_tests also fails. It seems that component extensions must have "key" in
manifest.json? How should I generate that key? Package the test extension and
find it in the packaged manifest.json?

Details:
[12268:12268:0417/213543:1474570745:FATAL:component_loader.cc(55)] Check failed:
manifest->GetString(extension_manifest_keys::kPublicKey, &raw_key). 
[12268:12268:0417/213543:1474570745:FATAL:component_loader.cc(55)] Check failed:
manifest->GetString(extension_manifest_keys::kPublicKey, &raw_key). 
 [0x00000161b764]
_ZN10extensions12_GLOBAL__N_110GenerateIdEPKN4base15DictionaryValueERKNS1_8FilePathE.isra.32
 [0x00000161b8c7]
extensions::ComponentLoader::ComponentExtensionInfo::ComponentExtensionInfo()
 [0x00000161c83c] extensions::ComponentLoader::Add()
 [0x00000161cf2d] extensions::ComponentLoader::Add()
 [0x000000740582] ExtensionBrowserTest::LoadExtensionAsComponentWithManifest()
 [0x000000730783] ExtensionApiTest::RunExtensionTestImpl()
 [0x000000730b64] ExtensionApiTest::RunComponentExtensionTest()
 [0x000000555f41] CommandLinePrivateApiTest_Basics_Test::RunTestOnMainThread()

[方觉(Fang Jue), 2013-04-18 05:51:38]

"key" is added to manifest.json. I followed the instruction in
https://developer.chrome.com/extensions/manifest.html#key to get the key.

[not at google - send to devlin, 2013-04-18 16:17:55]

Cool, lgtm (you don't have to wait for trivial changes like that)

[commit-bot: I haz the power, 2013-04-19 04:52:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fangjue23303@gmail.com/14027005/58002

[commit-bot: I haz the power, 2013-04-19 04:52:18]

Failed to apply patch for
chrome/browser/extensions/extension_function_histogram_value.h:
While running patch -p0 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/extensions/extension_function_histogram_value.h
  Hunk #1 FAILED at 503.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/extensions/extension_function_histogram_value.h.rej

Patch:       chrome/browser/extensions/extension_function_histogram_value.h
Index: chrome/browser/extensions/extension_function_histogram_value.h
===================================================================
--- chrome/browser/extensions/extension_function_histogram_value.h	(revision
194758)
+++ chrome/browser/extensions/extension_function_histogram_value.h	(working
copy)
@@ -503,6 +503,7 @@
   APP_CURRENTWINDOWINTERNAL_FULLSCREEN,
   DEVELOPERPRIVATE_LOADUNPACKEDCROS,
   NETWORKINGPRIVATE_REQUESTNETWORKSCAN,
+  COMMANDLINEPRIVATE_HASSWITCH,
   ENUM_BOUNDARY // Last entry: Add new entries above.
 };

[commit-bot: I haz the power, 2013-04-19 04:57:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fangjue23303@gmail.com/14027005/67002

[commit-bot: I haz the power, 2013-04-19 05:31:32]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win7_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
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-04-19 05:53:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fangjue23303@gmail.com/14027005/72003

[commit-bot: I haz the power, 2013-04-19 06:23:44]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win7_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...
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-04-19 10:58:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fangjue23303@gmail.com/14027005/72003

[commit-bot: I haz the power, 2013-04-19 10:59:04]

Failed to apply patch for
chrome/browser/extensions/extension_function_histogram_value.h:
While running patch -p0 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/extensions/extension_function_histogram_value.h
  Hunk #1 FAILED at 505.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/extensions/extension_function_histogram_value.h.rej

Patch:       chrome/browser/extensions/extension_function_histogram_value.h
Index: chrome/browser/extensions/extension_function_histogram_value.h
===================================================================
--- chrome/browser/extensions/extension_function_histogram_value.h	(revision
195056)
+++ chrome/browser/extensions/extension_function_histogram_value.h	(working
copy)
@@ -505,6 +505,7 @@
   NETWORKINGPRIVATE_REQUESTNETWORKSCAN,
   ENTERPRISE_PLATFORMKEYSPRIVATE_CHALLENGEMACHINEKEY,
   ENTERPRISE_PLATFORMKEYSPRIVATE_CHALLENGEUSERKEY,
+  COMMANDLINEPRIVATE_HASSWITCH,
   ENUM_BOUNDARY // Last entry: Add new entries above.
 };

[commit-bot: I haz the power, 2013-04-19 11:03:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fangjue23303@gmail.com/14027005/78004

[commit-bot: I haz the power, 2013-04-19 13:28:18]

Change committed as 195161