Skip to content
Snippets Groups Projects
Commit a9bd336e authored by Ryan Sultanem's avatar Ryan Sultanem Committed by Chromium LUCI CQ
Browse files

[BatchUpload] Add entry point to the BatchUploadService

Modifies existing histogram to record an enum for each entry point,
instead of always true.
Histogram affected: `Sync.BatchUpload.Opened`.
The histogram was only recorded at most on Canary, making it ok to
modify the way it is recorded.

Bug: b:375351323
Change-Id: I515d9869b157c98ca5aea678506cb3a8af72ea6f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5973104


Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Commit-Queue: Ryan Sultanem <rsult@google.com>
Reviewed-by: default avatarRushan Suleymanov <rushans@google.com>
Cr-Commit-Position: refs/heads/main@{#1378978}
parent 42fa4c2f
No related branches found
No related tags found
No related merge requests found
Showing
with 144 additions and 48 deletions
......@@ -108,7 +108,7 @@ class BatchUploadBrowserTest : public InProcessBrowserTest {
base::RunLoop run_loop;
batch_upload_service->OpenBatchUpload(
browser,
browser, BatchUploadService::EntryPoint::kPasswordManagerSettings,
base::BindOnce(&BatchUploadBrowserTest::OnBatchUploadShownResult,
base::Unretained(this), run_loop.QuitClosure()));
......@@ -317,6 +317,7 @@ class BatchUploadDelegateFake : public BatchUploadDelegate {
void ShowBatchUploadDialog(
Browser* browser,
std::vector<syncer::LocalDataDescription> local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback) override {
local_data_description_list_ = std::move(local_data_description_list);
complete_callback_ = std::move(complete_callback);
......
......@@ -7,6 +7,7 @@
#include "base/functional/callback_forward.h"
#include "base/memory/raw_ptr.h"
#include "chrome/browser/profiles/batch_upload/batch_upload_service.h"
#include "components/sync/service/local_data_description.h"
class Browser;
......@@ -25,6 +26,7 @@ class BatchUploadDelegate {
virtual void ShowBatchUploadDialog(
Browser* browser,
std::vector<syncer::LocalDataDescription> local_data_descriptions_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback) = 0;
};
......
......@@ -86,6 +86,7 @@ BatchUploadService::~BatchUploadService() = default;
void BatchUploadService::OpenBatchUpload(
Browser* browser,
EntryPoint entry_point,
base::OnceCallback<void(bool)> success_callback) {
if (!IsUserEligibleToOpenDialog()) {
std::move(success_callback).Run(false);
......@@ -106,6 +107,7 @@ void BatchUploadService::OpenBatchUpload(
// the local data descriptions, no other dialog opening is triggered.
state_.dialog_state_ = std::make_unique<ResettableState::DialogState>();
state_.dialog_state_->browser_ = browser;
state_.dialog_state_->entry_point_ = entry_point;
state_.dialog_state_->dialog_shown_callback_ = std::move(success_callback);
RequestLocalDataDescriptions();
......@@ -135,6 +137,7 @@ void BatchUploadService::OnGetLocalDataDescriptionsReady(
delegate_->ShowBatchUploadDialog(
state_.dialog_state_->browser_,
GetOrderedListOfNonEmptyDataDescriptions(std::move(local_data_map)),
state_.dialog_state_->entry_point_,
/*complete_callback=*/
base::BindOnce(&BatchUploadService::OnBatchUplaodDialogResult,
base::Unretained(this)));
......
......@@ -37,13 +37,29 @@ class BatchUploadService : public KeyedService {
BatchUploadService& operator=(const BatchUploadService&) = delete;
~BatchUploadService() override;
// Lists the different entry points to the Batch Upload Dialog.
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
//
// LINT.IfChange(EntryPoint)
enum class EntryPoint {
kPasswordManagerSettings = 0,
kPasswordPromoCard = 1,
kMaxValue = kPasswordPromoCard,
};
// LINT.ThenChange(/tools/metrics/histograms/metadata/sync/enums.xml:BatchUploadEntryPoint)
// Attempts to open the Batch Upload modal dialog that allows uploading the
// local profile data. The dialog will only be opened if there are some local
// data (of any type) to show and the dialog is not shown already in the
// profile. `dialog_shown_callback` returns whether the dialog was shown or
// not.
// TODO(crbug.com/375351323): Remove the default entry point value when adding
// the real entry points at the call sites.
void OpenBatchUpload(
Browser* browser,
EntryPoint entry_point = EntryPoint::kPasswordManagerSettings,
base::OnceCallback<void(bool)> dialog_shown_callback = base::DoNothing());
// Returns whether the dialog is currently showing on a browser.
......@@ -96,6 +112,8 @@ class BatchUploadService : public KeyedService {
struct DialogState {
// Browser that is showing the dialog.
raw_ptr<Browser> browser_;
// Entry point of the dialog.
EntryPoint entry_point_ = EntryPoint::kPasswordManagerSettings;
// Called when the decision about showing the dialog is made.
// Returns whether it was shown or not.
base::OnceCallback<void(bool)> dialog_shown_callback_;
......
......@@ -40,6 +40,7 @@ class BatchUploadDelegateMock : public BatchUploadDelegate {
ShowBatchUploadDialog,
(Browser * browser,
std::vector<syncer::LocalDataDescription> local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback),
(override));
};
......@@ -134,9 +135,11 @@ TEST_F(BatchUploadServiceTest, SignedOut) {
identity_manager().HasPrimaryAccount(signin::ConsentLevel::kSignin));
EXPECT_CALL(sync_service_mock(), GetLocalDataDescriptions(_, _)).Times(0);
EXPECT_CALL(delegate_mock(), ShowBatchUploadDialog(_, _, _)).Times(0);
EXPECT_CALL(delegate_mock(), ShowBatchUploadDialog(_, _, _, _)).Times(0);
EXPECT_CALL(opened_callback, Run(false)).Times(1);
service.OpenBatchUpload(nullptr, opened_callback.Get());
service.OpenBatchUpload(
nullptr, BatchUploadService::EntryPoint::kPasswordManagerSettings,
opened_callback.Get());
EXPECT_FALSE(service.IsDialogOpened());
}
......@@ -147,9 +150,11 @@ TEST_F(BatchUploadServiceTest, SignedPending) {
base::MockCallback<base::OnceCallback<void(bool)>> opened_callback;
EXPECT_CALL(sync_service_mock(), GetLocalDataDescriptions(_, _)).Times(0);
EXPECT_CALL(delegate_mock(), ShowBatchUploadDialog(_, _, _)).Times(0);
EXPECT_CALL(delegate_mock(), ShowBatchUploadDialog(_, _, _, _)).Times(0);
EXPECT_CALL(opened_callback, Run(false)).Times(1);
service.OpenBatchUpload(nullptr, opened_callback.Get());
service.OpenBatchUpload(
nullptr, BatchUploadService::EntryPoint::kPasswordManagerSettings,
opened_callback.Get());
EXPECT_FALSE(service.IsDialogOpened());
}
......@@ -161,9 +166,11 @@ TEST_F(BatchUploadServiceTest, Syncing) {
base::MockCallback<base::OnceCallback<void(bool)>> opened_callback;
EXPECT_CALL(sync_service_mock(), GetLocalDataDescriptions(_, _)).Times(0);
EXPECT_CALL(delegate_mock(), ShowBatchUploadDialog(_, _, _)).Times(0);
EXPECT_CALL(delegate_mock(), ShowBatchUploadDialog(_, _, _, _)).Times(0);
EXPECT_CALL(opened_callback, Run(false)).Times(1);
service.OpenBatchUpload(nullptr, opened_callback.Get());
service.OpenBatchUpload(
nullptr, BatchUploadService::EntryPoint::kPasswordManagerSettings,
opened_callback.Get());
EXPECT_FALSE(service.IsDialogOpened());
}
......@@ -179,9 +186,11 @@ TEST_F(BatchUploadServiceTest, NoLocalDataReturned) {
syncer::DataType::CONTACT_INFO},
_))
.Times(1);
EXPECT_CALL(delegate_mock(), ShowBatchUploadDialog(_, _, _)).Times(0);
EXPECT_CALL(delegate_mock(), ShowBatchUploadDialog(_, _, _, _)).Times(0);
EXPECT_CALL(opened_callback, Run(false)).Times(1);
service.OpenBatchUpload(nullptr, opened_callback.Get());
service.OpenBatchUpload(
nullptr, BatchUploadService::EntryPoint::kPasswordManagerSettings,
opened_callback.Get());
EXPECT_FALSE(service.IsDialogOpened());
}
......@@ -192,9 +201,11 @@ TEST_F(BatchUploadServiceTest, EmptyLocalDataReturned) {
SetReturnDescriptions(syncer::PASSWORDS, 0);
EXPECT_CALL(sync_service_mock(), GetLocalDataDescriptions(_, _)).Times(1);
EXPECT_CALL(delegate_mock(), ShowBatchUploadDialog(_, _, _)).Times(0);
EXPECT_CALL(delegate_mock(), ShowBatchUploadDialog(_, _, _, _)).Times(0);
EXPECT_CALL(opened_callback, Run(false)).Times(1);
service.OpenBatchUpload(nullptr, opened_callback.Get());
service.OpenBatchUpload(
nullptr, BatchUploadService::EntryPoint::kPasswordManagerSettings,
opened_callback.Get());
EXPECT_FALSE(service.IsDialogOpened());
}
......@@ -212,9 +223,11 @@ TEST_F(BatchUploadServiceTest,
EXPECT_CALL(
delegate_mock(),
ShowBatchUploadDialog(
_, std::vector<syncer::LocalDataDescription>{contact_info}, _));
_, std::vector<syncer::LocalDataDescription>{contact_info}, _, _));
EXPECT_CALL(opened_callback, Run(true)).Times(1);
service.OpenBatchUpload(nullptr, opened_callback.Get());
service.OpenBatchUpload(
nullptr, BatchUploadService::EntryPoint::kPasswordManagerSettings,
opened_callback.Get());
EXPECT_TRUE(service.IsDialogOpened());
}
......@@ -233,9 +246,11 @@ TEST_F(BatchUploadServiceTest,
delegate_mock(),
ShowBatchUploadDialog(
_, std::vector<syncer::LocalDataDescription>{passwords, contact_info},
_));
_, _));
EXPECT_CALL(opened_callback, Run(true)).Times(1);
service.OpenBatchUpload(nullptr, opened_callback.Get());
service.OpenBatchUpload(
nullptr, BatchUploadService::EntryPoint::kPasswordManagerSettings,
opened_callback.Get());
EXPECT_TRUE(service.IsDialogOpened());
}
......@@ -253,16 +268,19 @@ TEST_F(BatchUploadServiceTest, LocalDataReturnedShowsDialogAndReturnIdToMove) {
passwords, contact_infos};
BatchUploadSelectedDataTypeItemsCallback returned_complete_callback;
EXPECT_CALL(delegate_mock(),
ShowBatchUploadDialog(_, expected_descriptions, _))
ShowBatchUploadDialog(_, expected_descriptions, _, _))
.WillOnce(
[&](Browser* browser,
const std::vector<syncer::LocalDataDescription>&
local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback) {
returned_complete_callback = std::move(complete_callback);
});
EXPECT_CALL(opened_callback, Run(true)).Times(1);
service.OpenBatchUpload(nullptr, opened_callback.Get());
service.OpenBatchUpload(
nullptr, BatchUploadService::EntryPoint::kPasswordManagerSettings,
opened_callback.Get());
EXPECT_TRUE(service.IsDialogOpened());
std::map<syncer::DataType, std::vector<syncer::LocalDataItemModel::DataId>>
......@@ -287,16 +305,19 @@ TEST_F(BatchUploadServiceTest,
passwords, contact_infos};
BatchUploadSelectedDataTypeItemsCallback returned_complete_callback;
EXPECT_CALL(delegate_mock(),
ShowBatchUploadDialog(_, expected_descriptions, _))
ShowBatchUploadDialog(_, expected_descriptions, _, _))
.WillOnce(
[&](Browser* browser,
const std::vector<syncer::LocalDataDescription>&
local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback) {
returned_complete_callback = std::move(complete_callback);
});
EXPECT_CALL(opened_callback, Run(true)).Times(1);
service.OpenBatchUpload(nullptr, opened_callback.Get());
service.OpenBatchUpload(
nullptr, BatchUploadService::EntryPoint::kPasswordManagerSettings,
opened_callback.Get());
EXPECT_TRUE(service.IsDialogOpened());
std::map<syncer::DataType, std::vector<syncer::LocalDataItemModel::DataId>>
......
......@@ -13,9 +13,10 @@ BatchUploadUIDelegate::~BatchUploadUIDelegate() = default;
void BatchUploadUIDelegate::ShowBatchUploadDialog(
Browser* browser,
std::vector<syncer::LocalDataDescription> local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback) {
CHECK(browser);
ShowBatchUploadDialogInternal(*browser,
std::move(local_data_description_list),
std::move(complete_callback));
entry_point, std::move(complete_callback));
}
......@@ -20,6 +20,7 @@ class BatchUploadUIDelegate : public BatchUploadDelegate {
void ShowBatchUploadDialog(
Browser* browser,
std::vector<syncer::LocalDataDescription> local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback) override;
private:
......@@ -29,6 +30,7 @@ class BatchUploadUIDelegate : public BatchUploadDelegate {
void ShowBatchUploadDialogInternal(
Browser& browser,
std::vector<syncer::LocalDataDescription> local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback);
};
......
......@@ -106,11 +106,12 @@ BatchUploadDialogView::~BatchUploadDialogView() {
BatchUploadDialogView* BatchUploadDialogView::CreateBatchUploadDialogView(
Browser& browser,
std::vector<syncer::LocalDataDescription> local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback) {
std::unique_ptr<BatchUploadDialogView> dialog_view =
base::WrapUnique(new BatchUploadDialogView(
browser.profile(), std::move(local_data_description_list),
std::move(complete_callback)));
entry_point, std::move(complete_callback)));
BatchUploadDialogView* dialog_view_ptr = dialog_view.get();
gfx::NativeWindow window = browser.tab_strip_model()
......@@ -125,8 +126,10 @@ BatchUploadDialogView* BatchUploadDialogView::CreateBatchUploadDialogView(
BatchUploadDialogView::BatchUploadDialogView(
Profile* profile,
std::vector<syncer::LocalDataDescription> local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback)
: complete_callback_(std::move(complete_callback)) {
: complete_callback_(std::move(complete_callback)),
entry_point_(entry_point) {
SetModalType(ui::mojom::ModalType::kWindow);
// No native buttons.
SetButtons(static_cast<int>(ui::mojom::DialogButton::kNone));
......@@ -239,7 +242,7 @@ void BatchUploadDialogView::SetHeightAndShowWidget(int height) {
widget->Show();
RecordAvailableDataTypes(data_item_count_map_);
base::UmaHistogramBoolean("Sync.BatchUpload.Opened", true);
base::UmaHistogramEnumeration("Sync.BatchUpload.Opened", entry_point_);
}
}
......@@ -293,9 +296,10 @@ views::WebView* BatchUploadDialogView::GetWebViewForTesting() {
void BatchUploadUIDelegate::ShowBatchUploadDialogInternal(
Browser& browser,
std::vector<syncer::LocalDataDescription> local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback) {
BatchUploadDialogView::CreateBatchUploadDialogView(
browser, std::move(local_data_description_list),
browser, std::move(local_data_description_list), entry_point,
std::move(complete_callback));
}
......
......@@ -7,6 +7,7 @@
#include "base/scoped_observation.h"
#include "chrome/browser/profiles/batch_upload/batch_upload_delegate.h"
#include "chrome/browser/profiles/batch_upload/batch_upload_service.h"
#include "components/signin/public/identity_manager/account_info.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/sync/service/local_data_description.h"
......@@ -54,6 +55,7 @@ class BatchUploadDialogView : public views::DialogDelegateView,
static BatchUploadDialogView* CreateBatchUploadDialogView(
Browser& browser,
std::vector<syncer::LocalDataDescription> local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback);
views::WebView* GetWebViewForTesting();
......@@ -73,6 +75,7 @@ class BatchUploadDialogView : public views::DialogDelegateView,
explicit BatchUploadDialogView(
Profile* profile,
std::vector<syncer::LocalDataDescription> local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback);
// Callback to properly resize the view based on the loaded web ui content.
......@@ -111,6 +114,7 @@ class BatchUploadDialogView : public views::DialogDelegateView,
// Account info for which the data is showing.
AccountInfo primary_account_info_;
BatchUploadSelectedDataTypeItemsCallback complete_callback_;
BatchUploadService::EntryPoint entry_point_;
raw_ptr<views::WebView> web_view_;
......
......@@ -86,6 +86,7 @@ class BatchUploadDialogViewBrowserTest : public InProcessBrowserTest {
BatchUploadDialogView* CreateBatchUploadDialogView(
Profile* profile,
std::vector<syncer::LocalDataDescription> local_data_description_list,
BatchUploadService::EntryPoint entry_point,
BatchUploadSelectedDataTypeItemsCallback complete_callback) {
content::TestNavigationObserver observer{
GURL(chrome::kChromeUIBatchUploadURL)};
......@@ -93,7 +94,7 @@ class BatchUploadDialogViewBrowserTest : public InProcessBrowserTest {
BatchUploadDialogView* dialog_view =
BatchUploadDialogView::CreateBatchUploadDialogView(
*browser(), std::move(local_data_description_list),
*browser(), std::move(local_data_description_list), entry_point,
std::move(complete_callback));
observer.Wait();
......@@ -147,8 +148,11 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
std::vector<syncer::LocalDataDescription> descriptions;
syncer::DataType type = syncer::DataType::PASSWORDS;
descriptions.push_back(GetFakeLocalData(type, 1));
BatchUploadDialogView* dialog_view = CreateBatchUploadDialogView(
browser()->profile(), std::move(descriptions), mock_callback.Get());
BatchUploadService::EntryPoint entry_point =
BatchUploadService::EntryPoint::kPasswordManagerSettings;
BatchUploadDialogView* dialog_view =
CreateBatchUploadDialogView(browser()->profile(), std::move(descriptions),
entry_point, mock_callback.Get());
EXPECT_CALL(mock_callback, Run(kEmptySelectedMap)).Times(1);
......@@ -161,7 +165,8 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
{"Sync.BatchUpload.DialogCloseReason", 1}};
EXPECT_THAT(histogram_tester().GetTotalCountsForPrefix("Sync.BatchUpload."),
testing::ContainerEq(expected_histograms_count));
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", true, 1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", entry_point,
1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.DataTypeAvailable",
DataTypeHistogramValue(type), 1);
histogram_tester().ExpectUniqueSample(
......@@ -177,11 +182,14 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
syncer::DataType input_type = syncer::DataType::PASSWORDS;
EXPECT_CALL(mock_callback, Run(kEmptySelectedMap)).Times(1);
BatchUploadService::EntryPoint entry_point =
BatchUploadService::EntryPoint::kPasswordManagerSettings;
{
std::vector<syncer::LocalDataDescription> descriptions;
descriptions.push_back(GetFakeLocalData(input_type, 1));
BatchUploadDialogView* dialog_view = CreateBatchUploadDialogView(
browser()->profile(), std::move(descriptions), mock_callback.Get());
browser()->profile(), std::move(descriptions), entry_point,
mock_callback.Get());
// Simulate the widget closing without user action.
views::Widget* widget = dialog_view->GetWidget();
......@@ -197,7 +205,8 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
};
EXPECT_THAT(histogram_tester().GetTotalCountsForPrefix("Sync.BatchUpload."),
testing::ContainerEq(expected_histograms_count));
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", true, 1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", entry_point,
1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.DataTypeAvailable",
DataTypeHistogramValue(input_type), 1);
histogram_tester().ExpectUniqueSample(
......@@ -213,8 +222,11 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
std::vector<syncer::LocalDataDescription> descriptions;
syncer::DataType type = syncer::DataType::PASSWORDS;
descriptions.push_back(GetFakeLocalData(type, 1));
BatchUploadDialogView* dialog_view = CreateBatchUploadDialogView(
browser()->profile(), std::move(descriptions), mock_callback.Get());
BatchUploadService::EntryPoint entry_point =
BatchUploadService::EntryPoint::kPasswordPromoCard;
BatchUploadDialogView* dialog_view =
CreateBatchUploadDialogView(browser()->profile(), std::move(descriptions),
entry_point, mock_callback.Get());
// Pressing the escape key should dismiss the dialog and return empty result.
EXPECT_CALL(mock_callback, Run(kEmptySelectedMap)).Times(1);
......@@ -229,7 +241,8 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
};
EXPECT_THAT(histogram_tester().GetTotalCountsForPrefix("Sync.BatchUpload."),
testing::ContainerEq(expected_histograms_count));
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", true, 1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", entry_point,
1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.DataTypeAvailable",
DataTypeHistogramValue(type), 1);
histogram_tester().ExpectUniqueSample(
......@@ -256,8 +269,11 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
std::vector<syncer::LocalDataDescription> descriptions;
syncer::DataType type = syncer::DataType::PASSWORDS;
descriptions.push_back(GetFakeLocalData(type, 1));
BatchUploadDialogView* dialog_view = CreateBatchUploadDialogView(
browser()->profile(), std::move(descriptions), mock_callback.Get());
BatchUploadService::EntryPoint entry_point =
BatchUploadService::EntryPoint::kPasswordPromoCard;
BatchUploadDialogView* dialog_view =
CreateBatchUploadDialogView(browser()->profile(), std::move(descriptions),
entry_point, mock_callback.Get());
ASSERT_TRUE(dialog_view->GetWidget()->IsVisible());
// Signing out should close the dialog.
......@@ -271,7 +287,8 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
};
EXPECT_THAT(histogram_tester().GetTotalCountsForPrefix("Sync.BatchUpload."),
testing::ContainerEq(expected_histograms_count));
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", true, 1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", entry_point,
1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.DataTypeAvailable",
DataTypeHistogramValue(type), 1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.DialogCloseReason",
......@@ -298,8 +315,11 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
std::vector<syncer::LocalDataDescription> descriptions;
syncer::DataType type = syncer::DataType::PASSWORDS;
descriptions.push_back(GetFakeLocalData(type, 1));
BatchUploadDialogView* dialog_view = CreateBatchUploadDialogView(
browser()->profile(), std::move(descriptions), mock_callback.Get());
BatchUploadService::EntryPoint entry_point =
BatchUploadService::EntryPoint::kPasswordPromoCard;
BatchUploadDialogView* dialog_view =
CreateBatchUploadDialogView(browser()->profile(), std::move(descriptions),
entry_point, mock_callback.Get());
ASSERT_TRUE(dialog_view->GetWidget()->IsVisible());
// Signing out should close the dialog.
......@@ -313,7 +333,8 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
};
EXPECT_THAT(histogram_tester().GetTotalCountsForPrefix("Sync.BatchUpload."),
testing::ContainerEq(expected_histograms_count));
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", true, 1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", entry_point,
1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.DataTypeAvailable",
DataTypeHistogramValue(type), 1);
histogram_tester().ExpectUniqueSample(
......@@ -334,8 +355,11 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
syncer::DataType type2 = syncer::DataType::CONTACT_INFO;
int count2 = 2;
descriptions.push_back(GetFakeLocalData(type2, count2));
BatchUploadDialogView* dialog_view = CreateBatchUploadDialogView(
browser()->profile(), std::move(descriptions), mock_callback.Get());
BatchUploadService::EntryPoint entry_point =
BatchUploadService::EntryPoint::kPasswordPromoCard;
BatchUploadDialogView* dialog_view =
CreateBatchUploadDialogView(browser()->profile(), std::move(descriptions),
entry_point, mock_callback.Get());
std::map<syncer::DataType, std::vector<syncer::LocalDataItemModel::DataId>>
result;
......@@ -354,7 +378,8 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
};
EXPECT_THAT(histogram_tester().GetTotalCountsForPrefix("Sync.BatchUpload."),
testing::ContainerEq(expected_histograms_count));
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", true, 1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", entry_point,
1);
histogram_tester().ExpectBucketCount("Sync.BatchUpload.DataTypeAvailable",
DataTypeHistogramValue(type1), 1);
histogram_tester().ExpectBucketCount("Sync.BatchUpload.DataTypeAvailable",
......@@ -382,8 +407,11 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
syncer::DataType type2 = syncer::DataType::CONTACT_INFO;
int count2 = 2;
descriptions.push_back(GetFakeLocalData(type2, count2));
BatchUploadDialogView* dialog_view = CreateBatchUploadDialogView(
browser()->profile(), std::move(descriptions), mock_callback.Get());
BatchUploadService::EntryPoint entry_point =
BatchUploadService::EntryPoint::kPasswordPromoCard;
BatchUploadDialogView* dialog_view =
CreateBatchUploadDialogView(browser()->profile(), std::move(descriptions),
entry_point, mock_callback.Get());
std::map<syncer::DataType, std::vector<syncer::LocalDataItemModel::DataId>>
result;
......@@ -408,7 +436,8 @@ IN_PROC_BROWSER_TEST_F(BatchUploadDialogViewBrowserTest,
};
EXPECT_THAT(histogram_tester().GetTotalCountsForPrefix("Sync.BatchUpload."),
testing::ContainerEq(expected_histograms_count));
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", true, 1);
histogram_tester().ExpectUniqueSample("Sync.BatchUpload.Opened", entry_point,
1);
histogram_tester().ExpectBucketCount("Sync.BatchUpload.DataTypeAvailable",
DataTypeHistogramValue(type1), 1);
histogram_tester().ExpectBucketCount("Sync.BatchUpload.DataTypeAvailable",
......
......@@ -5,6 +5,7 @@
#include <string>
#include "base/i18n/number_formatting.h"
#include "chrome/browser/profiles/batch_upload/batch_upload_service.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/ui/test/test_browser_dialog.h"
#include "chrome/browser/ui/views/frame/browser_view.h"
......@@ -169,6 +170,7 @@ class BatchUploadDialogViewPixelTest
BatchUploadDialogView::CreateBatchUploadDialogView(
*browser(), fake_descriptions_,
BatchUploadService::EntryPoint::kPasswordManagerSettings,
/*complete_callback*/ base::DoNothing());
widget_waiter.WaitIfNeededAndGet();
......
......@@ -71,6 +71,15 @@ chromium-metrics-reviews@google.com.
<int value="5" label="Signout"/>
</enum>
<!-- LINT.IfChange(BatchUploadEntryPoint) -->
<enum name="BatchUploadEntryPoint">
<int value="0" label="Password manager settings"/>
<int value="1" label="Password promo card"/>
</enum>
<!-- LINT.ThenChange(/chrome/browser/profiles/batch_upload/batch_upload_service.h:EntryPoint) -->
<!-- LINT.IfChange(BookmarkGUIDSource) -->
<enum name="BookmarkGUIDSource">
......
......@@ -227,13 +227,13 @@ chromium-metrics-reviews@google.com.
</summary>
</histogram>
<histogram name="Sync.BatchUpload.Opened" enum="BooleanHit"
<histogram name="Sync.BatchUpload.Opened" enum="BatchUploadEntryPoint"
expires_after="2025-04-14">
<owner>rsult@google.com</owner>
<owner>chrome-signin-team@google.com</owner>
<summary>
Records that the Batch Upload Dialog was opened. Records when the dialog is
first shown.
Records the entry point from which the Batch Upload Dialog was opened.
Records when the dialog is first shown.
</summary>
</histogram>
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment