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

Issue 789732 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 781841



Sign in to add a comment

Investigate high volume of VIEW intents not dispatched to custom tabs

Project Member Reported by dskiba@chromium.org, Nov 29 2017

Issue description

This is a follow-up to  issue 780619  to investigate why number of VIEW intents not dispatched from onNewIntent() is so high.

The UMA to look at is Android.MainActivity.ExplicitMainViewIntentDispatched.OnNewIntent, and for 63.0.3239.60 we have the following:

Not dispatched  22,658  22.58%
Dispatched      77,699  77.42%

Splitting by OS we get:

Android_Lollipop:

Not dispatched  6,001   44.63%
Dispatched      7,446   55.37%

Android_Marshmallow:

Not dispatched  6,222   28.78%
Dispatched      15,395  71.22%

Android_N:

Not dispatched  9,719   20.99%
Dispatched      36,585  79.01%

On KitKat and below that code is not active (.Main points to ChromeLauncherActivity, not ChromeTabbedActivity), so numbers are zero.

 

Comment 1 by dskiba@chromium.org, Nov 29 2017

Just FYI that M64 canary/dev shows similar distribution, so we can do experiments there.

Comment 2 by dskiba@chromium.org, Nov 29 2017

Status: Started (was: Untriaged)

Comment 3 by dskiba@chromium.org, Nov 29 2017

Analysis of how different branches in LaunchIntentDispatcher.dispatch() handle VIEW intents:

private @Action int dispatch() {
    if (url == null && tabId == Tab.INVALID_TAB_ID && !incognito
            && intentHandler.handleWebSearchIntent(mIntent)) {
        return Action.FINISH_ACTIVITY;
    }
    => Only handles Intent.ACTION_SEARCH and MediaStore.INTENT_ACTION_MEDIA_SEARCH
       actions


    if (WebappLauncherActivity.bringWebappToFront(tabId)) {
        return Action.FINISH_ACTIVITY_REMOVE_TASK;
    }
    => Can handle VIEW intents, since it only cares about BRING_TAB_TO_FRONT extra


    if (mIntent.hasCategory(Notification.INTENT_CATEGORY_NOTIFICATION_PREFERENCES)) {
        NotificationPlatformBridge.launchNotificationPreferences(mActivity, mIntent);
        return Action.FINISH_ACTIVITY;
    }
    => Android calls activity that has this in the manifest, for Chrome it's
       .IntentDispatcher alias to ChromeLauncherActivity


    if (InstantAppsHandler.getInstance().handleIncomingIntent(
                mActivity, mIntent, mIsCustomTabIntent && !mIsHerbIntent, false)) {
        return Action.FINISH_ACTIVITY;
    }
    => Can handle VIEW intents since if seems to care only about extras


    if (FirstRunFlowSequencer.launch(mActivity, mIntent, false /* requiresBroadcast */,
                false /* preferLightweightFre */)) {
        return Action.FINISH_ACTIVITY;
    }
    => Irrelevant, since CTA is already running


    if (!mIsCustomTabIntent && !FeatureUtilities.isDocumentMode(mActivity)) {
        return dispatchToTabbedActivity();
    }
    => Dispatches to CTA itself (i.e. ends up calling onNewIntent() anyway)


    if (mIsCustomTabIntent) {
        if (!mIntent.getBooleanExtra(
                    TrustedWebUtils.EXTRA_LAUNCH_AS_TRUSTED_WEB_ACTIVITY, false)
                || !launchTrustedWebActivity()) {
            launchCustomTabActivity();
        }
        return Action.FINISH_ACTIVITY;
    }
    => CTA.onNewIntent() handles this case already


    if (DocumentModeAssassin.getInstance().isMigrationNecessary()) {
        Log.d(TAG, "Diverting to UpgradeActivity via " + mActivity.getClass().getName());
        UpgradeActivity.launchInstance(mActivity, mIntent);
        return Action.FINISH_ACTIVITY_REMOVE_TASK;
    }
    => Irrelevant


So we have two cases that are not handled by CTA.maybeDispatchExplicitMainViewIntent():

1. WebappLauncherActivity.bringWebappToFront()

2. InstantAppsHandler.handleIncomingIntent()
And the first results are in:

Other   10    01.51%
Chrome  654   98.49%

I suspect something in Chrome copies original CTA intent and then changes action to VIEW (among other things).

Project Member

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

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

commit 67f6261f18e00169fd532e45ab682bf531376a30
Author: Dmitry Skiba <dskiba@chromium.org>
Date: Wed Dec 13 20:50:11 2017

Crash debug builds on undispatched VIEW/.Main intents.

See the bug - we're somehow getting nontrivial number of non-CCT VIEW
intents sent directly to .Main activity alias.

We reported intent sources to a histogram and found that majority of the
intents are from Chrome itself.

Static analysis didn't find places where we construct a VIEW intent with
.Main activity alias and EXTRA_APPLICATION_ID set to Chrome.

This CL adds code to crash debug builds when we detect non-CCT VIEW
intent sent to .Main activity alias. There is also a command line flag
to disable crashing.

This is a temporary change that will be reverted once we find the cause.

Bug: 789732
Change-Id: Ic0ed3067e5cccbc48f913a02acf9b54925e3b93c
Reviewed-on: https://chromium-review.googlesource.com/817625
Commit-Queue: Dmitry Skiba <dskiba@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523868}
[modify] https://crrev.com/67f6261f18e00169fd532e45ab682bf531376a30/chrome/android/java/src/org/chromium/chrome/browser/ChromeSwitches.java
[modify] https://crrev.com/67f6261f18e00169fd532e45ab682bf531376a30/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

