Android P3A #7016

AlexeyBarabash · 2020-11-02T17:58:03Z

Submitter Checklist:

Test Plan:

I. Clean install

Install this PR apk as a clean installation
Launch, ensure you can't see informers
Go to Settings => Advanced => Privacy, ensure Automatically send completely private product analytics to Brave is unchecked
Restart app, Go to Settings => Advanced => Privacy, ensure Automatically send completely private product analytics to Brave is still unchecked
Check Automatically send completely... checkbox,
Restart app, ensure Automatically send completely... is still checked

II. Update install

Install version which does not contain this PR, for example, 1.18.45
Repeat all the steps from (I)

Advanced test to ensure PR is ready for onboarding changes, requires code change between runs, is not possible to perform by QA team

=====

This require to prepare special apk
comment out

            if (!BravePrefServiceBridge.getInstance().hasPathP3AEnabled()) {
                BravePrefServiceBridge.getInstance().setP3AEnabled(false);
            }

and uncomment

            if (!PackageUtils.isFirstInstall(this)) {
                BraveP3AInformers.show();
            } else {
                // On the first run P3A is must be set from the onboarding wizard
            }

at src/brave/android/java/org/chromium/chrome/browser/app/BraveActivity.java

III. Ready for onboarding, clean install

Install modified apk in a clean way
Run, ensure there is no informer displayed
Go to Settings => Advanced => Privacy, ensure Automatically send completely private... is checked
(P3A should be disabled or kept enabled by onboarding wizard, follow up issue)

IV. Ready for onboarding, update install

Install version which does not contain this PR, for example, 1.18.45
Run and exit to go through onboarding
Install modified apk
Run, ensure informer is shown
5.1 press Learn More ensure page https://brave.com/P3A is opened

5.2 repeat 5.1-5.4, press cross, ensure Automatically send completely private... is checked
5.3 repeat 5.1-5.4, press Disable, ensure Automatically send completely private... is unchecked
5.4 repeat 5.1-5.4, press Got it, ensure Automatically send completely private... is checked

Reviewer Checklist:

New files have MPL-2.0 license header.
Request a security/privacy review as needed.
Adequate test coverage exists to prevent regressions
Verify test plan is specified in PR before merging to source

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on.
All relevant documentation has been updated.

AlexeyBarabash · 2020-11-03T22:27:50Z

Current state:

Works initialization

11-02 22:36:36.563: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(217)] BraveP3AService::Init() Done!
11-02 22:36:36.563: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(218)] BraveP3AService parameters are:, average_upload_interval_ = 60 s, randomize_upload_interval_ = 1, upload_server_url_ = https://p3a.brave.com/, rotation_interval_ = 0 s
11-02 22:36:36.563: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(350)] Pyxis meta: android-bc developer 1.18.27 45 45 other other
11-02 22:36:36.564: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(485)] BraveP3AService new rotation timer will fire at 2020-11-08 22:00:00.000 UTC after 523403 s

Works upload

11-02 22:36:51.566: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(357)] BraveP3AService::StartScheduledUpload at 2020-11-02 20:36:51.566 UTC
11-02 22:36:51.567: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(364)] StartScheduledUpload - Nothing to stage.

11-02 22:40:20.569: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(357)] BraveP3AService::StartScheduledUpload at 2020-11-02 20:40:20.569 UTC
11-02 22:40:20.571: V/chromium(17534): [VERBOSE2:brave_p3a_log_store.cc(157)] BraveP3ALogStore::StageNextLog: staged Brave.Shields.UsageStatus
11-02 22:45:38.067: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(376)] StartScheduledUpload - Uploading 83 bytes of type p3a
11-02 22:45:38.204: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(462)] BraveP3AService::UploadFinished ok = 1 HTTP response = 200

Works lots of metrics:

11-02 22:36:37.341: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(434)] BraveP3AService::OnHistogramChanged: histogram_name = Brave.Core.BookmarksCountOnProfileLoad.2 Sample = 0 bucket = 0
11-02 22:36:37.362: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(434)] BraveP3AService::OnHistogramChanged: histogram_name = Brave.Search.DefaultEngine.2 Sample = 1 bucket = 1
11-02 22:36:37.374: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(434)] BraveP3AService::OnHistogramChanged: histogram_name = Brave.Core.IsDefault Sample = 0 bucket = 0
11-02 22:36:37.375: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(434)] BraveP3AService::OnHistogramChanged: histogram_name = Brave.Core.CrashReportsEnabled Sample = 0 bucket = 0
11-02 22:39:16.615: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(434)] BraveP3AService::OnHistogramChanged: histogram_name = Brave.Shields.UsageStatus Sample = 2 bucket = 2
11-02 22:44:18.636: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(434)] BraveP3AService::OnHistogramChanged: histogram_name = Brave.Rewards.WalletBalance.2 Sample = 2 bucket = 2
11-02 22:44:22.519: V/chromium(17534): [VERBOSE2:brave_p3a_service.cc(434)] BraveP3AService::OnHistogramChanged: histogram_name = Brave.Shields.UsageStatus Sample = 3 bucket = 3


