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

Issue 907353 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Task

Blocked on: View detail
issue 910016
issue 915034

Blocking:
issue 918089
issue 918986



Sign in to add a comment

Log more metrics for installed web apps

Project Member Reported by benwells@chromium.org, Nov 21

Issue description

There is currently SiteEngagementService.EngagementType and Webapp.Engagement.EngagementType. The former is for all engagement events, the latter is for engagement events for installed PWAs running in windows.

We should metrics for engagement events from:
* installed PWAs in tabs
* installed web apps (PWAs and non-PWAs) in tabs
* installed web apps (PWAs and non-PWAs) in windows

We should also do versions of all of these excluding default installed web apps / PWAs.

Lastly, we should have metrics recording all engagement events but split by:
* users who have no manually installed web apps (i.e. ignoring default installed).
* users who have 1-3 manually installed web apps
* users who have 4 or more manually installed web apps.

I don't really mind how we organise these metrics, but I'll note the name of Webapp.Engagement.EngagementType is pretty confusing (as it only counts PWAs). Maybe we should sunset that and have things like:
Webapp.EngagementType.PWAInTab
Webapp.EngagementType.WebAppInWindow
Webapp.EngagementType.NoWebAppsInstalled etc.

To make this happen I'd suggest having something that lives off WebAppProvider and subscribes to SiteEngagementService events.
 
Cc: -dominickn@google.com dominickn@chromium.org
Status: Assigned (was: Available)
Seems like there might be a combinatorial explosion if we want to log {tab vs window} × {PWAs vs non-PWAs} × {default vs non-default installed} × {0, 1–3, 4+ manually installed apps}.

That's 24 buckets right there.
Sorry I miscommunicated. The last one {0, 1-3, 4+} is just a break down of all engagement events, regardless of source.

Also, it isn't PWAs vs. non-PWAs, it's PWAs / all web apps.
Also, it isn't default vs non-default, it is all (default + non-default) / non-default.

So, all the metrics:
PWA tab
PWA window
WebApp tab
WebApp window
Non-default PWA tab*
Non-default PWA window*
Non-default WebApp tab
Non-default WebApp window

0 app engagement event types
1-3 app engagement event types
4+ app engagement event types

So, 11 total. If you want we could skip the Non-default PWA metric (*s) to bring it down to 9.
Blockedon: 910016
Labels: -Type-Bug OS-Chrome OS-Linux OS-Mac OS-Windows Type-Task
Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 10

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

commit fbe86f4341bbc1f65866a82684db2c1d748ecf8c
Author: Alexey Baskakov <loyso@chromium.org>
Date: Mon Dec 10 00:56:03 2018

BookmarkApp: Implement TabHelper to associate a tab with BookmarkApp.

- Add IsInNavigationScopeForLaunchUrl util.
- Add GetInstalledShortcutForUrl util.

TBR=ellyjones@chromium.org

Bug:  907353 
Change-Id: I09ff56744efc7317861ace71e8979670149cbeaa
Reviewed-on: https://chromium-review.googlesource.com/c/1355315
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615021}
[modify] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/ui/tab_helpers.cc
[modify] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/BUILD.gn
[modify] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/components/BUILD.gn
[add] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/components/web_app_tab_helper_base.cc
[add] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/components/web_app_tab_helper_base.h
[modify] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/extensions/BUILD.gn
[modify] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/extensions/bookmark_app_installation_task.cc
[add] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/extensions/bookmark_app_tab_helper.cc
[add] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/extensions/bookmark_app_tab_helper.h
[modify] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/extensions/bookmark_app_util.cc
[modify] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/extensions/bookmark_app_util.h
[add] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/extensions/bookmark_app_util_unittest.cc
[modify] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/web_app_provider.cc
[modify] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/web_app_provider.h
[add] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/web_app_tab_helper.cc
[add] https://crrev.com/fbe86f4341bbc1f65866a82684db2c1d748ecf8c/chrome/browser/web_applications/web_app_tab_helper.h

Question for Dom and Matt. What shoudl we use to distinguish between PWAs and non-PWAs once we stop differentiating them at the Bookmark App level?

