Commit 9c2fc554 authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

[Sync] Report Sync Errors in PasswordSyncBridge to Crashpad

Bug: 1044365
Change-Id: Ie3d5246c24a046c18d6d2bfa6d92d1fdd46839db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063013Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarJochen Eisinger <jochen@chromium.org>
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Auto-Submit: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743013}
parent abd7bad9
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/web_data_service_factory.h" #include "chrome/browser/web_data_service_factory.h"
#include "chrome/common/channel_info.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/password_manager/core/browser/login_database.h" #include "components/password_manager/core/browser/login_database.h"
#include "components/password_manager/core/browser/password_manager_constants.h" #include "components/password_manager/core/browser/password_manager_constants.h"
...@@ -120,6 +121,7 @@ AccountPasswordStoreFactory::BuildServiceInstanceFor( ...@@ -120,6 +121,7 @@ AccountPasswordStoreFactory::BuildServiceInstanceFor(
scoped_refptr<PasswordStore> ps = scoped_refptr<PasswordStore> ps =
new password_manager::PasswordStoreDefault(std::move(login_db)); new password_manager::PasswordStoreDefault(std::move(login_db));
if (!ps->Init(/*flare=*/base::DoNothing(), profile->GetPrefs(), if (!ps->Init(/*flare=*/base::DoNothing(), profile->GetPrefs(),
chrome::GetChannel(),
base::BindRepeating(&SyncEnabledOrDisabled, profile))) { base::BindRepeating(&SyncEnabledOrDisabled, profile))) {
// TODO(crbug.com/479725): Remove the LOG once this error is visible in the // TODO(crbug.com/479725): Remove the LOG once this error is visible in the
// UI. // UI.
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "chrome/browser/sync/glue/sync_start_util.h" #include "chrome/browser/sync/glue/sync_start_util.h"
#include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/sync/profile_sync_service_factory.h"
#include "chrome/browser/web_data_service_factory.h" #include "chrome/browser/web_data_service_factory.h"
#include "chrome/common/channel_info.h"
#include "chrome/common/chrome_paths_internal.h" #include "chrome/common/chrome_paths_internal.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/password_manager/core/browser/login_database.h" #include "components/password_manager/core/browser/login_database.h"
...@@ -153,7 +154,7 @@ PasswordStoreFactory::BuildServiceInstanceFor( ...@@ -153,7 +154,7 @@ PasswordStoreFactory::BuildServiceInstanceFor(
#endif #endif
DCHECK(ps); DCHECK(ps);
if (!ps->Init(sync_start_util::GetFlareForSyncableService(profile->GetPath()), if (!ps->Init(sync_start_util::GetFlareForSyncableService(profile->GetPath()),
profile->GetPrefs())) { profile->GetPrefs(), chrome::GetChannel())) {
// TODO(crbug.com/479725): Remove the LOG once this error is visible in the // TODO(crbug.com/479725): Remove the LOG once this error is visible in the
// UI. // UI.
LOG(WARNING) << "Could not initialize password store."; LOG(WARNING) << "Could not initialize password store.";
......
...@@ -281,6 +281,7 @@ jumbo_static_library("browser") { ...@@ -281,6 +281,7 @@ jumbo_static_library("browser") {
"//components/sync", "//components/sync",
"//components/sync_preferences", "//components/sync_preferences",
"//components/url_formatter", "//components/url_formatter",
"//components/version_info:version_info",
"//components/webdata/common", "//components/webdata/common",
"//google_apis", "//google_apis",
"//net", "//net",
......
...@@ -16,6 +16,7 @@ include_rules = [ ...@@ -16,6 +16,7 @@ include_rules = [
"+components/sync_preferences", "+components/sync_preferences",
"+components/ukm", "+components/ukm",
"+components/url_formatter", "+components/url_formatter",
"+components/version_info",
"+components/webdata/common", "+components/webdata/common",
"+crypto", "+crypto",
"+google_apis", "+google_apis",
......
...@@ -19,7 +19,8 @@ MockPasswordStore::CreateBackgroundTaskRunner() const { ...@@ -19,7 +19,8 @@ MockPasswordStore::CreateBackgroundTaskRunner() const {
} }
bool MockPasswordStore::InitOnBackgroundSequence( bool MockPasswordStore::InitOnBackgroundSequence(
const syncer::SyncableService::StartSyncFlare& flare) { const syncer::SyncableService::StartSyncFlare& flare,
version_info::Channel channel) {
return true; return true;
} }
......
...@@ -126,7 +126,8 @@ class MockPasswordStore : public PasswordStore { ...@@ -126,7 +126,8 @@ class MockPasswordStore : public PasswordStore {
scoped_refptr<base::SequencedTaskRunner> CreateBackgroundTaskRunner() scoped_refptr<base::SequencedTaskRunner> CreateBackgroundTaskRunner()
const override; const override;
bool InitOnBackgroundSequence( bool InitOnBackgroundSequence(
const syncer::SyncableService::StartSyncFlare& flare) override; const syncer::SyncableService::StartSyncFlare& flare,
version_info::Channel channel) override;
}; };
} // namespace password_manager } // namespace password_manager
......
...@@ -35,6 +35,7 @@ ...@@ -35,6 +35,7 @@
#include "components/password_manager/core/browser/statistics_table.h" #include "components/password_manager/core/browser/statistics_table.h"
#include "components/password_manager/core/browser/sync/password_sync_bridge.h" #include "components/password_manager/core/browser/sync/password_sync_bridge.h"
#include "components/password_manager/core/browser/sync/password_syncable_service.h" #include "components/password_manager/core/browser/sync/password_syncable_service.h"
#include "components/sync/base/report_unrecoverable_error.h"
#include "components/sync/driver/sync_driver_switches.h" #include "components/sync/driver/sync_driver_switches.h"
#include "components/sync/model_impl/client_tag_based_model_type_processor.h" #include "components/sync/model_impl/client_tag_based_model_type_processor.h"
#include "components/sync/model_impl/proxy_model_type_controller_delegate.h" #include "components/sync/model_impl/proxy_model_type_controller_delegate.h"
...@@ -142,6 +143,7 @@ PasswordStore::PasswordStore() ...@@ -142,6 +143,7 @@ PasswordStore::PasswordStore()
bool PasswordStore::Init(const syncer::SyncableService::StartSyncFlare& flare, bool PasswordStore::Init(const syncer::SyncableService::StartSyncFlare& flare,
PrefService* prefs, PrefService* prefs,
version_info::Channel channel,
base::RepeatingClosure sync_enabled_or_disabled_cb) { base::RepeatingClosure sync_enabled_or_disabled_cb) {
main_task_runner_ = base::SequencedTaskRunnerHandle::Get(); main_task_runner_ = base::SequencedTaskRunnerHandle::Get();
DCHECK(main_task_runner_); DCHECK(main_task_runner_);
...@@ -157,7 +159,8 @@ bool PasswordStore::Init(const syncer::SyncableService::StartSyncFlare& flare, ...@@ -157,7 +159,8 @@ bool PasswordStore::Init(const syncer::SyncableService::StartSyncFlare& flare,
"passwords", "PasswordStore::InitOnBackgroundSequence", this); "passwords", "PasswordStore::InitOnBackgroundSequence", this);
base::PostTaskAndReplyWithResult( base::PostTaskAndReplyWithResult(
background_task_runner_.get(), FROM_HERE, background_task_runner_.get(), FROM_HERE,
base::BindOnce(&PasswordStore::InitOnBackgroundSequence, this, flare), base::BindOnce(&PasswordStore::InitOnBackgroundSequence, this, flare,
channel),
base::BindOnce(&PasswordStore::OnInitCompleted, this)); base::BindOnce(&PasswordStore::OnInitCompleted, this));
} }
...@@ -636,12 +639,14 @@ PasswordStore::CreateBackgroundTaskRunner() const { ...@@ -636,12 +639,14 @@ PasswordStore::CreateBackgroundTaskRunner() const {
} }
bool PasswordStore::InitOnBackgroundSequence( bool PasswordStore::InitOnBackgroundSequence(
const syncer::SyncableService::StartSyncFlare& flare) { const syncer::SyncableService::StartSyncFlare& flare,
version_info::Channel channel) {
DCHECK(background_task_runner_->RunsTasksInCurrentSequence()); DCHECK(background_task_runner_->RunsTasksInCurrentSequence());
if (base::FeatureList::IsEnabled(switches::kSyncUSSPasswords)) { if (base::FeatureList::IsEnabled(switches::kSyncUSSPasswords)) {
sync_bridge_.reset(new PasswordSyncBridge( sync_bridge_.reset(new PasswordSyncBridge(
std::make_unique<syncer::ClientTagBasedModelTypeProcessor>( std::make_unique<syncer::ClientTagBasedModelTypeProcessor>(
syncer::PASSWORDS, base::DoNothing()), syncer::PASSWORDS,
base::BindRepeating(&syncer::ReportUnrecoverableError, channel)),
/*password_store_sync=*/this, sync_enabled_or_disabled_cb_)); /*password_store_sync=*/this, sync_enabled_or_disabled_cb_));
} else { } else {
DCHECK(!syncable_service_); DCHECK(!syncable_service_);
......
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "components/password_manager/core/browser/password_store_change.h" #include "components/password_manager/core/browser/password_store_change.h"
#include "components/password_manager/core/browser/password_store_sync.h" #include "components/password_manager/core/browser/password_store_sync.h"
#include "components/sync/model/syncable_service.h" #include "components/sync/model/syncable_service.h"
#include "components/version_info/version_info.h"
#if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED) #if defined(SYNC_PASSWORD_REUSE_DETECTION_ENABLED)
#include "components/password_manager/core/browser/hash_password_manager.h" #include "components/password_manager/core/browser/hash_password_manager.h"
...@@ -37,13 +38,13 @@ class PrefService; ...@@ -37,13 +38,13 @@ class PrefService;
namespace autofill { namespace autofill {
struct FormData; struct FormData;
struct PasswordForm; struct PasswordForm;
} } // namespace autofill
namespace syncer { namespace syncer {
class ModelTypeControllerDelegate; class ModelTypeControllerDelegate;
class ProxyModelTypeControllerDelegate; class ProxyModelTypeControllerDelegate;
class SyncableService; class SyncableService;
} } // namespace syncer
using StateSubscription = using StateSubscription =
base::CallbackList<void(const std::string& username)>::Subscription; base::CallbackList<void(const std::string& username)>::Subscription;
...@@ -134,6 +135,7 @@ class PasswordStore : protected PasswordStoreSync, ...@@ -134,6 +135,7 @@ class PasswordStore : protected PasswordStoreSync,
bool Init( bool Init(
const syncer::SyncableService::StartSyncFlare& flare, const syncer::SyncableService::StartSyncFlare& flare,
PrefService* prefs, PrefService* prefs,
version_info::Channel channel = version_info::Channel::UNKNOWN,
base::RepeatingClosure sync_enabled_or_disabled_cb = base::DoNothing()); base::RepeatingClosure sync_enabled_or_disabled_cb = base::DoNothing());
// RefcountedKeyedService: // RefcountedKeyedService:
...@@ -448,7 +450,8 @@ class PasswordStore : protected PasswordStoreSync, ...@@ -448,7 +450,8 @@ class PasswordStore : protected PasswordStoreSync,
// background sequence. Subclasses can add more logic. Returns true on // background sequence. Subclasses can add more logic. Returns true on
// success. // success.
virtual bool InitOnBackgroundSequence( virtual bool InitOnBackgroundSequence(
const syncer::SyncableService::StartSyncFlare& flare); const syncer::SyncableService::StartSyncFlare& flare,
version_info::Channel channel);
// Methods below will be run in PasswordStore's own sequence. // Methods below will be run in PasswordStore's own sequence.
// Synchronous implementation that reports usage metrics. // Synchronous implementation that reports usage metrics.
......
...@@ -22,8 +22,7 @@ PasswordStoreDefault::PasswordStoreDefault( ...@@ -22,8 +22,7 @@ PasswordStoreDefault::PasswordStoreDefault(
std::unique_ptr<LoginDatabase> login_db) std::unique_ptr<LoginDatabase> login_db)
: login_db_(std::move(login_db)) {} : login_db_(std::move(login_db)) {}
PasswordStoreDefault::~PasswordStoreDefault() { PasswordStoreDefault::~PasswordStoreDefault() = default;
}
void PasswordStoreDefault::ShutdownOnUIThread() { void PasswordStoreDefault::ShutdownOnUIThread() {
PasswordStore::ShutdownOnUIThread(); PasswordStore::ShutdownOnUIThread();
...@@ -31,7 +30,8 @@ void PasswordStoreDefault::ShutdownOnUIThread() { ...@@ -31,7 +30,8 @@ void PasswordStoreDefault::ShutdownOnUIThread() {
} }
bool PasswordStoreDefault::InitOnBackgroundSequence( bool PasswordStoreDefault::InitOnBackgroundSequence(
const syncer::SyncableService::StartSyncFlare& flare) { const syncer::SyncableService::StartSyncFlare& flare,
version_info::Channel channel) {
DCHECK(background_task_runner()->RunsTasksInCurrentSequence()); DCHECK(background_task_runner()->RunsTasksInCurrentSequence());
DCHECK(login_db_); DCHECK(login_db_);
bool success = true; bool success = true;
...@@ -42,7 +42,7 @@ bool PasswordStoreDefault::InitOnBackgroundSequence( ...@@ -42,7 +42,7 @@ bool PasswordStoreDefault::InitOnBackgroundSequence(
success = false; success = false;
LOG(ERROR) << "Could not create/open login database."; LOG(ERROR) << "Could not create/open login database.";
} }
return PasswordStore::InitOnBackgroundSequence(flare) && success; return PasswordStore::InitOnBackgroundSequence(flare, channel) && success;
} }
void PasswordStoreDefault::ReportMetricsImpl( void PasswordStoreDefault::ReportMetricsImpl(
...@@ -155,9 +155,8 @@ bool PasswordStoreDefault::RemoveStatisticsByOriginAndTimeImpl( ...@@ -155,9 +155,8 @@ bool PasswordStoreDefault::RemoveStatisticsByOriginAndTimeImpl(
const base::Callback<bool(const GURL&)>& origin_filter, const base::Callback<bool(const GURL&)>& origin_filter,
base::Time delete_begin, base::Time delete_begin,
base::Time delete_end) { base::Time delete_end) {
return login_db_ && return login_db_ && login_db_->stats_table().RemoveStatsByOriginAndTime(
login_db_->stats_table().RemoveStatsByOriginAndTime( origin_filter, delete_begin, delete_end);
origin_filter, delete_begin, delete_end);
} }
std::vector<std::unique_ptr<PasswordForm>> std::vector<std::unique_ptr<PasswordForm>>
......
...@@ -34,7 +34,8 @@ class PasswordStoreDefault : public PasswordStore { ...@@ -34,7 +34,8 @@ class PasswordStoreDefault : public PasswordStore {
// Opens |login_db_| on the background sequence. // Opens |login_db_| on the background sequence.
bool InitOnBackgroundSequence( bool InitOnBackgroundSequence(
const syncer::SyncableService::StartSyncFlare& flare) override; const syncer::SyncableService::StartSyncFlare& flare,
version_info::Channel channel) override;
// Implements PasswordStore interface. // Implements PasswordStore interface.
void ReportMetricsImpl(const std::string& sync_username, void ReportMetricsImpl(const std::string& sync_username,
......
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