Added a private chromeAuthPrivate API.

Added a private chromeAuthPrivate API.
Ceated a component hosted app that uses this to set cloud print credentials.


BUG=None
TEST=Unit-tests


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

[sanjeevr, 2011-06-24 17:58:17]

I added the updated generated documentation to the CL but since this is a
private API, I am not sure this is necessary. Please advise.

[Matt Perry, 2011-06-24 19:11:09]

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_chrome_auth_private_api.cc (right):

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_chrome_auth_private_api.cc:59:
result_.reset(Value::CreateStringValue(""));
you can omit this if you make the callback param optional

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_chrome_auth_private_apitest.cc (right):

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_chrome_auth_private_apitest.cc:37:
std::string("files/extensions/api_test/chrome_auth_private/") + path);
fyi, "foo" + path should work since path is a std::string.

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_chrome_auth_private_apitest.cc:39: //
Replace the host with 'www.example.com' so it matches the web store
comment doesn't match

http://codereview.chromium.org/7253001/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.json (right):

http://codereview.chromium.org/7253001/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.json:6092: "description": "A string
result code, which will be empty upon success."
if you make this optional, you can just omit it when not testing.

http://codereview.chromium.org/7253001/diff/1/chrome/test/data/extensions/api...
File
chrome/test/data/extensions/api_test/chrome_auth_private/enable_chrome_connector/cloud_print_success_tests.js
(right):