11-03 20:18:19.498: V/chromium(6677): [VERBOSE2:brave_p3a_log_store.cc(157)] BraveP3ALogStore::StageNextLog: staged Brave.Core.BookmarksCountOnProfileLoad.2
11-03 20:19:04.672: V/chromium(6677): [VERBOSE2:brave_p3a_log_store.cc(157)] BraveP3ALogStore::StageNextLog: staged Brave.Rewards.AutoContributionsState.2
11-03 20:19:34.784: V/chromium(6677): [VERBOSE2:brave_p3a_log_store.cc(157)] BraveP3ALogStore::StageNextLog: staged Brave.Search.DefaultEngine.2


11-04 19:04:59.227: V/chromium(29939): [VERBOSE2:brave_p3a_service.cc(434)] BraveP3AService::OnHistogramChanged: histogram_name = Brave.Savings.BandwidthSavingsMB Sample = 0 bucket = 0

At least doesn't work Brave.Uptime.BrowserOpenMinutes because it classes available only for desktop CHromium, so I had do disable it.

AlexeyBarabash · 2020-11-03T23:05:16Z

Here is a table of all metrics to keep understand what works and what doesn't

For items left blank I don't know the answer yet.

Metric	Works	Comments
Brave.Core.BookmarksCountOnProfileLoad.2	yes
Brave.Core.CrashReportsEnabled	N/A	but histogram is sent `BraveP3AService::OnHistogramChanged: histogram_name = Brave.Core.CrashReportsEnabled Sample = 0 bucket = 0`
Brave.Core.IsDefault	no	shell_integration_android.cc - NOTIMPLEMENTED() in GetDefaultBrowser()
Brave.Core.LastTimeIncognitoUsed	no	Desktop implementation relies on BrowserListObserver which is not available on Android Issue: brave/brave-browser#12540
Brave.Core.NumberOfExtensions	N/A	Android does not support extensions
Brave.Core.TabCount	yes, fixed	`"metrics/tab_stats_tracker.cc"` is under `#!is_android` at `/chrome/browser/BUILD.gn` for Android there is `org.chromium.chrome.browser.metrics.UmaSessionStats`
Brave.Core.TorEverUsed	N/A	Android does not support TOR
Brave.Core.WindowCount.2	N/A	the same, `"metrics/tab_stats_tracker.cc"` is under `#!is_android` at `/chrome/browser/BUILD.gn`, there is no alternative at `uma_session_stats.cc` . And makes sense, there no window concept on Android
Brave.Importer.ImporterSource	N/A	import is not available on Android
Brave.Omnibox.SearchCount	no	`brave/browser/ui/omnibox/brave_omnibox_client_impl.cc` is under `if (!is_android) {` guard Issue: brave/brave-browser#12541
Brave.P3A.SentAnswersCount	yes	histogram is caught `BraveP3AService::OnHistogramChanged: histogram_name = Brave.P3A.SentAnswersCount Sample = 0 bucket = 0`
Brave.Rewards.AdsState.2	yes
Brave.Rewards.AutoContributionsState.2	yes
Brave.Rewards.TipsState.2	yes
Brave.Rewards.WalletBalance.2	yes
Brave.Savings.BandwidthSavingsMB	yes
Brave.Search.DefaultEngine.2	yes
Brave.SpeedReader.ToggleCount	N/A
Brave.SpeedReader.Enabled	N/A
Brave.Shields.UsageStatus	N/A
Brave.Sync.Status	no	This was broken with sync v2 (Sync v1 place) Issue:brave/brave-browser#10349
Brave.Uptime.BrowserOpenMinutes	no	uses BrowserListObserver which is not available on Android, the same as `Brave.Core.LastTimeIncognitoUsed` Issue: brave/brave-browser#12542
Brave.Welcome.InteractionStatus	no	`brave/browser/ui/webui/brave_welcome_ui.cc` is under `if (!is_android)` guard Issue: brave/brave-browser#12543
P2A
Ad Opportunities
Brave.P2A.TotalAdOpportunities	yes
Brave.P2A.AdOpportunitiesPerSegment.*	yes
Ad Impressions
Brave.P2A.TotalAdImpressions	yes
Brave.P2A.AdImpressionsPerSegment.*	yes

AlexeyBarabash · 2020-11-10T21:00:30Z