Would it make sense to record metrics based on the result of the installability check of the current site?
Hm, that's a good one. Ideally we don't re-run the installability check on sites running in standalone (there's really no point). Is it possible to keep a stored bit indicating that the site is a PWA (e.g. a new variant of Extension::from_bookmark() for now, and something better on BMO)?
Storing a boolean per web apps sounds good to me. Basically it would just be something like:
-> installed via a promoted method (beforeinstallprompt, install menu item): true
-> installed via create shortcut: false.

Is that enough?
In that case we would be recording metrics for apps that were PWAs when they were installed as opposed to being a PWA at the time the user interacted with it. Is that what we want?

The edge case I'm wondering is if say someone installs Gmail today and then Gmail becomes a proper PWA, we would still count it as if it were a shortcut.
That's probably okay - given that we deprioritise installation for non-PWAs, the number of people who install something before it comes a PWA, then keep using it after it becomes a PWA should hopefully not skew our metrics that much. :)

Comment 14 Deleted

Hmmm. Interesting discussion. I think both installed-via-promoted-surface (which is slightly different to PWA-at-install-time) and PWA-when-used are interesting metrics but tell us different things:

1. installed-via-promoted-surface tells us about how useful our promotion surfaces are.

2. PWA-when-used tells us how people are using sites that are real valid PWAs.

(1) is what I think I'm most interested in now. (2) is also interesting but maybe to truly log that we also wouldn't care if the thing was installed or not.

Thoughts?
Just to clarify: for 1. we would be measuring how useful are the apps that we promoted, right?

I think that makes sense. We should find a better name to indicate that we are measuring installed-via-promoted-surface apps though.

Aside: do we care about the promoted surface? Should we distinguish between installed through install button in page vs. install button in three dot menu vs. install button in omnibox?

Also do we think we can get valuable data without UKMs? I'm concerned that one or two popular sites e.g. Spotify or Gmail will completely skew the metrics one way.
Yeah it would be a combination of how useful the apps are and also how prominent the promotion surfaces are. And yeah, I think it would probably be better to store an enum for how it was installed somehow.

Absolutely UKMs will be helpful.
Ben: is the enum a requirement? Or could we start with "installed through install button in page or in web app" vs. "installed through Create shortcut"?
No, I think it would be better, but not a requirement.
Another question: What do we want to know about default apps. The distinction between "installed through install button" vs "installed through Create Shortcut" doesn't make sense in this case. Should we just distinguish between launch container i.e. installed to launch in a window vs installed to launch in a tab.
Blockedon: 915034
Right. If we had an install source enum, 'default install' would be one of those.
Cc: raymes@chromium.org
raymes@ suggested: maybe we could have a metric for showing the offline dinosaur page in an installed app (to see how big a problem it is that we currently don't require the fetch handler to do anything).
Project Member

Comment 24 by bugdroid1@chromium.org, Dec 19

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

commit 4135e244116c51bd6af426bfe4978e4c4cf0cae0
Author: Alexey Baskakov <loyso@chromium.org>
Date: Wed Dec 19 07:08:34 2018

SiteEngagementService Unit Test: Add extra precondition.

If some third service depends on SiteEngagementServiceFactory and indirectly
creates SiteEngagementService instance as a part of TestingProfile then we
get data races in UMA histograms: unit tests fail.

This CL adds a CHECK that only one instance of SiteEngagementService exists in
the test fixture.

Bug:  907353 
Change-Id: I0028628d4391cc1260f19aa8291c3c299b9fb141
Reviewed-on: https://chromium-review.googlesource.com/c/1382715
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617749}
[modify] https://crrev.com/4135e244116c51bd6af426bfe4978e4c4cf0cae0/chrome/browser/engagement/site_engagement_service_factory.cc
[modify] https://crrev.com/4135e244116c51bd6af426bfe4978e4c4cf0cae0/chrome/browser/engagement/site_engagement_service_factory.h
[modify] https://crrev.com/4135e244116c51bd6af426bfe4978e4c4cf0cae0/chrome/browser/engagement/site_engagement_service_unittest.cc

Blocking: 918089
Blocking: 918986
Project Member

Comment 27 by bugdroid1@chromium.org, Jan 8

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

commit da22037beeb0774ee6d8a73718fb96fb86338aed
Author: Alexey Baskakov <loyso@chromium.org>
Date: Tue Jan 08 00:21:16 2019

WebApp: Implement SiteEngagement UMA histograms for Bookmark Apps.

The list:
    "WebApp.Engagement.InTab",
    "WebApp.Engagement.InWindow",
    "WebApp.Engagement.DefaultInstalled.InTab",
    "WebApp.Engagement.DefaultInstalled.InWindow",
    "WebApp.Engagement.UserInstalled.InTab",
    "WebApp.Engagement.UserInstalled.InWindow",
    "WebApp.Engagement.UserInstalled.FromInstallButton.InTab",
    "WebApp.Engagement.UserInstalled.FromInstallButton.InWindow",
    "WebApp.Engagement.UserInstalled.FromCreateShortcutButton.InTab",
    "WebApp.Engagement.UserInstalled.FromCreateShortcutButton.InWindow",
    "WebApp.Engagement.MoreThanThreeUserInstalledApps",
    "WebApp.Engagement.UpToThreeUserInstalledApps",
    "WebApp.Engagement.NoUserInstalledApps"

Bug:  907353 
Change-Id: I8208a02547431e99bb3025a1e85e32e8be95d221
Reviewed-on: https://chromium-review.googlesource.com/c/1369554
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620547}
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/engagement/site_engagement_service.h
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/extensions/browsertest_util.cc
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/extensions/browsertest_util.h
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/extensions/extension_browsertest.cc
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/extensions/extension_browsertest.h
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/extensions/application_launch.cc
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/extensions/hosted_app_browser_controller.cc
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/extensions/hosted_app_browser_controller.h
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/extensions/hosted_app_browsertest.cc
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/tab_helpers.cc
[add] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/web_applications/OWNERS
[add] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/web_applications/bookmark_app_browsertest.cc
[add] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/web_applications/web_app_metrics.cc
[add] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/web_applications/web_app_metrics.h
[add] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/web_applications/web_app_metrics_factory.cc
[add] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/ui/web_applications/web_app_metrics_factory.h
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/web_applications/components/web_app_tab_helper_base.h
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/web_applications/extensions/bookmark_app_tab_helper.cc
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/web_applications/extensions/bookmark_app_tab_helper.h
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/web_applications/extensions/bookmark_app_util.cc
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/web_applications/extensions/bookmark_app_util.h
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/web_applications/web_app_provider.cc
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/web_applications/web_app_provider.h
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/web_applications/web_app_tab_helper.cc
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/browser/web_applications/web_app_tab_helper.h
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/chrome/test/BUILD.gn
[modify] https://crrev.com/da22037beeb0774ee6d8a73718fb96fb86338aed/tools/metrics/histograms/histograms.xml

