diff options
author | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-26 22:29:17 +0000 |
---|---|---|
committer | thakis@chromium.org <thakis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-26 22:29:17 +0000 |
commit | 81d9b72d22a4fc9c8569cffe49018b67b2fb7844 (patch) | |
tree | 42c661f18f0a96431269656f701bbb9d745946df | |
parent | c127ae44a93140bc63dd2a423f3571e651474bef (diff) | |
download | chromium_src-81d9b72d22a4fc9c8569cffe49018b67b2fb7844.zip chromium_src-81d9b72d22a4fc9c8569cffe49018b67b2fb7844.tar.gz chromium_src-81d9b72d22a4fc9c8569cffe49018b67b2fb7844.tar.bz2 |
rlz: Hook up on mac, switch to chrome's network stack on win.
This CL conceptually does several things (most of them just one line).
1. Roll RLZ 105:118
106: Fix "expression result unused" warning caused by VERIFY() use.
107: rlz: Add an implementation of PingServer() that uses chrome's net stack.
108: Implement RlzValueStoreMac.
109: Move GetMachineId() to its own file. No intended behavior change.
110: Implement GetSystemTimeAsInt64() on mac.
111: Minor cleanups.
112: Don't pay a static initializer for expected_assertion_ when it's not used.
113: Rename rlz_lib2.cc and win/lib/rlz_lib.cc to win/lib/rlz_lib_win.cc
114: mac: Implement GetMachineId().
115: mac: Implement the locking part of ScopedRlzValueStoreLock.
116: Tweaks to make the use of chrome's net stack forceable through gyp.
117: Push RLZ_NETWORK_IMPLEMENTATION_ define to dependent targets.
118: Use base::mac::ScopedNSAutorleasePool only on mac.
2. Change rlz.cpp to use the blocking pool instead of the file thread.
3. Enable on mac.
4. Switch to chrome's network stack on windows
5. Switch RlzSendFinancialPingFunction to be an AsyncExtensionFunction
that calls SendFinancialPing on a worker thread. This is required because
extension functions run with a MessageLoop, so the MessageLoop in
SendFinancialPing in rlz would trigger an assert (and making that inner
loop nestable seems like a very bad idea). This change also removes
one instance of ScopedAllowIO and fixes a TODO.
BUG=46579
TEST=
1.) Do an official chrome build
2.) Add gratuitous logging in rlz.cc and other places and check that by default:
* The channel is reported as "stable"
* The brand code is the empty string
* This brand code counts as organic install
* RLZ exits early.
3.) Create ~/Library/Google/Google\ Chrome\ Brand.plist and add e.g. the string "BRAND" for key KSBrandID, restart chrome.
* Brand code is now "BRAND" (this depends on Chrome's Info.plist not having a KSBrandID key, which has precedence. Currently our Chromes seem to never have this key.)
* A ping is scheduled, but nothing is sent.
* Use the omnibox a little, which causes product events to be recorded.
4.) Restart chrome yet again, wait a bit.
* Logging in "SendFinancialPing()" should print:
pinging http://clients1.google.com:80/tools/pso/ping?as=chrome&brand=BRAND&pid=&hl=en&events=C1F,C1S&rep=2&rlz=C1:1C1_____enUS476,C2:1C2_____enUS476&id=0926C138C2EA77A791CB450D322D0183E5A8079300000001B5
ping completed!
TBR=sky
Review URL: https://chromiumcodereview.appspot.com/9699054
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@129028 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | DEPS | 2 | ||||
-rw-r--r-- | build/common.gypi | 7 | ||||
-rw-r--r-- | chrome/browser/browser_shutdown.cc | 5 | ||||
-rw-r--r-- | chrome/browser/chrome_browser_main.cc | 11 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_function_registry.cc | 2 | ||||
-rw-r--r-- | chrome/browser/rlz/rlz.cc | 45 | ||||
-rw-r--r-- | chrome/browser/rlz/rlz.h | 9 | ||||
-rw-r--r-- | chrome/browser/rlz/rlz_extension_api.cc | 70 | ||||
-rw-r--r-- | chrome/browser/rlz/rlz_extension_api.h | 24 | ||||
-rw-r--r-- | chrome/browser/rlz/rlz_extension_apitest.cc | 21 | ||||
-rw-r--r-- | chrome/browser/rlz/rlz_unittest.cc | 69 | ||||
-rw-r--r-- | chrome/browser/search_engines/search_terms_data.cc | 6 | ||||
-rw-r--r-- | chrome/browser/search_engines/search_terms_data.h | 4 | ||||
-rw-r--r-- | chrome/browser/search_engines/template_url.cc | 5 | ||||
-rw-r--r-- | chrome/browser/search_engines/template_url_service.cc | 2 | ||||
-rw-r--r-- | chrome/browser/search_engines/template_url_unittest.cc | 14 | ||||
-rw-r--r-- | chrome/chrome_browser.gypi | 16 | ||||
-rw-r--r-- | chrome/chrome_browser_extensions.gypi | 5 | ||||
-rw-r--r-- | chrome/chrome_tests.gypi | 12 | ||||
-rw-r--r-- | chrome/test/data/extensions/api_test/rlz/test.js | 36 |
20 files changed, 251 insertions, 114 deletions
@@ -58,7 +58,7 @@ vars = { "webrtc_revision": "1935", "jsoncpp_revision": "248", "nss_revision": "126189", - "rlz_revision": "105", + "rlz_revision": "118", } deps = { diff --git a/build/common.gypi b/build/common.gypi index 65b8dad..42de325 100644 --- a/build/common.gypi +++ b/build/common.gypi @@ -282,6 +282,9 @@ # GYP_DEFINES. 'tests_run%': 'check', + # Force rlz to use chrome's networking stack. + 'force_rlz_use_chrome_net%': 1, + 'conditions': [ # TODO(epoger): Figure out how to set use_skia=1 for Mac outside of # the 'conditions' clause. Initial attempts resulted in chromium and @@ -500,6 +503,7 @@ 'use_canvas_skia%': '<(use_canvas_skia)', 'tests_run%': '<(tests_run)', 'enable_automation%': '<(enable_automation)', + 'force_rlz_use_chrome_net%': '<(force_rlz_use_chrome_net)', # Whether to build for Wayland display server 'use_wayland%': 0, @@ -1143,6 +1147,9 @@ }, { # else: branding!="Chrome" 'defines': ['CHROMIUM_BUILD'], }], + ['branding=="Chrome" and (OS=="win" or OS=="mac")', { + 'defines': ['ENABLE_RLZ'], + }], ['component=="shared_library"', { 'defines': ['COMPONENT_BUILD'], }], diff --git a/chrome/browser/browser_shutdown.cc b/chrome/browser/browser_shutdown.cc index daffffb..5cd0114 100644 --- a/chrome/browser/browser_shutdown.cc +++ b/chrome/browser/browser_shutdown.cc @@ -42,6 +42,9 @@ #if defined(OS_WIN) #include "chrome/browser/browser_util_win.h" #include "chrome/browser/first_run/upgrade_util_win.h" +#endif + +#if defined(ENABLE_RLZ) #include "chrome/browser/rlz/rlz.h" #endif @@ -155,7 +158,7 @@ bool ShutdownPreThreadsStop() { prefs->CommitPendingWrite(); -#if defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD) +#if defined(ENABLE_RLZ) // Cleanup any statics created by RLZ. Must be done before NotificationService // is destroyed. RLZTracker::CleanupRlz(); diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index 70b629a..6413b349 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -150,7 +150,6 @@ #include "chrome/browser/first_run/try_chrome_dialog_view.h" #include "chrome/browser/first_run/upgrade_util_win.h" #include "chrome/browser/net/url_fixer_upper.h" -#include "chrome/browser/rlz/rlz.h" #include "chrome/browser/ui/views/user_data_dir_dialog.h" #include "chrome/installer/util/helper.h" #include "chrome/installer/util/install_util.h" @@ -168,6 +167,10 @@ #include "chrome/browser/mac/keystone_glue.h" #endif +#if defined(ENABLE_RLZ) +#include "chrome/browser/rlz/rlz.h" +#endif + #if defined(TOOLKIT_VIEWS) #include "ui/views/focus/accelerator_handler.h" #endif @@ -1609,8 +1612,9 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { ChromeBrowserMainPartsWin::RegisterApplicationRestart( parsed_command_line()); } +#endif // OS_WIN -#if defined(GOOGLE_CHROME_BUILD) +#if defined(ENABLE_RLZ) // Init the RLZ library. This just binds the dll and schedules a task on the // file thread to be run sometime later. If this is the first run we record // the installation event. @@ -1642,8 +1646,7 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() { // for the startup page if needed (i.e., when the startup page is set to // the home page). RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_HOME_PAGE, NULL); -#endif // GOOGLE_CHROME_BUILD -#endif // OS_WIN +#endif // defined(ENABLE_RLZ) // Configure modules that need access to resources. net::NetModule::SetResourceProvider(chrome_common_net::NetResourceProvider); diff --git a/chrome/browser/extensions/extension_function_registry.cc b/chrome/browser/extensions/extension_function_registry.cc index a6773b9..83f0c46 100644 --- a/chrome/browser/extensions/extension_function_registry.cc +++ b/chrome/browser/extensions/extension_function_registry.cc @@ -208,7 +208,7 @@ void ExtensionFunctionRegistry::ResetFunctions() { RegisterFunction<MetricsRecordLongTimeFunction>(); // RLZ. -#if defined(OS_WIN) +#if defined(OS_WIN) || defined(OS_MACOSX) RegisterFunction<RlzRecordProductEventFunction>(); RegisterFunction<RlzGetAccessPointRlzFunction>(); RegisterFunction<RlzSendFinancialPingFunction>(); diff --git a/chrome/browser/rlz/rlz.cc b/chrome/browser/rlz/rlz.cc index 8024106..5d9c8df 100644 --- a/chrome/browser/rlz/rlz.cc +++ b/chrome/browser/rlz/rlz.cc @@ -11,13 +11,8 @@ #include <algorithm> #include "base/bind.h" -#include "base/bind_helpers.h" -#include "base/file_path.h" #include "base/message_loop.h" -#include "base/path_service.h" #include "base/string_util.h" -#include "base/synchronization/lock.h" -#include "base/threading/thread.h" #include "base/threading/thread_restrictions.h" #include "base/utf_string_conversions.h" #include "chrome/browser/browser_process.h" @@ -26,13 +21,31 @@ #include "chrome/browser/search_engines/template_url_service.h" #include "chrome/browser/search_engines/template_url_service_factory.h" #include "chrome/common/chrome_notification_types.h" -#include "chrome/common/chrome_paths.h" -#include "chrome/common/env_vars.h" -#include "chrome/installer/util/google_update_settings.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/navigation_entry.h" #include "content/public/browser/notification_service.h" +#if defined(OS_WIN) +#include "chrome/installer/util/google_update_settings.h" +#else +namespace GoogleUpdateSettings { +static bool GetLanguage(string16* language) { + // TODO(thakis): Implement. + NOTIMPLEMENTED(); + return false; +} + +// The referral program is defunct and not used. No need to implement these +// functions on mac. +static bool GetReferral(string16* referral) { + return true; +} +static bool ClearReferral() { + return true; +} +} // namespace GoogleUpdateSettings +#endif + using content::BrowserThread; using content::NavigationEntry; @@ -188,6 +201,7 @@ bool RLZTracker::Init(bool first_run, int delay, bool google_default_search, void RLZTracker::ScheduleDelayedInit(int delay) { // The RLZTracker is a singleton object that outlives any runnable tasks // that will be queued up. + // TODO: Move to SequencedWorkerPool once http://crbug.com/119657 is fixed. BrowserThread::PostDelayedTask( BrowserThread::FILE, FROM_HERE, @@ -196,8 +210,12 @@ void RLZTracker::ScheduleDelayedInit(int delay) { } void RLZTracker::DelayedInit() { + worker_pool_token_ = BrowserThread::GetBlockingPool()->GetSequenceToken(); + bool schedule_ping = false; + rlz_lib::SetURLRequestContext(g_browser_process->system_request_context()); + // For organic brandcodes do not use rlz at all. Empty brandcode usually // means a chromium install. This is ok. std::string brand; @@ -227,10 +245,8 @@ void RLZTracker::DelayedInit() { } void RLZTracker::ScheduleFinancialPing() { - // TODO(thakis): Once akalin's TaskRunner around PostTask() is done, - // use that instead of the file thread. - BrowserThread::PostTask( - BrowserThread::FILE, + BrowserThread::GetBlockingPool()->PostSequencedWorkerTask( + worker_pool_token_, FROM_HERE, base::Bind(&RLZTracker::PingNowImpl, base::Unretained(this))); } @@ -385,8 +401,9 @@ bool RLZTracker::ScheduleGetAccessPointRlz(rlz_lib::AccessPoint point) { return false; string16* not_used = NULL; - BrowserThread::PostTask( - BrowserThread::FILE, FROM_HERE, + BrowserThread::GetBlockingPool()->PostSequencedWorkerTask( + worker_pool_token_, + FROM_HERE, base::Bind(base::IgnoreResult(&RLZTracker::GetAccessPointRlz), point, not_used)); return true; diff --git a/chrome/browser/rlz/rlz.h b/chrome/browser/rlz/rlz.h index c33214f..88bc3c2 100644 --- a/chrome/browser/rlz/rlz.h +++ b/chrome/browser/rlz/rlz.h @@ -8,7 +8,7 @@ #include "build/build_config.h" -#if defined(OS_WIN) +#if defined(OS_WIN) || defined(OS_MACOSX) #include <map> #include <string> @@ -16,6 +16,7 @@ #include "base/basictypes.h" #include "base/memory/singleton.h" #include "base/string16.h" +#include "base/threading/sequenced_worker_pool.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" #include "rlz/lib/rlz_lib.h" @@ -126,6 +127,10 @@ class RLZTracker : public content::NotificationObserver { bool google_default_search_; bool google_default_homepage_; + // Unique sequence token so that tasks posted by RLZTracker are executed + // sequentially in the blocking pool. + base::SequencedWorkerPool::SequenceToken worker_pool_token_; + // Keeps track if the RLZ tracker has already performed its delayed // initialization. bool already_ran_; @@ -145,6 +150,6 @@ class RLZTracker : public content::NotificationObserver { DISALLOW_COPY_AND_ASSIGN(RLZTracker); }; -#endif // defined(OS_WIN) +#endif // defined(OS_WIN) || defined(OS_MACOSX) #endif // CHROME_BROWSER_RLZ_RLZ_H_ diff --git a/chrome/browser/rlz/rlz_extension_api.cc b/chrome/browser/rlz/rlz_extension_api.cc index 2446770..9e761cad 100644 --- a/chrome/browser/rlz/rlz_extension_api.cc +++ b/chrome/browser/rlz/rlz_extension_api.cc @@ -4,11 +4,15 @@ #include "chrome/browser/rlz/rlz_extension_api.h" +#include "base/bind.h" #include "base/memory/scoped_ptr.h" +#include "base/threading/sequenced_worker_pool.h" #include "base/threading/thread_restrictions.h" #include "base/values.h" +#include "chrome/browser/browser_process.h" #include "chrome/common/extensions/extension.h" #include "rlz/lib/lib_values.h" +#include "rlz/lib/rlz_lib.h" namespace { @@ -113,16 +117,16 @@ bool RlzGetAccessPointRlzFunction::RunImpl() { return true; } -bool RlzSendFinancialPingFunction::RunImpl() { - // This can be slow if registry access goes to disk. Should preferably - // perform registry operations on the File thread. - // http://code.google.com/p/chromium/issues/detail?id=62098 - base::ThreadRestrictions::ScopedAllowIO allow_io; +RlzSendFinancialPingFunction::RlzSendFinancialPingFunction() { +} +RlzSendFinancialPingFunction::~RlzSendFinancialPingFunction() { +} + +bool RlzSendFinancialPingFunction::RunImpl() { std::string product_name; EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &product_name)); - rlz_lib::Product product; - EXTENSION_FUNCTION_VALIDATE(GetProductFromName(product_name, &product)); + EXTENSION_FUNCTION_VALIDATE(GetProductFromName(product_name, &product_)); ListValue* access_points_list; EXTENSION_FUNCTION_VALIDATE(args_->GetList(1, &access_points_list)); @@ -131,9 +135,9 @@ bool RlzSendFinancialPingFunction::RunImpl() { } // Allocate an access point array to pass to ClearProductState(). The array - // must be termindated with the value rlz_lib::NO_ACCESS_POINT, hence + 1 + // must be terminated with the value rlz_lib::NO_ACCESS_POINT, hence + 1 // when allocating the array. - scoped_array<rlz_lib::AccessPoint> access_points( + access_points_.reset( new rlz_lib::AccessPoint[access_points_list->GetSize() + 1]); size_t i; @@ -141,37 +145,49 @@ bool RlzSendFinancialPingFunction::RunImpl() { std::string ap_name; EXTENSION_FUNCTION_VALIDATE(access_points_list->GetString(i, &ap_name)); EXTENSION_FUNCTION_VALIDATE(rlz_lib::GetAccessPointFromName( - ap_name.c_str(), &access_points[i])); + ap_name.c_str(), &access_points_[i])); } - access_points[i] = rlz_lib::NO_ACCESS_POINT; + access_points_[i] = rlz_lib::NO_ACCESS_POINT; - std::string signature; - EXTENSION_FUNCTION_VALIDATE(args_->GetString(2, &signature)); + EXTENSION_FUNCTION_VALIDATE(args_->GetString(2, &signature_)); + EXTENSION_FUNCTION_VALIDATE(args_->GetString(3, &brand_)); + EXTENSION_FUNCTION_VALIDATE(args_->GetString(4, &id_)); + EXTENSION_FUNCTION_VALIDATE(args_->GetString(5, &lang_)); + EXTENSION_FUNCTION_VALIDATE(args_->GetBoolean(6, &exclude_machine_id_)); - std::string brand; - EXTENSION_FUNCTION_VALIDATE(args_->GetString(3, &brand)); + // |system_request_context| needs to run on the UI thread. + rlz_lib::SetURLRequestContext(g_browser_process->system_request_context()); - std::string id; - EXTENSION_FUNCTION_VALIDATE(args_->GetString(4, &id)); + content::BrowserThread::GetBlockingPool()->PostTask( + FROM_HERE, + base::Bind(&RlzSendFinancialPingFunction::WorkOnWorkerThread, this)); - std::string lang; - EXTENSION_FUNCTION_VALIDATE(args_->GetString(5, &lang)); - - bool exclude_machine_id; - EXTENSION_FUNCTION_VALIDATE(args_->GetBoolean(6, &exclude_machine_id)); + return true; +} +void RlzSendFinancialPingFunction::WorkOnWorkerThread() { // 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); + 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 post_task_result = content::BrowserThread::PostTask( + content::BrowserThread::UI, FROM_HERE, + base::Bind(&RlzSendFinancialPingFunction::RespondOnUIThread, this)); + DCHECK(post_task_result); +} + +void RlzSendFinancialPingFunction::RespondOnUIThread() { + DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); + SendResponse(true); } bool RlzClearProductStateFunction::RunImpl() { diff --git a/chrome/browser/rlz/rlz_extension_api.h b/chrome/browser/rlz/rlz_extension_api.h index d091c85..15c5835 100644 --- a/chrome/browser/rlz/rlz_extension_api.h +++ b/chrome/browser/rlz/rlz_extension_api.h @@ -8,9 +8,11 @@ #include "build/build_config.h" -#if defined(OS_WIN) +#if defined(OS_WIN) || defined(OS_MACOSX) +#include "base/memory/scoped_ptr.h" #include "chrome/browser/extensions/extension_function.h" +#include "rlz/lib/lib_values.h" class RlzRecordProductEventFunction : public SyncExtensionFunction { virtual bool RunImpl() OVERRIDE; @@ -22,11 +24,27 @@ class RlzGetAccessPointRlzFunction : public SyncExtensionFunction { DECLARE_EXTENSION_FUNCTION_NAME("experimental.rlz.getAccessPointRlz") }; -class RlzSendFinancialPingFunction : public SyncExtensionFunction { +class RlzSendFinancialPingFunction : public AsyncExtensionFunction { + public: + RlzSendFinancialPingFunction(); + virtual ~RlzSendFinancialPingFunction(); + DECLARE_EXTENSION_FUNCTION_NAME("experimental.rlz.sendFinancialPing") // Making this function protected so that it can be overridden in tests. protected: virtual bool RunImpl() OVERRIDE; + + private: + void WorkOnWorkerThread(); + void RespondOnUIThread(); + + rlz_lib::Product product_; + scoped_array<rlz_lib::AccessPoint> access_points_; + std::string signature_; + std::string brand_; + std::string id_; + std::string lang_; + bool exclude_machine_id_; }; class RlzClearProductStateFunction : public SyncExtensionFunction { @@ -34,6 +52,6 @@ class RlzClearProductStateFunction : public SyncExtensionFunction { DECLARE_EXTENSION_FUNCTION_NAME("experimental.rlz.clearProductState") }; -#endif // defined(OS_WIN) +#endif // defined(OS_WIN) || defined(OS_MACOSX) #endif // CHROME_BROWSER_RLZ_RLZ_EXTENSION_API_H_ diff --git a/chrome/browser/rlz/rlz_extension_apitest.cc b/chrome/browser/rlz/rlz_extension_apitest.cc index 454efa1..7a1810b 100644 --- a/chrome/browser/rlz/rlz_extension_apitest.cc +++ b/chrome/browser/rlz/rlz_extension_apitest.cc @@ -4,14 +4,19 @@ #include <map> -#include "base/win/registry.h" +#include "chrome/browser/browser_process.h" #include "chrome/browser/extensions/extension_function.h" #include "chrome/browser/extensions/extension_function_dispatcher.h" #include "chrome/browser/extensions/extension_apitest.h" #include "chrome/browser/rlz/rlz_extension_api.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/extensions/extension.h" -#include "rlz/win/lib/rlz_lib.h" +#include "net/base/mock_host_resolver.h" +#include "rlz/lib/rlz_lib.h" + +#if (OS_WIN) +#include "base/win/registry.h" +#endif class MockRlzSendFinancialPingFunction : public RlzSendFinancialPingFunction { virtual bool RunImpl(); @@ -37,6 +42,14 @@ ExtensionFunction* MockRlzSendFinancialPingFunctionFactory() { } IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Rlz) { + // The default test resolver doesn't allow lookups to *.google.com. That + // makes sense, but it does make RLZ's SendFinancialPing() fail -- so allow + // connections to google.com in this test. + scoped_refptr<net::RuleBasedHostResolverProc> resolver = + new net::RuleBasedHostResolverProc(host_resolver()); + resolver->AllowDirectLookup("*.google.com"); + net::ScopedDefaultHostResolverProc scoper(resolver); + CommandLine::ForCurrentProcess()->AppendSwitch( switches::kEnableExperimentalExtensionApis); @@ -49,6 +62,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Rlz) { rlz_lib::ClearProductState(rlz_lib::PINYIN_IME, access_points); rlz_lib::ClearProductState(rlz_lib::DESKTOP, access_points); +#if defined(OS_WIN) // Check that the state has really been cleared. base::win::RegKey key(HKEY_CURRENT_USER, L"Software\\Google\\Common\\Rlz\\Events\\N", @@ -58,6 +72,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Rlz) { key.Open(HKEY_CURRENT_USER, L"Software\\Google\\Common\\Rlz\\Events\\D", KEY_READ); ASSERT_FALSE(key.Valid()); +#endif // Mock out experimental.rlz.sendFinancialPing(). ASSERT_TRUE(ExtensionFunctionDispatcher::OverrideFunction( @@ -73,6 +88,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Rlz) { ASSERT_EQ(3, MockRlzSendFinancialPingFunction::expected_count()); ExtensionFunctionDispatcher::ResetFunctions(); +#if defined(OS_WIN) // Now make sure we recorded what was expected. If the code in test.js // changes, need to make appropriate changes here. key.Open(HKEY_CURRENT_USER, L"Software\\Google\\Common\\Rlz\\Events\\N", @@ -93,6 +109,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Rlz) { key.Open(HKEY_CURRENT_USER, L"Software\\Google\\Common\\Rlz\\Events\\D", KEY_READ); ASSERT_FALSE(key.Valid()); +#endif // Cleanup. rlz_lib::ClearProductState(rlz_lib::PINYIN_IME, access_points); diff --git a/chrome/browser/rlz/rlz_unittest.cc b/chrome/browser/rlz/rlz_unittest.cc index 33f04e3..cc19aa1 100644 --- a/chrome/browser/rlz/rlz_unittest.cc +++ b/chrome/browser/rlz/rlz_unittest.cc @@ -7,9 +7,8 @@ #include "base/memory/scoped_ptr.h" #include "base/stringprintf.h" #include "base/path_service.h" -#include "base/test/test_reg_util_win.h" +#include "base/scoped_temp_dir.h" #include "base/utf_string_conversions.h" -#include "base/win/registry.h" #include "chrome/browser/autocomplete/autocomplete.h" #include "chrome/browser/google/google_util.h" #include "chrome/browser/profiles/profile.h" @@ -21,21 +20,33 @@ #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_source.h" -#include "rlz/win/lib/rlz_lib.h" // InitializeTempHivesForTesting #include "testing/gtest/include/gtest/gtest.h" -using base::win::RegKey; +#if defined(OS_WIN) +#include "base/test/test_reg_util_win.h" +#include "base/win/registry.h" +#include "rlz/win/lib/rlz_lib.h" // InitializeTempHivesForTesting +#elif defined(OS_MACOSX) +#include "rlz/lib/rlz_value_store.h" // SetRlzStoreDirectory +#endif + using content::NavigationEntry; -using registry_util::RegistryOverrideManager; using testing::AssertionResult; using testing::AssertionSuccess; using testing::AssertionFailure; +#if defined(OS_WIN) +using base::win::RegKey; +using registry_util::RegistryOverrideManager; +#endif + namespace { +#if defined(OS_WIN) // Registry path to overridden hive. const wchar_t kRlzTempHkcu[] = L"rlz_hkcu"; const wchar_t kRlzTempHklm[] = L"rlz_hklm"; +#endif // Dummy RLZ string for the access points. const char kOmniboxRlzString[] = "test_omnibox"; @@ -146,7 +157,9 @@ class RlzLibTest : public testing::Test { protected: void SetMainBrand(const char* brand); void SetReactivationBrand(const char* brand); +#if defined(OS_WIN) void SetRegistryBrandValue(const wchar_t* name, const char* brand); +#endif void SimulateOmniboxUsage(); void SimulateHomepageUsage(); @@ -157,12 +170,18 @@ class RlzLibTest : public testing::Test { void ExpectReactivationRlzPingSent(bool expected); TestRLZTracker tracker_; +#if defined(OS_WIN) RegistryOverrideManager override_manager_; +#elif defined(OS_MACOSX) + ScopedTempDir temp_dir_; + scoped_ptr<google_util::BrandForTesting> brand_override_; +#endif }; void RlzLibTest::SetUp() { testing::Test::SetUp(); +#if defined(OS_WIN) // Before overriding HKLM for the tests, we need to set it up correctly // so that the rlz_lib calls work. This needs to be done before we do the // override. @@ -193,6 +212,10 @@ void RlzLibTest::SetUp() { // initialization performed above. override_manager_.OverrideRegistry(HKEY_LOCAL_MACHINE, kRlzTempHklm); override_manager_.OverrideRegistry(HKEY_CURRENT_USER, kRlzTempHkcu); +#elif defined(OS_MACOSX) + ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); + rlz_lib::testing::SetRlzStoreDirectory(temp_dir_.path()); +#endif // Make sure a non-organic brand code is set in the registry or the RLZTracker // is pretty much a no-op. @@ -201,23 +224,34 @@ void RlzLibTest::SetUp() { } void RlzLibTest::TearDown() { +#if defined(OS_MACOSX) + rlz_lib::testing::SetRlzStoreDirectory(FilePath()); +#endif testing::Test::TearDown(); } void RlzLibTest::SetMainBrand(const char* brand) { +#if defined(OS_WIN) SetRegistryBrandValue(google_update::kRegRLZBrandField, brand); +#elif defined(OS_MACOSX) + brand_override_.reset(new google_util::BrandForTesting(brand)); +#endif std::string check_brand; google_util::GetBrand(&check_brand); EXPECT_EQ(brand, check_brand); } void RlzLibTest::SetReactivationBrand(const char* brand) { + // TODO(thakis): Reactivation doesn't exist on Mac yet. +#if defined(OS_WIN) SetRegistryBrandValue(google_update::kRegRLZReactivationBrandField, brand); std::string check_brand; google_util::GetReactivationBrand(&check_brand); EXPECT_EQ(brand, check_brand); +#endif } +#if defined(OS_WIN) void RlzLibTest::SetRegistryBrandValue(const wchar_t* name, const char* brand) { BrowserDistribution* dist = BrowserDistribution::GetDistribution(); @@ -227,10 +261,11 @@ void RlzLibTest::SetRegistryBrandValue(const wchar_t* name, LONG result = key.DeleteValue(name); ASSERT_TRUE(ERROR_SUCCESS == result || ERROR_FILE_NOT_FOUND == result); } else { - string16 brand16 = ASCIIToWide(brand); + string16 brand16 = ASCIIToUTF16(brand); ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name, brand16.c_str())); } } +#endif void RlzLibTest::SimulateOmniboxUsage() { tracker_.Observe(chrome::NOTIFICATION_OMNIBOX_OPENED_URL, @@ -513,7 +548,7 @@ TEST_F(RlzLibTest, GetAccessPointRlzOnIoThread) { tracker_.set_assume_not_ui_thread(true); EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz)); - EXPECT_STREQ(kOmniboxRlzString, WideToUTF8(rlz).c_str()); + EXPECT_STREQ(kOmniboxRlzString, UTF16ToUTF8(rlz).c_str()); } TEST_F(RlzLibTest, GetAccessPointRlzNotOnIoThread) { @@ -537,11 +572,11 @@ TEST_F(RlzLibTest, GetAccessPointRlzIsCached) { tracker_.set_assume_not_ui_thread(true); EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz)); - EXPECT_STREQ(kOmniboxRlzString, WideToUTF8(rlz).c_str()); + EXPECT_STREQ(kOmniboxRlzString, UTF16ToUTF8(rlz).c_str()); tracker_.set_assume_not_ui_thread(false); EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz)); - EXPECT_STREQ(kOmniboxRlzString, WideToUTF8(rlz).c_str()); + EXPECT_STREQ(kOmniboxRlzString, UTF16ToUTF8(rlz).c_str()); } TEST_F(RlzLibTest, PingUpdatesRlzCache) { @@ -555,17 +590,17 @@ TEST_F(RlzLibTest, PingUpdatesRlzCache) { tracker_.set_assume_not_ui_thread(true); EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz)); - EXPECT_STREQ(kOmniboxRlzString, WideToUTF8(rlz).c_str()); + EXPECT_STREQ(kOmniboxRlzString, UTF16ToUTF8(rlz).c_str()); EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_HOME_PAGE, &rlz)); - EXPECT_STREQ(kHomepageRlzString, WideToUTF8(rlz).c_str()); + EXPECT_STREQ(kHomepageRlzString, UTF16ToUTF8(rlz).c_str()); // Make sure cache is valid. tracker_.set_assume_not_ui_thread(false); EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz)); - EXPECT_STREQ(kOmniboxRlzString, WideToUTF8(rlz).c_str()); + EXPECT_STREQ(kOmniboxRlzString, UTF16ToUTF8(rlz).c_str()); EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_HOME_PAGE, &rlz)); - EXPECT_STREQ(kHomepageRlzString, WideToUTF8(rlz).c_str()); + EXPECT_STREQ(kHomepageRlzString, UTF16ToUTF8(rlz).c_str()); // Perform ping. tracker_.set_assume_not_ui_thread(true); @@ -577,9 +612,9 @@ TEST_F(RlzLibTest, PingUpdatesRlzCache) { tracker_.set_assume_not_ui_thread(false); EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz)); - EXPECT_STREQ(kNewOmniboxRlzString, WideToUTF8(rlz).c_str()); + EXPECT_STREQ(kNewOmniboxRlzString, UTF16ToUTF8(rlz).c_str()); EXPECT_TRUE(RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_HOME_PAGE, &rlz)); - EXPECT_STREQ(kNewHomepageRlzString, WideToUTF8(rlz).c_str()); + EXPECT_STREQ(kNewHomepageRlzString, UTF16ToUTF8(rlz).c_str()); } TEST_F(RlzLibTest, ObserveHandlesBadArgs) { @@ -594,6 +629,8 @@ TEST_F(RlzLibTest, ObserveHandlesBadArgs) { content::Details<NavigationEntry>(entry.get())); } +// TODO(thakis): Reactivation doesn't exist on Mac yet. +#if defined(OS_WIN) TEST_F(RlzLibTest, ReactivationNonOrganicNonOrganic) { SetReactivationBrand("REAC"); @@ -636,4 +673,4 @@ TEST_F(RlzLibTest, ReactivationOrganicOrganic) { ExpectRlzPingSent(false); ExpectReactivationRlzPingSent(false); } - +#endif // defined(OS_WIN) diff --git a/chrome/browser/search_engines/search_terms_data.cc b/chrome/browser/search_engines/search_terms_data.cc index ab78bd3..080920e 100644 --- a/chrome/browser/search_engines/search_terms_data.cc +++ b/chrome/browser/search_engines/search_terms_data.cc @@ -13,7 +13,7 @@ #include "content/public/browser/browser_thread.h" #include "googleurl/src/gurl.h" -#if defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD) +#if defined(ENABLE_RLZ) #include "chrome/browser/google/google_util.h" #include "chrome/browser/rlz/rlz.h" #endif @@ -54,7 +54,7 @@ std::string SearchTermsData::GetApplicationLocale() const { return "en"; } -#if defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD) +#if defined(ENABLE_RLZ) string16 SearchTermsData::GetRlzParameterValue() const { return string16(); } @@ -89,7 +89,7 @@ std::string UIThreadSearchTermsData::GetApplicationLocale() const { return g_browser_process->GetApplicationLocale(); } -#if defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD) +#if defined(ENABLE_RLZ) string16 UIThreadSearchTermsData::GetRlzParameterValue() const { DCHECK(!BrowserThread::IsWellKnownThread(BrowserThread::UI) || BrowserThread::CurrentlyOn(BrowserThread::UI)); diff --git a/chrome/browser/search_engines/search_terms_data.h b/chrome/browser/search_engines/search_terms_data.h index c45716f..2345708 100644 --- a/chrome/browser/search_engines/search_terms_data.h +++ b/chrome/browser/search_engines/search_terms_data.h @@ -33,7 +33,7 @@ class SearchTermsData { // "en" and thus should be overridden where the result is actually meaningful. virtual std::string GetApplicationLocale() const; -#if defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD) +#if defined(ENABLE_RLZ) // Returns the value for the Chrome Omnibox rlz. This implementation returns // the empty string. virtual string16 GetRlzParameterValue() const; @@ -70,7 +70,7 @@ class UIThreadSearchTermsData : public SearchTermsData { // Implementation of SearchTermsData. virtual std::string GoogleBaseURLValue() const OVERRIDE; virtual std::string GetApplicationLocale() const OVERRIDE; -#if defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD) +#if defined(ENABLE_RLZ) virtual string16 GetRlzParameterValue() const OVERRIDE; #endif diff --git a/chrome/browser/search_engines/template_url.cc b/chrome/browser/search_engines/template_url.cc index 9981e52..b68fbbf 100644 --- a/chrome/browser/search_engines/template_url.cc +++ b/chrome/browser/search_engines/template_url.cc @@ -397,11 +397,10 @@ std::string TemplateURLRef::ReplaceSearchTermsUsingTermsData( // to happen so that we replace the RLZ template with the // empty string. (If we don't handle this case, we hit a // NOTREACHED below.) -#if defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD) +#if defined(ENABLE_RLZ) string16 rlz_string = search_terms_data.GetRlzParameterValue(); if (!rlz_string.empty()) { - rlz_string = L"rlz=" + rlz_string + L"&"; - url.insert(i->index, UTF16ToUTF8(rlz_string)); + url.insert(i->index, "rlz=" + UTF16ToUTF8(rlz_string) + "&"); } #endif break; diff --git a/chrome/browser/search_engines/template_url_service.cc b/chrome/browser/search_engines/template_url_service.cc index 3c40566..1b34f39 100644 --- a/chrome/browser/search_engines/template_url_service.cc +++ b/chrome/browser/search_engines/template_url_service.cc @@ -1587,7 +1587,7 @@ void TemplateURLService::SetDefaultSearchProviderNoNotify( const TemplateURLRef* url_ref = url->url(); if (url_ref && url_ref->HasGoogleBaseURLs()) { GoogleURLTracker::RequestServerCheck(); -#if defined(OS_WIN) && defined(GOOGLE_CHROME_BUILD) +#if defined(ENABLE_RLZ) // Needs to be evaluated. See http://crbug.com/62328. base::ThreadRestrictions::ScopedAllowIO allow_io; RLZTracker::RecordProductEvent(rlz_lib::CHROME, diff --git a/chrome/browser/search_engines/template_url_unittest.cc b/chrome/browser/search_engines/template_url_unittest.cc index 594978c..8ebd3070 100644 --- a/chrome/browser/search_engines/template_url_unittest.cc +++ b/chrome/browser/search_engines/template_url_unittest.cc @@ -11,6 +11,10 @@ #include "chrome/browser/search_engines/template_url.h" #include "testing/gtest/include/gtest/gtest.h" +#if defined(ENABLE_RLZ) +#include "chrome/browser/google/google_util.h" +#endif + // TestSearchTermsData -------------------------------------------------------- // Simple implementation of SearchTermsData. @@ -409,11 +413,15 @@ TEST_F(TemplateURLTest, Suggestions) { } } -#if defined(OS_WIN) +#if defined(OS_WIN) || defined(OS_MACOSX) TEST_F(TemplateURLTest, RLZ) { string16 rlz_string; -#if defined(GOOGLE_CHROME_BUILD) - RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz_string); +#if defined(ENABLE_RLZ) + std::string brand; + if (google_util::GetBrand(&brand) && !brand.empty() && + !google_util::IsOrganic(brand)) { + RLZTracker::GetAccessPointRlz(rlz_lib::CHROME_OMNIBOX, &rlz_string); + } #endif TemplateURL t_url; diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 2589374..a95ba51 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -4609,13 +4609,22 @@ }, ], }], + ['OS=="win" or OS=="mac"', { + 'dependencies': [ + '../rlz/rlz.gyp:rlz_lib', + ], + }, { # 'OS!="win" and OS!="mac" + 'sources/': [ + # Exclude all of rlz. + ['exclude', '^browser/rlz/'], + ], + }], ['OS=="win"', { 'include_dirs': [ '<(DEPTH)/third_party/wtl/include', ], 'dependencies': [ '../google_update/google_update.gyp:google_update', - '../rlz/rlz.gyp:rlz_lib', '../third_party/iaccessible2/iaccessible2.gyp:iaccessible2', '../third_party/isimpledom/isimpledom.gyp:isimpledom', '../ui/views/views.gyp:views', @@ -4660,10 +4669,6 @@ # Exclude parts of password_manager. ['exclude', '^browser/password_manager/ie7_password\\.cc$'], - # Exclude all of rlz. - ['exclude', '^browser/rlz/'], - ['exclude', '^browser/extensions/extension_rlz_module'], - # Exclude all of views. ['exclude', '^browser/ui/views/'], @@ -4698,7 +4703,6 @@ # This exclude duplicates the one just above because of the # order of evaluation of the 'sources/' rule above, the # conditions, and this 'sources/' rule. - ['exclude', '^browser/extensions/extension_rlz_module'], ['exclude', '^browser/google/google_update.cc'], ['exclude', '^browser/google/google_update.h'], ['exclude', '^browser/platform_util_common_linux.cc'], diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi index 83c2266..55b9d81 100644 --- a/chrome/chrome_browser_extensions.gypi +++ b/chrome/chrome_browser_extensions.gypi @@ -499,7 +499,6 @@ }], ['OS=="win"', { 'dependencies': [ - '../rlz/rlz.gyp:rlz_lib', '../third_party/iaccessible2/iaccessible2.gyp:iaccessible2', '../third_party/isimpledom/isimpledom.gyp:isimpledom', ], @@ -519,10 +518,6 @@ ], 'sources/': [ ['include', '^browser/extensions/'], - # This exclude duplicates the one just above because of the - # order of evaluation of the 'sources/' rule above, the - # conditions, and this 'sources/' rule. - ['exclude', '^browser/extensions/extension_rlz_module'], # Other excluded stuff. ['exclude', '^browser/extensions/browser_action_test_util_gtk.cc'], diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 68ba37d..b56da6a 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -2352,6 +2352,11 @@ 'browser/spellchecker/spellchecker_platform_engine_unittest.cc', ], }], + ['OS!="win" and OS!="mac"', { + 'sources!': [ + 'browser/rlz/rlz_unittest.cc', + ], + }], ['OS=="win"', { 'dependencies': [ 'chrome_version_resources', @@ -2413,7 +2418,6 @@ 'app/chrome_dll.rc', 'browser/accessibility/browser_accessibility_win_unittest.cc', 'browser/bookmarks/bookmark_node_data_unittest.cc', - 'browser/rlz/rlz_unittest.cc', 'browser/search_engines/template_url_scraper_unittest.cc', 'browser/ui/views/bookmarks/bookmark_editor_view_unittest.cc', 'browser/ui/views/extensions/browser_action_drag_data_unittest.cc', @@ -3082,6 +3086,11 @@ '../content/browser/accessibility/dump_accessibility_tree_helper.cc', ], }], + ['OS!="win" and OS!="mac"', { + 'sources!': [ + 'browser/rlz/rlz_extension_apitest.cc', + ], + }], ['OS=="win"', { 'sources': [ '<(SHARED_INTERMEDIATE_DIR)/chrome/browser_resources.rc', @@ -3128,7 +3137,6 @@ 'app/chrome_dll.rc', 'app/chrome_dll_resource.h', 'app/chrome_version.rc.version', - 'browser/rlz/rlz_extension_apitest.cc', ], }], ['toolkit_uses_gtk == 1', { diff --git a/chrome/test/data/extensions/api_test/rlz/test.js b/chrome/test/data/extensions/api_test/rlz/test.js index 1214b6d..086a6fb 100644 --- a/chrome/test/data/extensions/api_test/rlz/test.js +++ b/chrome/test/data/extensions/api_test/rlz/test.js @@ -138,24 +138,24 @@ chrome.test.runTests([ // 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(); - } - }); + 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', |