Skip to content

Commit

Permalink
Make use of InvalidationService
Browse files Browse the repository at this point in the history
The InvalidationService was introduced r199520.  That commit added the
InvalidationService interface and several implementations of it, but
made no use of the new code.  This commit builds on that work.

Up until now, TICL invalidations were handled on the sync thread.  The
related objects were instantiated and owned by the SyncBackendHost and
SyncManager.  All requests to update the set of object registrations had
to be passed to the sync thread.  Components that wanted to receive
invalidations but were not part of sync had to route their communication
with the invalidations server through ProfileSyncService to get to the
sync thread.  Things were a bit different on Android, but the system
still tried to pretend that invalidations were owned by the sync thread.

The new InvalidationService implementation is a ProfileKeyedService that
is mostly independent from sync.  It still relies on sync to manage sign
in and fetch the appropriate auth tokens.  However, it's now much easier
for components outside of sync to communication with the invalidations
server.

The new system allows us to remove a lot of invalidations-related code
from the ProfileSyncService, SyncBackendHost and SyncManager.  Sync is
now just one of many clients of the InvalidationService.  The
SyncBackendHost is responsible for forwarding messages back and forth
between the InvalidationService and the sync thread.

TBR=sky,erg
BUG=124137

Review URL: https://chromiumcodereview.appspot.com/15580002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@208315 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rlarocque@chromium.org committed Jun 25, 2013
1 parent ccdcf43 commit 389f566
Show file tree
Hide file tree
Showing 80 changed files with 953 additions and 2,042 deletions.
41 changes: 21 additions & 20 deletions chrome/browser/chrome_to_mobile_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
#include "base/strings/utf_string_conversions.h"
#include "chrome/app/chrome_command_ids.h"
#include "chrome/browser/chrome_to_mobile_service_factory.h"
#include "chrome/browser/invalidation/invalidation_service.h"
#include "chrome/browser/invalidation/invalidation_service_factory.h"
#include "chrome/browser/printing/cloud_print/cloud_print_url.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/token_service.h"
#include "chrome/browser/signin/token_service_factory.h"
#include "chrome/browser/sync/profile_sync_service.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_command_controller.h"
#include "chrome/browser/ui/browser_finder.h"
Expand Down Expand Up @@ -261,23 +261,24 @@ void ChromeToMobileService::RegisterUserPrefs(
ChromeToMobileService::ChromeToMobileService(Profile* profile)
: weak_ptr_factory_(this),
profile_(profile),
sync_invalidation_enabled_(false) {
invalidation_enabled_(false) {
// TODO(msw): Unit tests do not provide profiles; see http://crbug.com/122183
ProfileSyncService* profile_sync_service =
profile_ ? ProfileSyncServiceFactory::GetForProfile(profile_) : NULL;
if (profile_sync_service) {

invalidation::InvalidationService* invalidation_service = profile_ ?
invalidation::InvalidationServiceFactory::GetForProfile(profile_) : NULL;
if (invalidation_service) {
CloudPrintURL cloud_print_url(profile_);
cloud_print_url_ = cloud_print_url.GetCloudPrintServiceURL();
sync_invalidation_enabled_ =
(profile_sync_service->GetInvalidatorState() ==
invalidation_enabled_ =
(invalidation_service->GetInvalidatorState() ==
syncer::INVALIDATIONS_ENABLED);
// Register for cloud print device list invalidation notifications.
profile_sync_service->RegisterInvalidationHandler(this);
invalidation_service->RegisterInvalidationHandler(this);
syncer::ObjectIdSet ids;
ids.insert(invalidation::ObjectId(
ipc::invalidation::ObjectSource::CHROME_COMPONENTS,
kSyncInvalidationObjectIdChromeToMobileDeviceList));
profile_sync_service->UpdateRegisteredInvalidationIds(this, ids);
invalidation_service->UpdateRegisteredInvalidationIds(this, ids);
}
}

Expand All @@ -292,7 +293,7 @@ bool ChromeToMobileService::HasMobiles() const {
}

const base::ListValue* ChromeToMobileService::GetMobiles() const {
return sync_invalidation_enabled_ ?
return invalidation_enabled_ ?
profile_->GetPrefs()->GetList(prefs::kChromeToMobileDeviceList) : NULL;
}

Expand Down Expand Up @@ -384,10 +385,10 @@ void ChromeToMobileService::LearnMore(Browser* browser) const {
void ChromeToMobileService::Shutdown() {
// TODO(msw): Unit tests do not provide profiles; see http://crbug.com/122183
// Unregister for cloud print device list invalidation notifications.
ProfileSyncService* profile_sync_service =
profile_ ? ProfileSyncServiceFactory::GetForProfile(profile_) : NULL;
if (profile_sync_service)
profile_sync_service->UnregisterInvalidationHandler(this);
invalidation::InvalidationService* invalidation_service = profile_ ?
invalidation::InvalidationServiceFactory::GetForProfile(profile_) : NULL;
if (invalidation_service)
invalidation_service->UnregisterInvalidationHandler(this);
}

void ChromeToMobileService::OnURLFetchComplete(const net::URLFetcher* source) {
Expand Down Expand Up @@ -472,7 +473,7 @@ void ChromeToMobileService::OnGetTokenFailure(

void ChromeToMobileService::OnInvalidatorStateChange(
syncer::InvalidatorState state) {
sync_invalidation_enabled_ = (state == syncer::INVALIDATIONS_ENABLED);
invalidation_enabled_ = (state == syncer::INVALIDATIONS_ENABLED);
}

void ChromeToMobileService::OnIncomingInvalidation(
Expand All @@ -482,12 +483,12 @@ void ChromeToMobileService::OnIncomingInvalidation(
ipc::invalidation::ObjectSource::CHROME_COMPONENTS,
kSyncInvalidationObjectIdChromeToMobileDeviceList)));
// TODO(msw): Unit tests do not provide profiles; see http://crbug.com/122183
ProfileSyncService* profile_sync_service =
profile_ ? ProfileSyncServiceFactory::GetForProfile(profile_) : NULL;
if (profile_sync_service) {
invalidation::InvalidationService* invalidation_service = profile_ ?
invalidation::InvalidationServiceFactory::GetForProfile(profile_) : NULL;
if (invalidation_service) {
// TODO(dcheng): Only acknowledge the invalidation once the device search
// has finished. http://crbug.com/156843.
profile_sync_service->AcknowledgeInvalidation(
invalidation_service->AcknowledgeInvalidation(
invalidation_map.begin()->first,
invalidation_map.begin()->second.ack_handle);
}
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/chrome_to_mobile_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ class ChromeToMobileService : public BrowserContextKeyedService,

Profile* profile_;

// Sync invalidation service state. Chrome To Mobile requires this service to
// to keep the mobile device list up to date and prevent page send failures.
bool sync_invalidation_enabled_;
// Invalidation service state. Chrome To Mobile requires this service to keep
// the mobile device list up to date and prevent page send failures.
bool invalidation_enabled_;

// Used to recieve TokenService notifications for GaiaOAuth2LoginRefreshToken.
content::NotificationRegistrar registrar_;
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/chrome_to_mobile_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
#include "chrome/browser/chrome_to_mobile_service_factory.h"

#include "chrome/browser/chrome_to_mobile_service.h"
#include "chrome/browser/invalidation/invalidation_service_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/token_service_factory.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
#include "components/browser_context_keyed_service/browser_context_dependency_manager.h"

// static
Expand Down Expand Up @@ -36,7 +36,7 @@ ChromeToMobileServiceFactory::ChromeToMobileServiceFactory()
: BrowserContextKeyedServiceFactory(
"ChromeToMobileService",
BrowserContextDependencyManager::GetInstance()) {
DependsOn(ProfileSyncServiceFactory::GetInstance());
DependsOn(invalidation::InvalidationServiceFactory::GetInstance());
DependsOn(TokenServiceFactory::GetInstance());
// TODO(msw): Uncomment this once it exists.
// DependsOn(PrefServiceFactory::GetInstance());
Expand Down
36 changes: 19 additions & 17 deletions chrome/browser/drive/drive_notification_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

#include "base/metrics/histogram.h"
#include "chrome/browser/drive/drive_notification_observer.h"
#include "chrome/browser/invalidation/invalidation_service.h"
#include "chrome/browser/invalidation/invalidation_service_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sync/profile_sync_service.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
#include "google/cacheinvalidation/types.pb.h"

namespace drive {
Expand Down Expand Up @@ -42,15 +42,16 @@ DriveNotificationManager::~DriveNotificationManager() {}

void DriveNotificationManager::Shutdown() {
// Unregister for Drive notifications.
ProfileSyncService* profile_sync_service =
ProfileSyncServiceFactory::GetForProfile(profile_);
if (!profile_sync_service || !push_notification_registered_) {
invalidation::InvalidationService* invalidation_service =
invalidation::InvalidationServiceFactory::GetForProfile(profile_);
if (!invalidation_service || !push_notification_registered_) {
return;
}

profile_sync_service->UpdateRegisteredInvalidationIds(
this, syncer::ObjectIdSet());
profile_sync_service->UnregisterInvalidationHandler(this);
// We unregister the handler without updating unregistering our IDs on
// purpose. See the class comment on the InvalidationService interface for
// more information.
invalidation_service->UnregisterInvalidationHandler(this);
}

void DriveNotificationManager::OnInvalidatorStateChange(
Expand All @@ -76,9 +77,10 @@ void DriveNotificationManager::OnIncomingInvalidation(

// TODO(dcheng): Only acknowledge the invalidation once the fetch has
// completed. http://crbug.com/156843
ProfileSyncService* profile_sync_service =
ProfileSyncServiceFactory::GetForProfile(profile_);
profile_sync_service->AcknowledgeInvalidation(
invalidation::InvalidationService* invalidation_service =
invalidation::InvalidationServiceFactory::GetForProfile(profile_);
DCHECK(invalidation_service);
invalidation_service->AcknowledgeInvalidation(
invalidation_map.begin()->first,
invalidation_map.begin()->second.ack_handle);

Expand Down Expand Up @@ -128,19 +130,19 @@ void DriveNotificationManager::NotifyObserversToUpdate(
void DriveNotificationManager::RegisterDriveNotifications() {
DCHECK(!push_notification_enabled_);

ProfileSyncService* profile_sync_service =
ProfileSyncServiceFactory::GetForProfile(profile_);
if (!profile_sync_service)
invalidation::InvalidationService* invalidation_service =
invalidation::InvalidationServiceFactory::GetForProfile(profile_);
if (!invalidation_service)
return;

profile_sync_service->RegisterInvalidationHandler(this);
invalidation_service->RegisterInvalidationHandler(this);
syncer::ObjectIdSet ids;
ids.insert(invalidation::ObjectId(
ipc::invalidation::ObjectSource::COSMO_CHANGELOG,
kDriveInvalidationObjectId));
profile_sync_service->UpdateRegisteredInvalidationIds(this, ids);
invalidation_service->UpdateRegisteredInvalidationIds(this, ids);
push_notification_registered_ = true;
OnInvalidatorStateChange(profile_sync_service->GetInvalidatorState());
OnInvalidatorStateChange(invalidation_service->GetInvalidatorState());

UMA_HISTOGRAM_BOOLEAN("Drive.PushNotificationRegistered",
push_notification_registered_);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/drive/drive_notification_manager_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "chrome/browser/drive/drive_notification_manager_factory.h"

#include "chrome/browser/drive/drive_notification_manager.h"
#include "chrome/browser/invalidation/invalidation_service_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sync/profile_sync_service.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
Expand Down Expand Up @@ -33,6 +34,7 @@ DriveNotificationManagerFactory::DriveNotificationManagerFactory()
"DriveNotificationManager",
BrowserContextDependencyManager::GetInstance()) {
DependsOn(ProfileSyncServiceFactory::GetInstance());
DependsOn(invalidation::InvalidationServiceFactory::GetInstance());
}

DriveNotificationManagerFactory::~DriveNotificationManagerFactory() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@
#include "chrome/browser/extensions/extension_system_factory.h"
#include "chrome/browser/extensions/token_cache/token_cache_service.h"
#include "chrome/browser/extensions/token_cache/token_cache_service_factory.h"
#include "chrome/browser/invalidation/invalidation_service.h"
#include "chrome/browser/invalidation/invalidation_service_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/token_service.h"
#include "chrome/browser/signin/token_service_factory.h"
#include "chrome/browser/sync/profile_sync_service.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
#include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
#include "chrome/common/chrome_notification_types.h"
#include "chrome/common/extensions/api/push_messaging.h"
Expand Down Expand Up @@ -302,17 +302,18 @@ PushMessagingAPI::GetFactoryInstance() {
void PushMessagingAPI::Observe(int type,
const content::NotificationSource& source,
const content::NotificationDetails& details) {
ProfileSyncService* pss = ProfileSyncServiceFactory::GetForProfile(profile_);
invalidation::InvalidationService* invalidation_service =
invalidation::InvalidationServiceFactory::GetForProfile(profile_);
// This may be NULL; for example, for the ChromeOS guest user. In these cases,
// just return without setting up anything, since it won't work anyway.
if (!pss)
if (!invalidation_service)
return;

if (!event_router_)
event_router_.reset(new PushMessagingEventRouter(profile_));
if (!handler_) {
handler_.reset(new PushMessagingInvalidationHandler(
pss, event_router_.get()));
invalidation_service, event_router_.get()));
}
switch (type) {
case chrome::NOTIFICATION_EXTENSION_INSTALLED: {
Expand Down Expand Up @@ -351,7 +352,7 @@ void PushMessagingAPI::SetMapperForTest(
template <>
void ProfileKeyedAPIFactory<PushMessagingAPI>::DeclareFactoryDependencies() {
DependsOn(ExtensionSystemFactory::GetInstance());
DependsOn(ProfileSyncServiceFactory::GetInstance());
DependsOn(invalidation::InvalidationServiceFactory::GetInstance());
}

} // namespace extensions
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,23 @@
#include "chrome/browser/extensions/extension_apitest.h"
#include "chrome/browser/extensions/extension_test_message_listener.h"
#include "chrome/browser/extensions/platform_app_launcher.h"
#include "chrome/browser/invalidation/fake_invalidation_service.h"
#include "chrome/browser/invalidation/invalidation_service.h"
#include "chrome/browser/invalidation/invalidation_service_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sync/profile_sync_service.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/test/base/ui_test_utils.h"
#include "google/cacheinvalidation/types.pb.h"
#include "sync/notifier/fake_invalidator.h"
#include "testing/gmock/include/gmock/gmock.h"

using ::testing::_;
using ::testing::SaveArg;
using ::testing::StrictMock;

using invalidation::InvalidationServiceFactory;

namespace extensions {

namespace {
Expand Down Expand Up @@ -52,17 +56,43 @@ MockInvalidationMapper::~MockInvalidationMapper() {}

class PushMessagingApiTest : public ExtensionApiTest {
public:
PushMessagingApiTest()
: fake_invalidation_service_(NULL) {
}

virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
ExtensionApiTest::SetUpCommandLine(command_line);
}

virtual void SetUp() OVERRIDE {
InvalidationServiceFactory::GetInstance()->
SetBuildOnlyFakeInvalidatorsForTest(true);
ExtensionApiTest::SetUp();
}

virtual void SetUpOnMainThread() OVERRIDE {
ExtensionApiTest::SetUpOnMainThread();
fake_invalidation_service_ =
static_cast<invalidation::FakeInvalidationService*>(
InvalidationServiceFactory::GetInstance()->GetForProfile(
profile()));
}

void EmitInvalidation(
const invalidation::ObjectId& object_id,
const std::string& payload) {
fake_invalidation_service_->EmitInvalidationForTest(object_id, payload);
}

PushMessagingAPI* GetAPI() {
return PushMessagingAPI::Get(profile());
}

PushMessagingEventRouter* GetEventRouter() {
return PushMessagingAPI::Get(profile())->GetEventRouterForTest();
}

invalidation::FakeInvalidationService* fake_invalidation_service_;
};

IN_PROC_BROWSER_TEST_F(PushMessagingApiTest, EventDispatch) {
Expand Down Expand Up @@ -92,18 +122,14 @@ IN_PROC_BROWSER_TEST_F(PushMessagingApiTest, ReceivesPush) {
ui_test_utils::NavigateToURL(
browser(), extension->GetResourceURL("event_dispatch.html"));

ProfileSyncService* pss =
ProfileSyncServiceFactory::GetForProfile(profile());
ASSERT_TRUE(pss);

// PushMessagingInvalidationHandler suppresses the initial invalidation on
// each subchannel at install, so trigger the suppressions first.
for (int i = 0; i < 3; ++i) {
pss->EmitInvalidationForTest(
EmitInvalidation(
ExtensionAndSubchannelToObjectId(extension->id(), i), std::string());
}

pss->EmitInvalidationForTest(
EmitInvalidation(
ExtensionAndSubchannelToObjectId(extension->id(), 1), "payload");
EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h"
#include "chrome/browser/extensions/api/push_messaging/push_messaging_invalidation_handler_delegate.h"
#include "chrome/browser/invalidation/invalidation_frontend.h"
#include "chrome/browser/invalidation/invalidation_service.h"
#include "chrome/common/extensions/extension.h"
#include "google/cacheinvalidation/types.pb.h"

Expand Down Expand Up @@ -78,7 +78,7 @@ bool ObjectIdToExtensionAndSubchannel(const invalidation::ObjectId& object_id,
} // namespace

PushMessagingInvalidationHandler::PushMessagingInvalidationHandler(
invalidation::InvalidationFrontend* service,
invalidation::InvalidationService* service,
PushMessagingInvalidationHandlerDelegate* delegate)
: service_(service),
delegate_(delegate) {
Expand Down
Loading

0 comments on commit 389f566

Please sign in to comment.