diff options
author | rogerta@google.com <rogerta@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-09-13 18:18:18 +0000 |
---|---|---|
committer | rogerta@google.com <rogerta@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-09-13 18:18:18 +0000 |
commit | f0eb8fe0a5426d157b02326d94c6c76ff6f6b7bc (patch) | |
tree | ce415302e4293700c8f4d99f63075e6bb9e4f208 | |
parent | ffa59d622418c4093bc4adefe80201af0ab11e56 (diff) | |
download | chromium_src-f0eb8fe0a5426d157b02326d94c6c76ff6f6b7bc.zip chromium_src-f0eb8fe0a5426d157b02326d94c6c76ff6f6b7bc.tar.gz chromium_src-f0eb8fe0a5426d157b02326d94c6c76ff6f6b7bc.tar.bz2 |
Revert 59138 - Revert 59123 - Don't use the return value of rlz_lib::SendFinancialPing() as the return value
of the chrome extension API, since a false does not represent an error. Added
an optional callback to the API so that caller can determine if the ping was
sent or not.
This CL is to reapply change 58827, which was reverted due to a test break.
I found an uninitialized variable in the underlying RLZ code, see
http://code.google.com/p/rlz/source/detail?r=18 for the fix. This CL includes
a bump of the RLZ library to get this fix. The tests have been expanded a bit
too since 58827.
BUG=54294
TEST=n/a
Review URL: http://codereview.chromium.org/3350016
Review URL: http://codereview.chromium.org/3330019
TBR=sky@chromium.org
Review URL: http://codereview.chromium.org/3323028
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@59248 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | DEPS | 2 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_rlz_apitest.cc | 6 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_rlz_module.cc | 16 | ||||
-rw-r--r-- | chrome/common/extensions/api/extension_api.json | 3 | ||||
-rw-r--r-- | chrome/test/data/extensions/api_test/rlz/test.js | 27 |
5 files changed, 43 insertions, 11 deletions
@@ -217,7 +217,7 @@ deps_os = { Var("nacl_revision")), "src/rlz": - (Var("googlecode_url") % "rlz") + "/trunk@17", + (Var("googlecode_url") % "rlz") + "/trunk@18", }, "mac": { "src/chrome/tools/test/reference_build/chrome_mac": diff --git a/chrome/browser/extensions/extension_rlz_apitest.cc b/chrome/browser/extensions/extension_rlz_apitest.cc index c9c1416..ce97ae0f 100644 --- a/chrome/browser/extensions/extension_rlz_apitest.cc +++ b/chrome/browser/extensions/extension_rlz_apitest.cc @@ -55,6 +55,10 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Rlz) { KEY_READ); ASSERT_FALSE(key.Valid()); + key.Open(HKEY_CURRENT_USER, L"Software\\Google\\Common\\Rlz\\Events\\D", + KEY_READ); + ASSERT_FALSE(key.Valid()); + // Mock out experimental.rlz.sendFinancialPing(). ASSERT_TRUE(ExtensionFunctionDispatcher::OverrideFunction( "experimental.rlz.sendFinancialPing", @@ -66,7 +70,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Rlz) { // Now run all the tests. ASSERT_TRUE(RunExtensionTest("rlz")) << message_; - ASSERT_EQ(1, MockRlzSendFinancialPingFunction::expected_count()); + ASSERT_EQ(3, MockRlzSendFinancialPingFunction::expected_count()); ExtensionFunctionDispatcher::ResetFunctions(); // Now make sure we recorded what was expected. If the code in test.js diff --git a/chrome/browser/extensions/extension_rlz_module.cc b/chrome/browser/extensions/extension_rlz_module.cc index 25724be..e31fc69 100644 --- a/chrome/browser/extensions/extension_rlz_module.cc +++ b/chrome/browser/extensions/extension_rlz_module.cc @@ -144,10 +144,18 @@ bool RlzSendFinancialPingFunction::RunImpl() { bool exclude_machine_id; EXTENSION_FUNCTION_VALIDATE(args_->GetBoolean(6, &exclude_machine_id)); - return rlz_lib::SendFinancialPing(product, access_points.get(), - signature.c_str(), brand.c_str(), - id.c_str(), lang.c_str(), - exclude_machine_id); + // rlz_lib::SendFinancialPing() will not send a ping more often than once in + // any 24-hour period. Calling it more often has no effect. If a ping is + // not sent false is returned, but this is not an error, so we should not + // use the return value of rlz_lib::SendFinancialPing() as the return value + // of this function. Callers interested in the return value can register + // an optional callback function. + bool sent = rlz_lib::SendFinancialPing(product, access_points.get(), + signature.c_str(), brand.c_str(), + id.c_str(), lang.c_str(), + exclude_machine_id); + result_.reset(Value::CreateBooleanValue(sent)); + return true; } bool RlzClearProductStateFunction::RunImpl() { diff --git a/chrome/common/extensions/api/extension_api.json b/chrome/common/extensions/api/extension_api.json index 21d2292..0583904 100644 --- a/chrome/common/extensions/api/extension_api.json +++ b/chrome/common/extensions/api/extension_api.json @@ -3141,7 +3141,8 @@ {"name": "brand", "type": "string"}, {"name": "id", "type": "string"}, {"name": "lang", "type": "string"}, - {"name": "exclude_machine_id", "type": "boolean"} + {"name": "exclude_machine_id", "type": "boolean"}, + {"name": "callback", "type": "function", "optional": true, "parameters": [{"name": "sent", "type": "boolean"}]} ] }, { diff --git a/chrome/test/data/extensions/api_test/rlz/test.js b/chrome/test/data/extensions/api_test/rlz/test.js index 6fb4239..1214b6d 100644 --- a/chrome/test/data/extensions/api_test/rlz/test.js +++ b/chrome/test/data/extensions/api_test/rlz/test.js @@ -135,11 +135,30 @@ chrome.test.runTests([ } catch(ex) { } - // Valid call. + // Valid call. Should send a ping. + chrome.experimental.rlz.sendFinancialPing('D', ['D3'], 'sig', 'TEST', + 'id', 'en', false, + function(sent) { + if (sent) { + chrome.test.succeed(); + } else { + chrome.test.fail(); + } + }); + + // Try another call, this time the ping should not be sent. + chrome.experimental.rlz.sendFinancialPing('D', ['D3'], 'sig', 'TEST', + 'id', 'en', false, + function(sent) { + if (sent) { + chrome.test.fail(); + } else { + chrome.test.succeed(); + } + }); + + // Valid call. Test that callback does not need to be specified. chrome.experimental.rlz.sendFinancialPing('D', ['D3'], 'sig', 'TEST', 'id', 'en', false); - - chrome.test.succeed(); } ]); - |