http://codereview.chromium.org/7253001/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/chrome_auth_private/enable_chrome_connector/cloud_print_success_tests.js:13:
chrome.test.callbackPass(function(result) {
indent -1

http://codereview.chromium.org/7253001/diff/1/chrome/test/data/extensions/api...
File
chrome/test/data/extensions/api_test/chrome_auth_private/installed_component_app/cloud_print_fail.js
(right):

http://codereview.chromium.org/7253001/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/chrome_auth_private/installed_component_app/cloud_print_fail.js:13:
chrome.chromeAuthPrivate.setCloudPrintCredentials(
fix indent

[sanjeevr, 2011-06-24 22:38:48]

Made review changes and merged to ToT. PTAL

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_chrome_auth_private_api.cc (right):

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_chrome_auth_private_api.cc:59:
result_.reset(Value::CreateStringValue(""));
On 2011/06/24 19:11:09, Matt Perry wrote:
> you can omit this if you make the callback param optional

The callback param is optional. However I got an ASSERT somewhere when I called
SendResponse without having set result_. I changed the code to call SendResponse
only for tests.

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_chrome_auth_private_apitest.cc (right):

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_chrome_auth_private_apitest.cc:37:
std::string("files/extensions/api_test/chrome_auth_private/") + path);
On 2011/06/24 19:11:09, Matt Perry wrote:
> fyi, "foo" + path should work since path is a std::string.

Done. I had copied this from the webstore tests.

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_chrome_auth_private_apitest.cc:39: //
Replace the host with 'www.example.com' so it matches the web store
On 2011/06/24 19:11:09, Matt Perry wrote:
> comment doesn't match

Done.

http://codereview.chromium.org/7253001/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.json (right):

http://codereview.chromium.org/7253001/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.json:6092: "description": "A string
result code, which will be empty upon success."
On 2011/06/24 19:11:09, Matt Perry wrote:
> if you make this optional, you can just omit it when not testing.

You mean make the result argument optional?

http://codereview.chromium.org/7253001/diff/1/chrome/test/data/extensions/api...
File
chrome/test/data/extensions/api_test/chrome_auth_private/enable_chrome_connector/cloud_print_success_tests.js
(right):

http://codereview.chromium.org/7253001/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/chrome_auth_private/enable_chrome_connector/cloud_print_success_tests.js:13:
chrome.test.callbackPass(function(result) {
On 2011/06/24 19:11:09, Matt Perry wrote:
> indent -1

Done.

http://codereview.chromium.org/7253001/diff/1/chrome/test/data/extensions/api...
File
chrome/test/data/extensions/api_test/chrome_auth_private/installed_component_app/cloud_print_fail.js
(right):

http://codereview.chromium.org/7253001/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/chrome_auth_private/installed_component_app/cloud_print_fail.js:13:
chrome.chromeAuthPrivate.setCloudPrintCredentials(
On 2011/06/24 19:11:09, Matt Perry wrote:
> fix indent

Done.

[Matt Perry, 2011-06-24 22:44:29]

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_chrome_auth_private_api.cc (right):

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_chrome_auth_private_api.cc:59:
result_.reset(Value::CreateStringValue(""));
On 2011/06/24 22:38:48, sanjeevr wrote:
> On 2011/06/24 19:11:09, Matt Perry wrote:
> > you can omit this if you make the callback param optional
> 
> The callback param is optional. However I got an ASSERT somewhere when I
called
> SendResponse without having set result_. I changed the code to call
SendResponse
> only for tests.

You should still call SendResponse, or some state in the renderer won't get
cleaned up. In this patch, the callback param isn't marked with "optional": true
in extension_api.json. Did you try that?

http://codereview.chromium.org/7253001/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.json (right):

http://codereview.chromium.org/7253001/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.json:6092: "description": "A string
result code, which will be empty upon success."
On 2011/06/24 22:38:48, sanjeevr wrote:
> On 2011/06/24 19:11:09, Matt Perry wrote:
> > if you make this optional, you can just omit it when not testing.
> 
> You mean make the result argument optional?

right

http://codereview.chromium.org/7253001/diff/6001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_chrome_auth_private_apitest.cc (right):

http://codereview.chromium.org/7253001/diff/6001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_chrome_auth_private_apitest.cc:53:
SetCloudPrintCredentialsFunction::SetTestMode(true);
since this sets a global variable, do you need to set it back to false after
this test?

[sanjeevr, 2011-06-24 23:05:26]

Changes made.

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_chrome_auth_private_api.cc (right):

http://codereview.chromium.org/7253001/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_chrome_auth_private_api.cc:59:
result_.reset(Value::CreateStringValue(""));
On 2011/06/24 22:44:29, Matt Perry wrote:
> On 2011/06/24 22:38:48, sanjeevr wrote:
> > On 2011/06/24 19:11:09, Matt Perry wrote:
> > > you can omit this if you make the callback param optional
> > 
> > The callback param is optional. However I got an ASSERT somewhere when I
> called
> > SendResponse without having set result_. I changed the code to call
> SendResponse
> > only for tests.
> 
> You should still call SendResponse, or some state in the renderer won't get
> cleaned up. In this patch, the callback param isn't marked with "optional":
true
> in extension_api.json. Did you try that?

Done.

http://codereview.chromium.org/7253001/diff/1/chrome/common/extensions/api/ex...
File chrome/common/extensions/api/extension_api.json (right):

http://codereview.chromium.org/7253001/diff/1/chrome/common/extensions/api/ex...
chrome/common/extensions/api/extension_api.json:6092: "description": "A string
result code, which will be empty upon success."
On 2011/06/24 22:44:29, Matt Perry wrote:
> On 2011/06/24 22:38:48, sanjeevr wrote:
> > On 2011/06/24 19:11:09, Matt Perry wrote:
> > > if you make this optional, you can just omit it when not testing.
> > 
> > You mean make the result argument optional?
> 
> right

Done.

http://codereview.chromium.org/7253001/diff/6001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_chrome_auth_private_apitest.cc (right):

http://codereview.chromium.org/7253001/diff/6001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_chrome_auth_private_apitest.cc:53:
SetCloudPrintCredentialsFunction::SetTestMode(true);
On 2011/06/24 22:44:30, Matt Perry wrote:
> since this sets a global variable, do you need to set it back to false after
> this test?

Done.

[Matt Perry, 2011-06-24 23:07:19]

LGTM

[commit-bot: I haz the power, 2011-06-25 06:26:04]

Change committed as 90496