Running into this locally:

java.lang.IllegalStateException: VIEW intent sent to .Main activity alias was not dispatched. PLEASE report the following info to crbug.com/789732: "Intent { act=android.intent.action.VIEW dat=chrome-native://bookmarks/folder/3 flg=0x10000000 pkg=com.google.android.apps.chrome cmp=com.google.android.apps.chrome/.Main (has extras) }, extras.keySet = [trusted_application_code_extra, com.android.browser.application_id, com.google.chrome.transition_type, org.chromium.chrome.browser.timestamp]". Use --dont-crash-on-view-main-intents flag to disable this check.

Build: ToT
Android Build Number: 7.1.2
Device: Nexus 9

This was using Monochrome.apk with is_debug = true.

Comment 8 by dskiba@chromium.org, Jan 19 2018

Woohoo, thanks for reporting! I'll take a look.

Comment 9 by dskiba@chromium.org, Jan 19 2018

Turned out we have places where we construct VIEW intents with component name of the current activity. One of such places is responsible for the intent in #7:

...
  [7] org.chromium.chrome.browser.IntentHandler.startActivityForTrustedIntentInternal (IntentHandler.java:196)
  [8] org.chromium.chrome.browser.IntentHandler.startActivityForTrustedIntent (IntentHandler.java:185)
  [9] org.chromium.chrome.browser.bookmarks.BookmarkUtils.openUrl (BookmarkUtils.java:52)
  [10] org.chromium.chrome.browser.bookmarks.BookmarkUtils.showBookmarkManager (BookmarkUtils.java:16)
  [11] org.chromium.chrome.browser.ChromeTabbedActivity$$Lambda$7.run (unknown)
  [12] org.chromium.chrome.browser.compositor.CompositorViewHolder.hideKeyboard (CompositorViewHolder.java:315)
  [13] org.chromium.chrome.browser.ChromeTabbedActivity.onMenuOrKeyboardAction (ChromeTabbedActivity.java:716)
  [14] org.chromium.chrome.browser.ChromeActivity.onOptionsItemSelected (ChromeActivity.java:770)
  [15] org.chromium.chrome.browser.appmenu.AppMenu.onItemClick (AppMenu.java:169)
  [16] org.chromium.chrome.browser.appmenu.AppMenuAdapter$$Lambda$3.onClick (unknown)
  [17] android.view.View.performClick (View.java:6,294)
  [18] android.view.View$PerformClick.run (View.java:24,770)
  [19] android.os.Handler.handleCallback (Handler.java:790)
  [20] android.os.Handler.dispatchMessage (Handler.java:99)