@jamesmudgett @bradleyrichter could you please review the UI changes? (mostly standard controls)

Preferences, light theme

Preferences, dark theme

Informer, light theme

Informer, dark theme

samartnik

lgtm

SergeyZhukovsky

++

android/java/org/chromium/chrome/browser/preferences/BravePrefServiceBridge.java

jamesmudgett · 2020-11-11T20:34:15Z

Looks great

iefremov · 2020-11-12T09:12:06Z

components/p3a/buildflags.gni

@@ -1,5 +1,5 @@
 import("//build/config/features.gni")

 declare_args() {
-  brave_p3a_enabled = !is_ios && !is_android
+  brave_p3a_enabled = !is_ios


AFAIU, is_ios is redundant, because we don't build p3a on ios anyway. So I suggest to drop the whole buildflag

I didn't know that, I know that a lot of brave-core code is used on iOS, especially for rewards and sync now.

if we drop !is_ios , I would like to put the comment that p3a is not used for iOS, like

# for iOS P3A is not used brave_p3a_enabled = true

so to keep the existing for me seems good because it is self-described line

AFAIK generally we don't use is_ios since it wouldn't work anyway, only selected components you mention support that. So I think we should be safe to drop the flag - @bsclifton could you help us?

iOS guys told it should be safe. I will remove that guard and run CI.

CI for iOS succeeded with that change 95553fa

browser/ui/android/strings/android_brave_strings.grd

browser/p3a/p3a_core_metrics.h

browser/brave_local_state_prefs.cc

AlexeyBarabash · 2020-11-12T15:38:06Z

macOS build failed by reason

17:32:37  In file included from ../../brave/browser/ui/bookmark/brave_bookmark_tab_helper.cc:11:
17:32:37  ../../chrome/browser/ui/webui/new_tab_page/new_tab_page_ui.h:13:10: fatal error: 'chrome/browser/ui/webui/new_tab_page/new_tab_page.mojom.h' file not found
17:32:37  #include "chrome/browser/ui/webui/new_tab_page/new_tab_page.mojom.h"
17:32:37           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

hard to believe it is caused by this PR

AlexeyBarabash · 2020-11-12T21:38:37Z

macOS CI failed for test-install

20:13:01  Contents
20:13:01  + open '/Volumes/Brave Browser Nightly/Brave Browser Nightly.app/Contents/MacOS/Brave Browser Nightly'
20:13:01  + sleep 10
20:13:11  + ps aux
20:13:11  + grep 'Brave Browser Nightly'
20:13:11  jenkins          61584   0.7  0.0  4399508    920   ??  S    10:13AM   0:00.00 grep Brave Browser Nightly
20:13:11  + killall 'Brave Browser Nightly'
20:13:11  No matching processes belonging to you were found

iefremov

Approving to unblock the merge, but would like to have P3A buildflag removed

AlexeyBarabash · 2020-11-16T18:39:26Z

Windows CI comleted with status UNSTABLE.

One browser-test failed and one timed-out

19:18:19  [705/705] RewardsBrowserTest.ResetRewardsWithBAT (TIMED OUT)
19:18:19  1 test failed:
19:18:19      RewardsContributionBrowserTest.TipConnectedPublisherVerified (../../brave/components/brave_rewards/browser/test/rewards_contribution_browsertest.cc:585)
19:18:19  1 test timed out:
19:18:19      RewardsBrowserTest.ResetRewardsWithBAT (../../brave/components/brave_rewards/browser/test/rewards_browsertest.cc:374)

both are not related to the PR

AlexeyBarabash self-assigned this Nov 2, 2020

AlexeyBarabash force-pushed the android_p3a branch 2 times, most recently from c3b6b24 to 3051388 Compare November 3, 2020 22:02

AlexeyBarabash force-pushed the android_p3a branch from 3051388 to fdd2c76 Compare November 3, 2020 22:59

AlexeyBarabash force-pushed the android_p3a branch from 600f6bf to 24b0e13 Compare November 10, 2020 15:32

AlexeyBarabash requested review from deeppandya, samartnik, mkarolin, iefremov, bradleyrichter and jamesmudgett November 10, 2020 20:43

AlexeyBarabash marked this pull request as ready for review November 10, 2020 20:46

AlexeyBarabash force-pushed the android_p3a branch from a1f827f to 27e54cc Compare November 10, 2020 20:46

AlexeyBarabash mentioned this pull request Nov 10, 2020

Fixes includes violation from components to browser at brave_sync_profile_sync_service.cc #7107

Merged

33 tasks

AlexeyBarabash requested a review from SergeyZhukovsky November 11, 2020 19:58

samartnik approved these changes Nov 11, 2020

View reviewed changes

SergeyZhukovsky approved these changes Nov 11, 2020

View reviewed changes

deeppandya reviewed Nov 11, 2020

View reviewed changes

android/java/org/chromium/chrome/browser/preferences/BravePrefServiceBridge.java Show resolved Hide resolved

jamesmudgett approved these changes Nov 11, 2020

View reviewed changes

iefremov reviewed Nov 12, 2020

View reviewed changes

browser/ui/android/strings/android_brave_strings.grd Outdated Show resolved Hide resolved

iefremov reviewed Nov 12, 2020

View reviewed changes

browser/p3a/p3a_core_metrics.h Outdated Show resolved Hide resolved

iefremov reviewed Nov 12, 2020

View reviewed changes

browser/brave_local_state_prefs.cc Show resolved Hide resolved

AlexeyBarabash force-pushed the android_p3a branch from 27e54cc to ad8ab49 Compare November 12, 2020 13:59

iefremov approved these changes Nov 13, 2020

View reviewed changes

AlexeyBarabash added 10 commits November 16, 2020 14:36

Enabled brave_p3a_enabled flag for Android

f8c9c4e

Map Android's Tabs.TabCountPerLoad into Brave.Core.TabCount

ba55873

Comment for Brave.Core.LastTimeIncognitoUsed metric

c5cbc10

Java support for P3A preferences

0c0dd8f

Checkbox for P3A in Android Settings=>Advanced=>Privacy

b56afd6

Made Android support brave_p3a_enabled gn argument

e24c42d

Android P3A informer for updated users

cf414af

Adjust P3A preferences for future onboarding

c2b3946

Do not include p3a_core_metrics.h on Android at all

6a4a4ad

Removing is_ios guard from brave_p3a_enabled variable

cfb2f36

AlexeyBarabash force-pushed the android_p3a branch from 95553fa to cfb2f36 Compare November 16, 2020 12:37

AlexeyBarabash merged commit 6b3a9ba into master Nov 16, 2020

AlexeyBarabash deleted the android_p3a branch November 16, 2020 18:42

AlexeyBarabash mentioned this pull request Nov 16, 2020

Implement onboarding for P3A on Android brave/brave-browser#12723

Closed

AlexeyBarabash added this to the 1.19.x - Nightly milestone Nov 18, 2020

AlexeyBarabash mentioned this pull request Jan 13, 2021

Enable p3a by default android (Uplift to 1.19.x) #7566

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Android P3A #7016

Android P3A #7016

AlexeyBarabash commented Nov 2, 2020 •

edited

Loading

AlexeyBarabash commented Nov 3, 2020 •

edited

Loading

AlexeyBarabash commented Nov 3, 2020 •

edited

Loading

AlexeyBarabash commented Nov 10, 2020

samartnik left a comment

SergeyZhukovsky left a comment

jamesmudgett commented Nov 11, 2020

iefremov Nov 12, 2020

AlexeyBarabash Nov 12, 2020

iefremov Nov 13, 2020

AlexeyBarabash Nov 13, 2020

AlexeyBarabash Nov 13, 2020

AlexeyBarabash commented Nov 12, 2020

AlexeyBarabash commented Nov 12, 2020

iefremov left a comment

AlexeyBarabash commented Nov 16, 2020 •

edited

Loading

Android P3A #7016

Android P3A #7016

Conversation

AlexeyBarabash commented Nov 2, 2020 • edited Loading

Submitter Checklist:

Test Plan:

Reviewer Checklist:

After-merge Checklist:

AlexeyBarabash commented Nov 3, 2020 • edited Loading

AlexeyBarabash commented Nov 3, 2020 • edited Loading

AlexeyBarabash commented Nov 10, 2020

samartnik left a comment

Choose a reason for hiding this comment

SergeyZhukovsky left a comment

Choose a reason for hiding this comment

jamesmudgett commented Nov 11, 2020

iefremov Nov 12, 2020

Choose a reason for hiding this comment

AlexeyBarabash Nov 12, 2020

Choose a reason for hiding this comment

iefremov Nov 13, 2020

Choose a reason for hiding this comment

AlexeyBarabash Nov 13, 2020

Choose a reason for hiding this comment

AlexeyBarabash Nov 13, 2020

Choose a reason for hiding this comment

AlexeyBarabash commented Nov 12, 2020

AlexeyBarabash commented Nov 12, 2020

iefremov left a comment

Choose a reason for hiding this comment

AlexeyBarabash commented Nov 16, 2020 • edited Loading

AlexeyBarabash commented Nov 2, 2020 •

edited

Loading

AlexeyBarabash commented Nov 3, 2020 •

edited

Loading

AlexeyBarabash commented Nov 3, 2020 •

edited

Loading

AlexeyBarabash commented Nov 16, 2020 •

edited

Loading