Skip to content

Commit

Permalink
sync: Expose ProtocolEvents on ProfileSyncService
Browse files Browse the repository at this point in the history
Adds code to the sync engine to have it generate protocol events when it
contacts the server.  These events are then sent through the
SyncSession, SyncManager, SyncBackendHostCore, SyncBackendHost, and
finally to the ProfileSyncService.

Objects on the UI thread can register with the ProfileSyncService as
observers of these events, though this CL does not introduce any of
these listeners.

BUG=349301

Review URL: https://codereview.chromium.org/203463005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@258685 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rlarocque@chromium.org committed Mar 21, 2014
1 parent 560b54e commit eb370fc
Show file tree
Hide file tree
Showing 48 changed files with 268 additions and 25 deletions.
3 changes: 3 additions & 0 deletions chrome/browser/sync/glue/sync_backend_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
// Fetches the DeviceInfo tracker.
virtual SyncedDeviceTracker* GetSyncedDeviceTracker() const = 0;

// Sets whether or not the frontend will be notified of network events.
virtual void SetForwardProtocolEvents(bool forward) = 0;

virtual base::MessageLoop* GetSyncLoopForTesting() = 0;

DISALLOW_COPY_AND_ASSIGN(SyncBackendHost);
Expand Down
18 changes: 18 additions & 0 deletions chrome/browser/sync/glue/sync_backend_host_core.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "chrome/browser/sync/glue/synced_device_tracker.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/chrome_version_info.h"
#include "sync/internal_api/public/events/protocol_event.h"
#include "sync/internal_api/public/http_post_provider_factory.h"
#include "sync/internal_api/public/internal_components_factory.h"
#include "sync/internal_api/public/sessions/sync_session_snapshot.h"
Expand Down Expand Up @@ -316,6 +317,18 @@ void SyncBackendHostCore::OnMigrationRequested(syncer::ModelTypeSet types) {
types);
}

void SyncBackendHostCore::OnProtocolEvent(
const syncer::ProtocolEvent& event) {
// TODO(rlarocque): Find a way to pass event_clone as a scoped_ptr.
if (forward_protocol_events_) {
scoped_ptr<syncer::ProtocolEvent> event_clone(event.Clone());
host_.Call(
FROM_HERE,
&SyncBackendHostImpl::HandleProtocolEventOnFrontendLoop,
event_clone.release());
}
}

void SyncBackendHostCore::DoOnInvalidatorStateChange(
syncer::InvalidatorState state) {
DCHECK_EQ(base::MessageLoop::current(), sync_loop_);
Expand Down Expand Up @@ -592,6 +605,11 @@ void SyncBackendHostCore::DoRetryConfiguration(
retry_callback);
}

void SyncBackendHostCore::SetForwardProtocolEvents(bool enabled) {
DCHECK_EQ(base::MessageLoop::current(), sync_loop_);
forward_protocol_events_ = enabled;
}

