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

Issue 780619 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

High number of VIEW/.Main intents not dispatched by CTA.onNewIntents

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

Issue description

Android.MainActivity.ExplicitMainViewIntentDispatched.OnNewIntent shows that 30% of VIEW intents with explicit com.google.android.apps.chrome.Main class are not dispatched as CCT intents.

We found that Chrome sends com.google.android.apps.chrome.Main intents in IntentHandler.startChromeLauncherActivityForTrustedIntent().

We need to change TAB_ACTIVITY_COMPONENT_CLASS_NAME to com.google.android.apps.chrome.IntentDispatcher in order to get correct picture of external VIEW/.Main intents.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 2 2017

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

commit af78fa9e4ea9f41ee0bf2b079de71cc82c006649
Author: Dmitry Skiba <dskiba@chromium.org>
Date: Thu Nov 02 18:10:18 2017

Don't send internal VIEW intents to .Main alias.

.Main activity alias points to ChromeTabbedActivity, but its VIEW intent
dispatching code is temporary and only handles CCT intents.

This CL changes IntentHandler to send intents to .IntentDispatcher alias,
which does proper dispatching.

Bug:  780619 
Change-Id: I12253656171f867df3478b1984ab8fc27ca66e3f
Reviewed-on: https://chromium-review.googlesource.com/750254
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Commit-Queue: Dmitry Skiba <dskiba@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513551}
[modify] https://crrev.com/af78fa9e4ea9f41ee0bf2b079de71cc82c006649/chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java

Hey dskiba@, is this critical enough to be merged into M63 release branch?

If it is just about getting a better understanding of explicit VIEW intents in beta, do you plan to disable it before M63 goes to stable?
Yes, we need to merge this. This serves two critical functions:
1) It ensures Chrome code path that dispatches the VIEW intents functions correctly.
2) This is the only way we can check that this issue accounts for all of the histogram hits and there aren't any other issues in the wild we're hitting.

This change is small and quite safe. It will not need to be disabled -- this is a permanent change.
Labels: Merge-Request-63
Project Member

Comment 5 by sheriffbot@chromium.org, Nov 2 2017

Labels: -Merge-Request-63 Hotlist-Merge-Reject Merge-Reject-63
The bug is marked as P3 or Feature. It should not be merged as M63 is in beta. 
Please contact the approriate milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Pri-3 -Hotlist-Merge-Reject -Merge-Reject-63 Merge-Request-63 Pri-1
Project Member

Comment 7 by sheriffbot@chromium.org, Nov 2 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-63 Merge-Request-63
Merge approved. Please make sure to verify the change branch 3239 after merging it.
Project Member

Comment 9 by sheriffbot@chromium.org, Nov 3 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-63 Merge-Approved-63
My bad!
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 6 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d4c7bd46771face53a4413cfc064a4cb9fb8c2b

commit 1d4c7bd46771face53a4413cfc064a4cb9fb8c2b
Author: Dmitry Skiba <dskiba@chromium.org>
Date: Mon Nov 06 17:48:45 2017

[Merge to M63] Don't send internal VIEW intents to .Main alias.

.Main activity alias points to ChromeTabbedActivity, but its VIEW intent
dispatching code is temporary and only handles CCT intents.

This CL changes IntentHandler to send intents to .IntentDispatcher alias,
which does proper dispatching.

Bug:  780619 
Change-Id: I12253656171f867df3478b1984ab8fc27ca66e3f
Reviewed-on: https://chromium-review.googlesource.com/750254
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Commit-Queue: Dmitry Skiba <dskiba@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#513551}(cherry picked from commit af78fa9e4ea9f41ee0bf2b079de71cc82c006649)
Reviewed-on: https://chromium-review.googlesource.com/755073
Reviewed-by: Dmitry Skiba <dskiba@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#388}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/1d4c7bd46771face53a4413cfc064a4cb9fb8c2b/chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java

Status: Fixed (was: Assigned)
The number of VIEW intents not dispatched by onNewIntent() dropped to 17% in 63.0.3239.41. We might need to dig more on this (in a separate issue).
Follow up: issue 789732.

Sign in to add a comment