showBookmarkManager() calls openUrl() with activity.getComponentName(), and for CTA started via .Main alias it's com.google.android.apps.chrome/.Main.
Current Android.MainActivity.UndispatchedExplicitMainViewIntentSource data:

Other	430	04.45%
Chrome	9,212	95.43%
Gsa	11	00.11%


I'm hitting this on current ToT clicking items from the History activity.
Blocking: 781841
The history activity crash from #11 is from here:

  [1] org.chromium.chrome.browser.IntentHandler.startActivityForTrustedIntentInternal (IntentHandler.java:672)
  [2] org.chromium.chrome.browser.IntentHandler.startActivityForTrustedIntent (IntentHandler.java:651)
  [3] org.chromium.chrome.browser.history.HistoryManager.openUrl (HistoryManager.java:281)
  [4] org.chromium.chrome.browser.history.HistoryItem.open (HistoryItem.java:103)
  [5] org.chromium.chrome.browser.history.HistoryItemView.onClick (HistoryItemView.java:149)
  [6] org.chromium.chrome.browser.widget.selection.SelectableItemView.onClick (SelectableItemView.java:146)
  [7] android.view.View.performClick (View.java:5,637)
  [8] android.view.View$PerformClick.run (View.java:22,429)
  [9] android.os.Handler.handleCallback (Handler.java:751)
  [10] android.os.Handler.dispatchMessage (Handler.java:95)
  [11] android.os.Looper.loop (Looper.java:154)
  [12] android.app.ActivityThread.main (ActivityThread.java:6,121)
  [13] java.lang.reflect.Method.invoke (native method)
  [14] com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:889)
  [15] com.android.internal.os.ZygoteInit.main (ZygoteInit.java:779)


HistoryManager.openUrl() calls HistoryManager.getOpenUrlIntent(), which creates VIEW intent with mActivity.getComponentName().
Cc: tedc...@chromium.org
Ted, so here are two places which end up sending VIEW intents to activity.getComponentName(), which can be .Main activity. I feel we should fix these two cases to use ChromeLauncherActivity instead?

Or just remove the check and let them be?
If we want the nuclear option, could we just override startActivity on AsyncInitializationActivity, ChromeApplication, and SynchronousInitializationActivity to change the component if we see it as .Main?

If it is .Main, I think changing to ChromeLauncherActivity is correct, but we also wouldn't want to change it always if it were ChromeTabbedActivity2 for example.
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 15 2018

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

commit e484d3ec0ec66c07b8f18a3e5f8e2d1c19822d39
Author: Ted Choc <tedchoc@google.com>
Date: Fri Jun 15 20:52:17 2018

Do not dispatch intents from within Chrome to the .Main alias.

The .Main alias is meant for the launcher activity triggering
only, so we want to ensure we don't pollute that with intents
from within Chrome.

The two sources found thus far are bookmarks and history.

BUG=789732

Change-Id: I90dc10019a2d07076439f80f3f280d4635f2e02e
Reviewed-on: https://chromium-review.googlesource.com/1102573
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567781}
[modify] https://crrev.com/e484d3ec0ec66c07b8f18a3e5f8e2d1c19822d39/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
[modify] https://crrev.com/e484d3ec0ec66c07b8f18a3e5f8e2d1c19822d39/chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkUtils.java
[modify] https://crrev.com/e484d3ec0ec66c07b8f18a3e5f8e2d1c19822d39/chrome/android/java/src/org/chromium/chrome/browser/history/HistoryManager.java

Sign in to add a comment