void SyncBackendHostCore::DeleteSyncDataFolder() {
DCHECK_EQ(base::MessageLoop::current(), sync_loop_);
if (base::DirectoryExists(sync_data_folder_path_)) {
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/sync/glue/sync_backend_host_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ class SyncBackendHostCore
virtual void OnActionableError(
const syncer::SyncProtocolError& sync_error) OVERRIDE;
virtual void OnMigrationRequested(syncer::ModelTypeSet types) OVERRIDE;
virtual void OnProtocolEvent(const syncer::ProtocolEvent& event) OVERRIDE;

// SyncEncryptionHandler::Observer implementation.
virtual void OnPassphraseRequired(
Expand Down Expand Up @@ -208,6 +209,8 @@ class SyncBackendHostCore
return synced_device_tracker_.get();
}

void SetForwardProtocolEvents(bool forward);

// Delete the sync data folder to cleanup backend data. Happens the first
// time sync is enabled for a user (to prevent accidentally reusing old
// sync databases), as well as shutdown when you're no longer syncing.
Expand Down Expand Up @@ -285,6 +288,9 @@ class SyncBackendHostCore
// Should not be used for anything except for UMAs and logging.
const bool has_sync_setup_completed_;

// Set when we've been asked to forward sync protocol events to the frontend.
bool forward_protocol_events_;

base::WeakPtrFactory<SyncBackendHostCore> weak_ptr_factory_;

DISALLOW_COPY_AND_ASSIGN(SyncBackendHostCore);
Expand Down
17 changes: 17 additions & 0 deletions chrome/browser/sync/glue/sync_backend_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_source.h"
#include "sync/internal_api/public/base_transaction.h"
#include "sync/internal_api/public/events/protocol_event.h"
#include "sync/internal_api/public/http_bridge.h"
#include "sync/internal_api/public/internal_components_factory.h"
#include "sync/internal_api/public/internal_components_factory_impl.h"
Expand Down Expand Up @@ -474,6 +475,14 @@ SyncedDeviceTracker* SyncBackendHostImpl::GetSyncedDeviceTracker() const {
return core_->synced_device_tracker();
}

void SyncBackendHostImpl::SetForwardProtocolEvents(bool forward) {
DCHECK(initialized());
registrar_->sync_thread()->message_loop()->PostTask(
FROM_HERE,
base::Bind(&SyncBackendHostCore::SetForwardProtocolEvents,
core_, forward));
}

void SyncBackendHostImpl::InitCore(scoped_ptr<DoInitializeOptions> options) {
registrar_->sync_thread()->message_loop()->PostTask(FROM_HERE,
base::Bind(&SyncBackendHostCore::DoInitialize,
Expand Down Expand Up @@ -744,6 +753,14 @@ void SyncBackendHostImpl::HandleConnectionStatusChangeOnFrontendLoop(
frontend_->OnConnectionStatusChange(status);
}

void SyncBackendHostImpl::HandleProtocolEventOnFrontendLoop(
syncer::ProtocolEvent* event) {
scoped_ptr<syncer::ProtocolEvent> scoped_event(event);
if (!frontend_)
return;
frontend_->OnProtocolEvent(*scoped_event);
}

base::MessageLoop* SyncBackendHostImpl::GetSyncLoopForTesting() {
return registrar_->sync_thread()->message_loop();
}
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/sync/glue/sync_backend_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ class SyncBackendHostImpl
virtual void GetModelSafeRoutingInfo(
syncer::ModelSafeRoutingInfo* out) const OVERRIDE;
virtual SyncedDeviceTracker* GetSyncedDeviceTracker() const OVERRIDE;
virtual void SetForwardProtocolEvents(bool forward) OVERRIDE;
virtual base::MessageLoop* GetSyncLoopForTesting() OVERRIDE;

protected:
Expand Down Expand Up @@ -166,6 +167,11 @@ class SyncBackendHostImpl
// frontend's sync configure retry method.
void HandleControlTypesDownloadRetry();

// Forwards a ProtocolEvent to the frontend. Will not be called unless a
// call to SetForwardProtocolEvents() explicitly requested that we start
// forwarding these events.
void HandleProtocolEventOnFrontendLoop(syncer::ProtocolEvent* event);

SyncFrontend* frontend() { return frontend_; }

private:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class MockSyncFrontend : public SyncFrontend {
void(syncer::ModelTypeSet, bool));
MOCK_METHOD0(OnEncryptionComplete, void());
MOCK_METHOD1(OnMigrationNeededForTypes, void(syncer::ModelTypeSet));
MOCK_METHOD1(OnProtocolEvent, void(const syncer::ProtocolEvent&));
MOCK_METHOD1(OnExperimentsChanged,
void(const syncer::Experiments&));
MOCK_METHOD1(OnActionableError,
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/sync/glue/sync_backend_host_mock.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ base::MessageLoop* SyncBackendHostMock::GetSyncLoopForTesting() {
return NULL;
}

void SyncBackendHostMock::SetForwardProtocolEvents(bool enabled) {}

void SyncBackendHostMock::set_fail_initial_download(bool should_fail) {
fail_initial_download_ = should_fail;
}
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/sync/glue/sync_backend_host_mock.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ class SyncBackendHostMock : public SyncBackendHost {

virtual SyncedDeviceTracker* GetSyncedDeviceTracker() const OVERRIDE;

virtual void SetForwardProtocolEvents(bool enabled) OVERRIDE;

virtual base::MessageLoop* GetSyncLoopForTesting() OVERRIDE;

void set_fail_initial_download(bool should_fail);
Expand Down
29 changes: 29 additions & 0 deletions chrome/browser/sync/profile_sync_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,13 @@ void ProfileSyncService::OnSyncConfigureRetry() {
NotifyObservers();
}

void ProfileSyncService::OnProtocolEvent(
const syncer::ProtocolEvent& event) {
FOR_EACH_OBSERVER(browser_sync::ProtocolEventObserver,
protocol_event_observers_,
OnProtocolEvent(event));
}

void ProfileSyncService::OnDataTypeRequestsSyncStartup(
syncer::ModelType type) {
DCHECK(syncer::UserTypes().Has(type));
Expand Down Expand Up @@ -936,6 +943,10 @@ void ProfileSyncService::OnBackendInitialized(
sync_js_controller_.AttachJsBackend(js_backend);
debug_info_listener_ = debug_info_listener;

if (protocol_event_observers_.might_have_observers()) {
backend_->SetForwardProtocolEvents(true);
}

// If we have a cached passphrase use it to decrypt/encrypt data now that the
// backend is initialized. We want to call this before notifying observers in
// case this operation affects the "passphrase required" status.
Expand Down Expand Up @@ -2070,6 +2081,24 @@ void ProfileSyncService::RemoveObserver(
observers_.RemoveObserver(observer);
}

void ProfileSyncService::AddProtocolEventObserver(
browser_sync::ProtocolEventObserver* observer) {
protocol_event_observers_.AddObserver(observer);
if (backend_) {
backend_->SetForwardProtocolEvents(
protocol_event_observers_.might_have_observers());
}
}

void ProfileSyncService::RemoveProtocolEventObserver(
browser_sync::ProtocolEventObserver* observer) {
protocol_event_observers_.RemoveObserver(observer);
if (backend_) {
backend_->SetForwardProtocolEvents(
protocol_event_observers_.might_have_observers());
}
}

bool ProfileSyncService::HasObserver(
ProfileSyncServiceBase::Observer* observer) const {
return observers_.HasObserver(observer);
Expand Down
8 changes: 8 additions & 0 deletions chrome/browser/sync/profile_sync_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "chrome/browser/sync/glue/synced_device_tracker.h"
#include "chrome/browser/sync/profile_sync_service_base.h"
#include "chrome/browser/sync/profile_sync_service_observer.h"
#include "chrome/browser/sync/protocol_event_observer.h"
#include "chrome/browser/sync/sessions2/sessions_sync_manager.h"
#include "chrome/browser/sync/startup_controller.h"
#include "components/keyed_service/core/keyed_service.h"
Expand Down Expand Up @@ -273,6 +274,11 @@ class ProfileSyncService : public ProfileSyncServiceBase,
virtual bool HasObserver(
ProfileSyncServiceBase::Observer* observer) const OVERRIDE;


void AddProtocolEventObserver(browser_sync::ProtocolEventObserver* observer);
void RemoveProtocolEventObserver(
browser_sync::ProtocolEventObserver* observer);

void RegisterAuthNotifications();
void UnregisterAuthNotifications();

Expand Down Expand Up @@ -365,6 +371,7 @@ class ProfileSyncService : public ProfileSyncServiceBase,
debug_info_listener,
bool success) OVERRIDE;
virtual void OnSyncCycleCompleted() OVERRIDE;
virtual void OnProtocolEvent(const syncer::ProtocolEvent& event) OVERRIDE;
virtual void OnSyncConfigureRetry() OVERRIDE;
virtual void OnConnectionStatusChange(
syncer::ConnectionStatus status) OVERRIDE;
Expand Down Expand Up @@ -911,6 +918,7 @@ class ProfileSyncService : public ProfileSyncServiceBase,
scoped_ptr<browser_sync::DataTypeManager> data_type_manager_;

ObserverList<ProfileSyncServiceBase::Observer> observers_;
ObserverList<browser_sync::ProtocolEventObserver> protocol_event_observers_;

syncer::SyncJsController sync_js_controller_;

Expand Down
13 changes: 13 additions & 0 deletions chrome/browser/sync/protocol_event_observer.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/sync/protocol_event_observer.h"

namespace browser_sync {

ProtocolEventObserver::ProtocolEventObserver() {}

ProtocolEventObserver::~ProtocolEventObserver() {}

} // namespace browser_sync
24 changes: 24 additions & 0 deletions chrome/browser/sync/protocol_event_observer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_SYNC_PROTOCOL_EVENT_OBSERVER_H_
#define CHROME_BROWSER_SYNC_PROTOCOL_EVENT_OBSERVER_H_

namespace syncer {
class ProtocolEvent;
}

namespace browser_sync {

class ProtocolEventObserver {
public:
ProtocolEventObserver();
virtual ~ProtocolEventObserver();

virtual void OnProtocolEvent(const syncer::ProtocolEvent& event) = 0;
};

} // namespace browser_sync

#endif // CHROME_BROWSER_SYNC_PROTOCOL_EVENT_OBSERVER_H_
2 changes: 2 additions & 0 deletions chrome/chrome_browser.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -2360,6 +2360,8 @@
'browser/sync/profile_sync_service_model_type_selection_android.h',
'browser/sync/profile_sync_service_observer.cc',
'browser/sync/profile_sync_service_observer.h',
'browser/sync/protocol_event_observer.cc',
'browser/sync/protocol_event_observer.h',
'browser/sync/sessions2/notification_service_sessions_router.cc',
'browser/sync/sessions2/notification_service_sessions_router.h',
'browser/sync/sessions2/session_data_type_controller2.cc',
Expand Down
9 changes: 9 additions & 0 deletions components/sync_driver/sync_frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
namespace syncer {
class DataTypeDebugInfoListener;
class JsBackend;
class ProtocolEvent;
} // namespace syncer

namespace sync_pb {
Expand Down Expand Up @@ -53,6 +54,14 @@ class SyncFrontend {
// retried.
virtual void OnSyncConfigureRetry() = 0;

// Informs the frontned of some network event. These notifications are
// disabled by default and must be enabled through an explicit request to the
// SyncBackendHost.
//
// It's disabld by default to avoid copying data across threads when no one
// is listening for it.
virtual void OnProtocolEvent(const syncer::ProtocolEvent& event) = 0;

// The status of the connection to the sync server has changed.
virtual void OnConnectionStatusChange(
syncer::ConnectionStatus status) = 0;
Expand Down
1 change: 1 addition & 0 deletions sync/engine/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ include_rules = [
"+sync/base",
"+sync/internal_api/public/base",
"+sync/internal_api/public/engine",
"+sync/internal_api/public/events",
"+sync/internal_api/public/sessions",
"+sync/internal_api/public/test",
"+sync/internal_api/public/util",
Expand Down
2 changes: 2 additions & 0 deletions sync/engine/all_status.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ void AllStatus::OnThrottledTypesChanged(ModelTypeSet throttled_types) {

void AllStatus::OnMigrationRequested(ModelTypeSet) {}

void AllStatus::OnProtocolEvent(const ProtocolEvent&) {}

SyncStatus AllStatus::status() const {
base::AutoLock lock(mutex_);
return status_;
Expand Down
1 change: 1 addition & 0 deletions sync/engine/all_status.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class AllStatus : public SyncEngineEventListener {
virtual void OnRetryTimeChanged(base::Time retry_time) OVERRIDE;
virtual void OnThrottledTypesChanged(ModelTypeSet throttled_types) OVERRIDE;
virtual void OnMigrationRequested(ModelTypeSet types) OVERRIDE;
virtual void OnProtocolEvent(const ProtocolEvent& event) OVERRIDE;

SyncStatus status() const;

Expand Down
17 changes: 17 additions & 0 deletions sync/engine/commit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include "sync/engine/commit_util.h"
#include "sync/engine/syncer.h"
#include "sync/engine/syncer_proto_util.h"
#include "sync/internal_api/public/events/commit_request_event.h"
#include "sync/internal_api/public/events/commit_response_event.h"
#include "sync/sessions/sync_session.h"

namespace syncer {
Expand Down Expand Up @@ -97,11 +99,26 @@ SyncerError Commit::PostAndProcessResponse(
}

DVLOG(1) << "Sending commit message.";

CommitRequestEvent request_event(
base::Time::Now(),
message_.commit().entries_size(),
request_types,
message_);
session->SendProtocolEvent(request_event);

TRACE_EVENT_BEGIN0("sync", "PostCommit");
const SyncerError post_result = SyncerProtoUtil::PostClientToServerMessage(
&message_, &response_, session);
TRACE_EVENT_END0("sync", "PostCommit");

// TODO(rlarocque): Use result that includes errors captured later?
CommitResponseEvent response_event(
base::Time::Now(),
post_result,
response_);
session->SendProtocolEvent(response_event);

if (post_result != SYNCER_OK) {
LOG(WARNING) << "Post commit failed";
return post_result;
Expand Down
Loading

0 comments on commit eb370fc

Please sign in to comment.