Project Member

Comment 29 by bugdroid1@chromium.org, Jan 9

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

commit ba48955236490b2c9110333b3006dfe9e9851dc5
Author: Alexey Baskakov <loyso@chromium.org>
Date: Wed Jan 09 05:55:17 2019

WebApp: Make sure that we record UMA histograms only for allowed profiles.

No recording for off-the-record, guest and system profiles.

Bug:  907353 
Change-Id: Icafb7ba6ba61d22f91f3d83e2b79c63c56d22860
Reviewed-on: https://chromium-review.googlesource.com/c/1395837
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621056}
[modify] https://crrev.com/ba48955236490b2c9110333b3006dfe9e9851dc5/chrome/browser/ui/web_applications/web_app_metrics.cc

Status: Fixed (was: Started)
Status: Started (was: Fixed)
Project Member

Comment 32 by bugdroid1@chromium.org, Jan 11

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

commit 2571dc8c84018b9f2a027fbcd938e50f3eacdd80
Author: Alexey Baskakov <loyso@chromium.org>
Date: Fri Jan 11 07:28:42 2019

WebApp: Rework filtering of allowed profiles for metrics.

Decide in Factory's GetBrowserContextToUse whether to bypass creation of
WebAppMetrics service for a given profile.

This is a much cleaner and more efficient way to do so,
No behavior change.

Bug:  907353 
Change-Id: I925977faca2e5dd917fe16a910bd65110215257a
Reviewed-on: https://chromium-review.googlesource.com/c/1405163
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621934}
[modify] https://crrev.com/2571dc8c84018b9f2a027fbcd938e50f3eacdd80/chrome/browser/ui/web_applications/web_app_metrics.cc
[modify] https://crrev.com/2571dc8c84018b9f2a027fbcd938e50f3eacdd80/chrome/browser/ui/web_applications/web_app_metrics_factory.cc
[modify] https://crrev.com/2571dc8c84018b9f2a027fbcd938e50f3eacdd80/chrome/browser/ui/web_applications/web_app_metrics_factory.h

Status: Fixed (was: Started)

Sign in to add a comment