New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 793082 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Add tests for Sync consent to UKM

Project Member Reported by bcwh...@chromium.org, Dec 7 2017

Issue description

Add test coverage that for sync consent that checks whether it's properly hooked up to UKM as well as checking edge cases (custom passphrase, multi-profile, logging out of sync); Found via sync consent Audit. 

 
UKM disabled when any un-sync'd profile has an open window exists:
https://cs.chromium.org/chromium/src/chrome/browser/metrics/ukm_browsertest.cc?rcl=afccf64114b233fe65af7526b2f015efb2d1e7c1&l=148

UKM disabled when TYPED_URLS is deselected exists:
https://cs.chromium.org/chromium/src/chrome/browser/metrics/ukm_browsertest.cc?rcl=afccf64114b233fe65af7526b2f015efb2d1e7c1&l=213

That leaves custom passphrase and full logout.

Note that you can change to a custom passphrase while a user is already syncing  (!), so checks need to listen for that change too.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca0ec21fa47658e4974020082992a4f367da3cd0

commit ca0ec21fa47658e4974020082992a4f367da3cd0
Author: Brian White <bcwhite@chromium.org>
Date: Wed Dec 13 22:00:54 2017

Add test for UKM disable when passphrase given.

This uses a StatusChangeChecker to wait for the "sync" thread to flush
and requires changes to that class to support cross-posting of tasks
to accomplish such.

Bug:  793082 
Change-Id: I899a100fcdde97d804fff166868486b701b5699b
Reviewed-on: https://chromium-review.googlesource.com/818016
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523898}
[modify] https://crrev.com/ca0ec21fa47658e4974020082992a4f367da3cd0/chrome/browser/metrics/ukm_browsertest.cc
[modify] https://crrev.com/ca0ec21fa47658e4974020082992a4f367da3cd0/chrome/browser/sync/test/integration/profile_sync_service_harness.cc
[modify] https://crrev.com/ca0ec21fa47658e4974020082992a4f367da3cd0/chrome/browser/sync/test/integration/profile_sync_service_harness.h

Status: Fixed (was: Started)
The "logout" test was included in the "custom passphrase" CL so I believe this is done.

mpearson, my test calls SetEncryptionPassphrase after Sync is set-up so I believe covers the case you mentioned.  Let me know if otherwise.
Looks like it covers the case I mentioned, thanks.

Comment 6 by rkaplow@google.com, Jan 9 2018

Owner: holte@chromium.org
Status: Assigned (was: Fixed)
Reopening as these tests I believe are not handling iOS
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb060c31b6722af9f1fdc0254215e86a635ffa94

commit cb060c31b6722af9f1fdc0254215e86a635ffa94
Author: Steven Holte <holte@chromium.org>
Date: Thu Feb 01 01:51:06 2018

Add UKM SecondaryPassphraseTest

Bug:  793082 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I2576f1ae6d40bb20fa641bfaf24089db4dd5e29e
Reviewed-on: https://chromium-review.googlesource.com/884941
Commit-Queue: Steven Holte <holte@chromium.org>
Reviewed-by: Louis Romero <lpromero@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533520}
[modify] https://crrev.com/cb060c31b6722af9f1fdc0254215e86a635ffa94/ios/chrome/browser/metrics/ukm_egtest.mm

Comment 8 by holte@chromium.org, Feb 1 2018

Status: Fixed (was: Assigned)
Android work remains in crbug/804445
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9b60ddbdbc1239ecf7a0b8630fb602185bb32cb9

commit 9b60ddbdbc1239ecf7a0b8630fb602185bb32cb9
Author: Mohammad Refaat <mrefaat@chromium.org>
Date: Thu Feb 01 23:29:23 2018

Revert "Add UKM SecondaryPassphraseTest"

This reverts commit cb060c31b6722af9f1fdc0254215e86a635ffa94.

Reason for revert: Failing in 
https://logs.chromium.org/v/?s=chrome%2Fbb%2Finternal.bling.main%2Fipad11-device-x64%2F3061%2F%2B%2Frecipes%2Fsteps%2Fios_chrome_integration_egtests__iPad_Air_2_iOS_11.2.1__on_iOS-11.2.1%2F0%2Fstdout

Original change's description:
> Add UKM SecondaryPassphraseTest
> 
> Bug:  793082 
> Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
> Change-Id: I2576f1ae6d40bb20fa641bfaf24089db4dd5e29e
> Reviewed-on: https://chromium-review.googlesource.com/884941
> Commit-Queue: Steven Holte <holte@chromium.org>
> Reviewed-by: Louis Romero <lpromero@chromium.org>
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533520}

TBR=rkaplow@chromium.org,holte@chromium.org,lpromero@chromium.org

Change-Id: Ibab7e13f9e611ad2232c3f3574aeaf1f66737526
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  793082 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/898463
Reviewed-by: Mohammad Refaat <mrefaat@chromium.org>
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533852}
[modify] https://crrev.com/9b60ddbdbc1239ecf7a0b8630fb602185bb32cb9/ios/chrome/browser/metrics/ukm_egtest.mm

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2386eb5cfc72c0bf8f2d82ee1895fd4a222c27b0

commit 2386eb5cfc72c0bf8f2d82ee1895fd4a222c27b0
Author: Steven Holte <holte@chromium.org>
Date: Thu Feb 08 17:31:59 2018

Reland iOS UKM SecondaryPassphrase test.

Test is disabled on devices due to crbug.com/806784.

Bug:  793082 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Iec98546347260a5e7151521d9e9e2a414c8026c5
Reviewed-on: https://chromium-review.googlesource.com/907750
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Commit-Queue: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535423}
[modify] https://crrev.com/2386eb5cfc72c0bf8f2d82ee1895fd4a222c27b0/ios/chrome/browser/metrics/ukm_egtest.mm

Sign in to add a comment