Correctly record WebAPK install source for Launch.HomescreenSource

chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappLauncherActivity.java

Index: chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappLauncherActivity.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappLauncherActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappLauncherActivity.java
index 254a3c01086eced6705b77be521e2dcd7a7b0e6d..1a054cf089017db7956d2162c9c1365bb8f75501 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappLauncherActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappLauncherActivity.java
@@ -79,7 +79,11 @@ public class WebappLauncherActivity extends Activity {
         // - the intent was sent by Chrome.
         if (validWebApk || isValidMacForUrl(webappUrl, webappMac)
                 || wasIntentFromChrome(intent)) {
-            LaunchMetrics.recordHomeScreenLaunchIntoStandaloneActivity(webappUrl, webappSource);
+            if (!validWebApk) {
+                // We record the launch metrics for WebAPKs in the WebApkActivity. This is because
+                // we can only get the source info from WebappDataStorage, not in the APK.
+                LaunchMetrics.recordHomeScreenLaunchIntoStandaloneActivity(webappUrl, webappSource);
+            }

[pkotwicz, 2017/05/05 21:05:43]

Can we query the source from WebappDataStorage here?

Also your code in WebApkActivity#onResumeWithNative() seems to ignore the source
when it is passed via the intent (e.g. when the WebAPK is launched from a
notification)

[Xi Han, 2017/05/08 20:55:28]

If we query the source here, we could potentially slow down the launch of
WebAPKs, right? In fact, I don't think recording launch metrics for webapk in
here is right. The WebappLauncherActivity is always created no matter whether
the associated WebApkActivity has been created or not. We ends up recording more
than the real launch times.
Good catch, updated. Now we only records the source once, and we record the
source of Notification when a WebAPK is launched from a notification, but not if
the notification just brings a existing WebAPK to foreground.

[pkotwicz, 2017/05/09 18:37:36]

- I think that the launch metrics are recorded each time WebappLauncherActivity
is created instead of when a Webapp/WebAPK is created on purpose. I don't have a
strong opinion as to whether we should record metrics when
WebappLauncherActivity is created or when WebappActivity/WebApkActivity is
created but it would be nice to be consistent for WebAPKs and Webapps

- Querying WebappDataStorage is cheap compared to isValidWebApk()

- In the future, we might check WebappDataStorage to determine whether a WebAPK
is valid prior to running isValidWebApk() to speed up startup time (because
verifying unbound WebAPKs is so slow) so we might end up querying
WebappDataStorage in WebappLauncherActivity anyways

[Xi Han, 2017/05/10 20:49:58]

Yes, we would check WebappDataStorage here. Updated my code. Now we don't need
to specialize WebAPK case. I guess I understand why the
LaunchMetrics.recordHomeScreenLaunchIntoStandaloneActivity() is called here.
This is because it doesn't matter where it is called, the
LaunchMetrics.commitMetrics() is called only once in
ChromeActvity#onResumeWithNative().

             Intent launchIntent = createWebappLaunchIntent(webappInfo, webappSource, validWebApk);
             startActivity(launchIntent);
             return;