Commit 7135c17f authored by Maksim Moskvitin's avatar Maksim Moskvitin Committed by Commit Bot

[Sync:USS] Support logging for USS Nigori

We used to ask Directory itself to provide Nigori node for debugging.
With USS implementation enabled we need to change the source to
corresponding ModelTypeController. Since Nigori's ModelTypeController
not available inside ProfileSyncService, we plumb it through SyncEngine
and SyncEngineBackend.

There is a side change: when Directory implementation of Nigori is
enabled, we start call GetAllNodesForTypeFromDirectory() from sync
sequence, instead of UI sequence. It should be safe, since logging
already supports plumbing to different sequences and Directory's
functions could be called from any sequence (protected by
transactions).

Bug: 922900
Change-Id: Ifdc5ffb1a5a99e51a0ec88643f9325d1c4a287de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1800772
Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com>
Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701127}
parent e5d49124
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "components/sync/base/invalidation_adapter.h" #include "components/sync/base/invalidation_adapter.h"
#include "components/sync/base/sync_base_switches.h" #include "components/sync/base/sync_base_switches.h"
#include "components/sync/driver/configure_context.h" #include "components/sync/driver/configure_context.h"
#include "components/sync/driver/directory_data_type_controller.h"
#include "components/sync/driver/model_type_controller.h" #include "components/sync/driver/model_type_controller.h"
#include "components/sync/driver/sync_driver_switches.h" #include "components/sync/driver/sync_driver_switches.h"
#include "components/sync/engine/cycle/commit_counters.h" #include "components/sync/engine/cycle/commit_counters.h"
...@@ -681,6 +682,20 @@ void SyncEngineBackend::DoOnInvalidatorClientIdChange( ...@@ -681,6 +682,20 @@ void SyncEngineBackend::DoOnInvalidatorClientIdChange(
sync_manager_->UpdateInvalidationClientId(client_id); sync_manager_->UpdateInvalidationClientId(client_id);
} }
void SyncEngineBackend::GetNigoriNodeForDebugging(AllNodesCallback callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (nigori_controller_) {
// USS implementation of Nigori.
nigori_controller_->GetAllNodes(std::move(callback));
} else {
// Directory implementation of Nigori.
DCHECK(user_share_.directory);
std::move(callback).Run(
NIGORI, DirectoryDataTypeController::GetAllNodesForTypeFromDirectory(
NIGORI, user_share_.directory.get()));
}
}
bool SyncEngineBackend::HasUnsyncedItemsForTest() const { bool SyncEngineBackend::HasUnsyncedItemsForTest() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(sync_manager_); DCHECK(sync_manager_);
......
...@@ -46,6 +46,10 @@ class SyncEngineBackend : public base::RefCountedThreadSafe<SyncEngineBackend>, ...@@ -46,6 +46,10 @@ class SyncEngineBackend : public base::RefCountedThreadSafe<SyncEngineBackend>,
public SyncManager::Observer, public SyncManager::Observer,
public TypeDebugInfoObserver { public TypeDebugInfoObserver {
public: public:
using AllNodesCallback =
base::OnceCallback<void(const ModelType,
std::unique_ptr<base::ListValue>)>;
SyncEngineBackend(const std::string& name, SyncEngineBackend(const std::string& name,
const base::FilePath& sync_data_folder, const base::FilePath& sync_data_folder,
const base::WeakPtr<SyncEngineImpl>& host); const base::WeakPtr<SyncEngineImpl>& host);
...@@ -174,6 +178,9 @@ class SyncEngineBackend : public base::RefCountedThreadSafe<SyncEngineBackend>, ...@@ -174,6 +178,9 @@ class SyncEngineBackend : public base::RefCountedThreadSafe<SyncEngineBackend>,
// Notify about change in client id. // Notify about change in client id.
void DoOnInvalidatorClientIdChange(const std::string& client_id); void DoOnInvalidatorClientIdChange(const std::string& client_id);
// Returns a ListValue representing Nigori node.
void GetNigoriNodeForDebugging(AllNodesCallback callback);
bool HasUnsyncedItemsForTest() const; bool HasUnsyncedItemsForTest() const;
private: private:
......
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
#include "components/invalidation/impl/invalidation_switches.h" #include "components/invalidation/impl/invalidation_switches.h"
#include "components/invalidation/public/invalidation_service.h" #include "components/invalidation/public/invalidation_service.h"
#include "components/invalidation/public/object_id_invalidation_map.h" #include "components/invalidation/public/object_id_invalidation_map.h"
#include "components/sync/base/bind_to_task_runner.h"
#include "components/sync/base/invalidation_helper.h" #include "components/sync/base/invalidation_helper.h"
#include "components/sync/base/sync_prefs.h" #include "components/sync/base/sync_prefs.h"
#include "components/sync/driver/glue/sync_engine_backend.h" #include "components/sync/driver/glue/sync_engine_backend.h"
...@@ -445,6 +446,14 @@ void SyncEngineImpl::SetInvalidationsForSessionsEnabled(bool enabled) { ...@@ -445,6 +446,14 @@ void SyncEngineImpl::SetInvalidationsForSessionsEnabled(bool enabled) {
DCHECK(success); DCHECK(success);
} }
void SyncEngineImpl::GetNigoriNodeForDebugging(AllNodesCallback callback) {
DCHECK(backend_);
sync_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&SyncEngineBackend::GetNigoriNodeForDebugging, backend_,
BindToCurrentSequence(std::move(callback))));
}
void SyncEngineImpl::OnInvalidatorClientIdChange(const std::string& client_id) { void SyncEngineImpl::OnInvalidatorClientIdChange(const std::string& client_id) {
sync_task_runner_->PostTask( sync_task_runner_->PostTask(
FROM_HERE, FROM_HERE,
......
...@@ -92,6 +92,7 @@ class SyncEngineImpl : public SyncEngine, public InvalidationHandler { ...@@ -92,6 +92,7 @@ class SyncEngineImpl : public SyncEngine, public InvalidationHandler {
bool empty_jar, bool empty_jar,
const base::Closure& callback) override; const base::Closure& callback) override;
void SetInvalidationsForSessionsEnabled(bool enabled) override; void SetInvalidationsForSessionsEnabled(bool enabled) override;
void GetNigoriNodeForDebugging(AllNodesCallback callback) override;
// InvalidationHandler implementation. // InvalidationHandler implementation.
void OnInvalidatorStateChange(InvalidatorState state) override; void OnInvalidatorStateChange(InvalidatorState state) override;
......
...@@ -1652,10 +1652,10 @@ void ProfileSyncService::GetAllNodesForDebugging( ...@@ -1652,10 +1652,10 @@ void ProfileSyncService::GetAllNodesForDebugging(
&GetAllNodesRequestHelper::OnReceivedNodesForType, helper)); &GetAllNodesRequestHelper::OnReceivedNodesForType, helper));
} }
} else { } else {
// Control Types. // We should have no data type controller only for Nigori.
helper->OnReceivedNodesForType( DCHECK_EQ(type, NIGORI);
type, DirectoryDataTypeController::GetAllNodesForTypeFromDirectory( engine_->GetNigoriNodeForDebugging(base::BindOnce(
type, GetUserShare()->directory.get())); &GetAllNodesRequestHelper::OnReceivedNodesForType, helper));
} }
} }
} }
......
...@@ -108,4 +108,6 @@ void FakeSyncEngine::OnCookieJarChanged(bool account_mismatch, ...@@ -108,4 +108,6 @@ void FakeSyncEngine::OnCookieJarChanged(bool account_mismatch,
void FakeSyncEngine::SetInvalidationsForSessionsEnabled(bool enabled) {} void FakeSyncEngine::SetInvalidationsForSessionsEnabled(bool enabled) {}
void FakeSyncEngine::GetNigoriNodeForDebugging(AllNodesCallback callback) {}
} // namespace syncer } // namespace syncer
...@@ -88,7 +88,7 @@ class FakeSyncEngine : public SyncEngine { ...@@ -88,7 +88,7 @@ class FakeSyncEngine : public SyncEngine {
bool empty_jar, bool empty_jar,
const base::Closure& callback) override; const base::Closure& callback) override;
void SetInvalidationsForSessionsEnabled(bool enabled) override; void SetInvalidationsForSessionsEnabled(bool enabled) override;
void GetNigoriNodeForDebugging(AllNodesCallback callback) override;
void set_fail_initial_download(bool should_fail); void set_fail_initial_download(bool should_fail);
private: private:
......
...@@ -60,6 +60,7 @@ class MockSyncEngine : public SyncEngine { ...@@ -60,6 +60,7 @@ class MockSyncEngine : public SyncEngine {
MOCK_METHOD1(ClearServerData, void(const base::Closure&)); MOCK_METHOD1(ClearServerData, void(const base::Closure&));
MOCK_METHOD3(OnCookieJarChanged, void(bool, bool, const base::Closure&)); MOCK_METHOD3(OnCookieJarChanged, void(bool, bool, const base::Closure&));
MOCK_METHOD1(SetInvalidationsForSessionsEnabled, void(bool)); MOCK_METHOD1(SetInvalidationsForSessionsEnabled, void(bool));
MOCK_METHOD1(GetNigoriNodeForDebugging, void(AllNodesCallback));
}; };
} // namespace syncer } // namespace syncer
......
...@@ -43,6 +43,8 @@ class UnrecoverableErrorHandler; ...@@ -43,6 +43,8 @@ class UnrecoverableErrorHandler;
// interface will handle crossing threads if necessary. // interface will handle crossing threads if necessary.
class SyncEngine : public ModelTypeConfigurer { class SyncEngine : public ModelTypeConfigurer {
public: public:
using AllNodesCallback =
base::OnceCallback<void(ModelType, std::unique_ptr<base::ListValue>)>;
using HttpPostProviderFactoryGetter = using HttpPostProviderFactoryGetter =
base::OnceCallback<std::unique_ptr<HttpPostProviderFactory>( base::OnceCallback<std::unique_ptr<HttpPostProviderFactory>(
CancelationSignal*)>; CancelationSignal*)>;
...@@ -190,6 +192,9 @@ class SyncEngine : public ModelTypeConfigurer { ...@@ -190,6 +192,9 @@ class SyncEngine : public ModelTypeConfigurer {
// Enables/Disables invalidations for session sync related datatypes. // Enables/Disables invalidations for session sync related datatypes.
virtual void SetInvalidationsForSessionsEnabled(bool enabled) = 0; virtual void SetInvalidationsForSessionsEnabled(bool enabled) = 0;
// Returns a ListValue representing Nigori node.
virtual void GetNigoriNodeForDebugging(AllNodesCallback callback) = 0;
private: private:
DISALLOW_COPY_AND_ASSIGN(SyncEngine); DISALLOW_COPY_AND_ASSIGN(SyncEngine);
}; };
......
...@@ -229,9 +229,13 @@ void NigoriModelTypeProcessor::GetAllNodesForDebugging( ...@@ -229,9 +229,13 @@ void NigoriModelTypeProcessor::GetAllNodesForDebugging(
AllNodesCallback callback) { AllNodesCallback callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
std::unique_ptr<base::DictionaryValue> root_node;
std::unique_ptr<EntityData> entity_data = bridge_->GetData(); std::unique_ptr<EntityData> entity_data = bridge_->GetData();
if (entity_data) { if (!entity_data) {
std::move(callback).Run(syncer::NIGORI,
std::make_unique<base::ListValue>());
return;
}
if (entity_) { if (entity_) {
const sync_pb::EntityMetadata& metadata = entity_->metadata(); const sync_pb::EntityMetadata& metadata = entity_->metadata();
// Set id value as directory, "s" means server. // Set id value as directory, "s" means server.
...@@ -240,24 +244,19 @@ void NigoriModelTypeProcessor::GetAllNodesForDebugging( ...@@ -240,24 +244,19 @@ void NigoriModelTypeProcessor::GetAllNodesForDebugging(
entity_data->modification_time = entity_data->modification_time =
ProtoTimeToTime(metadata.modification_time()); ProtoTimeToTime(metadata.modification_time());
} }
std::unique_ptr<base::DictionaryValue> root_node;
root_node = entity_data->ToDictionaryValue(); root_node = entity_data->ToDictionaryValue();
if (entity_) { if (entity_) {
root_node->Set("metadata", EntityMetadataToValue(entity_->metadata())); root_node->Set("metadata", EntityMetadataToValue(entity_->metadata()));
} }
} else {
root_node = std::make_unique<base::DictionaryValue>();
}
// Function isTypeRootNode in sync_node_browser.js use PARENT_ID and // Function isTypeRootNode in sync_node_browser.js use PARENT_ID and
// UNIQUE_SERVER_TAG to check if the node is root node. isChildOf in // UNIQUE_SERVER_TAG to check if the node is root node. isChildOf in
// sync_node_browser.js uses modelType to check if root node is parent of real // sync_node_browser.js uses modelType to check if root node is parent of real
// data node. NON_UNIQUE_NAME will be the name of node to display. // data node.
root_node->SetString("ID", "NIGORI_ROOT");
root_node->SetString("PARENT_ID", "r"); root_node->SetString("PARENT_ID", "r");
root_node->SetString("UNIQUE_SERVER_TAG", "Nigori"); root_node->SetString("UNIQUE_SERVER_TAG", "Nigori");
root_node->SetBoolean("IS_DIR", false); root_node->SetString("modelType", ModelTypeToString(NIGORI));
root_node->SetString("modelType", "Nigori");
root_node->SetString("NON_UNIQUE_NAME", "Nigori");
auto all_nodes = std::make_unique<base::ListValue>(); auto all_nodes = std::make_unique<base::ListValue>();
all_nodes->Append(std::move(root_node)); all_nodes->Append(std::move(root_node));
......
...@@ -1051,11 +1051,25 @@ void NigoriSyncBridgeImpl::UpdateCryptographerFromNonKeystoreNigori( ...@@ -1051,11 +1051,25 @@ void NigoriSyncBridgeImpl::UpdateCryptographerFromNonKeystoreNigori(
std::unique_ptr<EntityData> NigoriSyncBridgeImpl::GetData() { std::unique_ptr<EntityData> NigoriSyncBridgeImpl::GetData() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(cryptographer_->CanEncrypt()); if (passphrase_type_ == NigoriSpecifics::UNKNOWN) {
DCHECK_NE(passphrase_type_, NigoriSpecifics::UNKNOWN); // Bridge never received NigoriSpecifics from the server. This line should
// be reachable only from processor's GetAllNodesForDebugging().
DCHECK(!cryptographer_->CanEncrypt());
DCHECK(!pending_keys_.has_value());
return nullptr;
}
NigoriSpecifics specifics; NigoriSpecifics specifics;
if (cryptographer_->CanEncrypt()) {
EncryptKeyBag(*cryptographer_, specifics.mutable_encryption_keybag()); EncryptKeyBag(*cryptographer_, specifics.mutable_encryption_keybag());
} else {
DCHECK(pending_keys_.has_value());
// This case is reachable only from processor's GetAllNodesForDebugging(),
// since currently commit is never issued while bridge has |pending_keys_|.
// Note: with complete support of TRUSTED_VAULT mode, commit might be
// issued in this case as well.
*specifics.mutable_encryption_keybag() = *pending_keys_;
}
specifics.set_keybag_is_frozen(true); specifics.set_keybag_is_frozen(true);
specifics.set_encrypt_everything(encrypt_everything_); specifics.set_encrypt_everything(encrypt_everything_);
if (encrypt_everything_) { if (encrypt_everything_) {
...@@ -1081,6 +1095,8 @@ std::unique_ptr<EntityData> NigoriSyncBridgeImpl::GetData() { ...@@ -1081,6 +1095,8 @@ std::unique_ptr<EntityData> NigoriSyncBridgeImpl::GetData() {
} }
// TODO(crbug.com/922900): add other fields support. // TODO(crbug.com/922900): add other fields support.
NOTIMPLEMENTED(); NOTIMPLEMENTED();
DCHECK(IsValidNigoriSpecifics(specifics));
auto entity_data = std::make_unique<EntityData>(); auto entity_data = std::make_unique<EntityData>();
*entity_data->specifics.mutable_nigori() = std::move(specifics); *entity_data->specifics.mutable_nigori() = std::move(specifics);
entity_data->name = kNigoriNonUniqueName; entity_data->name = kNigoriNonUniqueName;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment