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

Issue 847921 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR

Blocked on:
issue 841133



Sign in to add a comment

Chrome shows up multiple times in Default Browser app picker (Android).

Project Member Reported by mthiesse@chromium.org, May 30 2018

Issue description

Chrome M67 is showing up multiple times in the Default Browser app picker under Settings > Apps > Advanced > Default Apps > Browser App.

This is because we now have two entry points in Chrome for View intents (VR and non-VR), and the Android platform code has a bug where it lists the same package multiple times, once for each component/alias that supports http/s View intents (even though it makes no distinction between components and targets the package rather than the component for default browser intents).
 
Screenshot_20180530-133017.png
109 KB View Download
Cc: tedc...@chromium.org yfried...@chromium.org mariakho...@chromium.org
Ted, Maria, wanted to loop you in on some of the ways we're considering handling this type of issue in general as this isn't the first time we've had issues like this.

One way we could solve this is by having VR intents go through the ChromeLauncherActivity, using the same IntentDispatcher alias. However, this has a number of problems:
1. We don't have a very reliable way of detecting VR intents. b/72217949 talks about this more, but briefly, VR intents typically perform intent *resolution* with the Daydream category, but then strip the category and send an explicit intent to the resolved component. We can use the DaydreamApi#isInVrSession API to detect if the user is in-headset when we receive the intent which kind of works, but it fails in multiple cases, like if the user skips VR entry flows Chrome could just launch in 2D when the user is expecting VR, or during Daydream's headset timeout after exiting VR, Chrome would erroneously launch into VR by tapping the 2D launcher icon. This would also make intent dispatching slightly slower as we have to make a bunch of Daydream API calls, but I think the calls are relatively cheap.
2. We use intent filters to make sure intents we don't support (like search intents) go to 2D Chrome rather than VR. We could do move this logic into LaunchIntentDispatcher but I feel like that could get messy (I could be wrong).
3. We'd have to move some messy platform bug workarounds, like relaunching the launcher into VR, into ChromeLauncherActivity. Kind of gross.

Alternatively, we can keep working around these symptoms as they crop up. I can trick Android into thinking the VR Intent Dispatcher isn't default browser material by adding <data android:host="*" /> to the Intent filters which fixes this bug.

Even more alternatively, we could just file an Android platform bug on their Browser App picker and ignore the issue - I expect the default Browser App picker doesn't see much use...

Thoughts?
Blockedon: 841133
(filed b/84207432)
I think my preferred method would be using <data android:host="*"> hack. IMO, if you leave it as-is we will see a sufficient volume of complaints because of all the devices where multiple browsers come pre-installed.


I guess the question now is, can I merge this back to 68 if  issue 841133  is only getting resolved in 68?

If I merge this back to 68, and a user has both 67 stable and 68 dev/beta/canary, then I think they'll see the intent picker (though I should double check) on navigation.

I think I'll have to wait a milestone for  issue 841133  to be fixed on stable :/
I don't understand why that issue affects you? I think we check that external resolvers are not Chrome in our flow before sending intents.
Are you sure, Maria? I don't think |getSpecializedHandlersWithFilter| alone checks it's not chrome but it can check if the filters match a specific package name. Maybe you're referring to a different bit and I'm missing it though.

We could add a simple check to ensure the resolved filter isn't chrome. That'd decouple the patches
I was thinking about "willChromeHandleIntent()" function, but I guess that's only currently used for incognito flow.

However, I wonder what the actual behaviour of "startActivityIfNeeded" is if we were to dispatch to ourselves. It might just work. So I would suggest starting by just testing this out before making any changes to the ExternalNavigationHandler.
Looks like LineageOS based ROMS show Chrome twice in their intent picker >.<  issue 849638 .
Cc: shiva...@samsung.com mthiesse@chromium.org
 Issue 851884  has been merged into this issue.
Labels: -M-68 M-69 Target-69
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 11

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

commit 3bb7dfb7bf825213c3d489e2471fe3c8823675e9
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed Jul 11 19:22:07 2018

Hide VR Intent Dispatcher from Android Default Browser list.

The Android platform's Default Browser code has a bug where it allows
the same package to show up multiple times despite not being able to
differentiate between the multiple components within the package that
caused it to detect multiple browsers within the package.

We can trick it into thinking the VR intent handling code isn't default
browser material by providing a wildcard host, making Android think
the intent handler uses a URI authority when it really doesn't.

Bug:  847921 
Change-Id: I2b271fc1c99495a0e80e301ff432690dd5a2d0a6
Reviewed-on: https://chromium-review.googlesource.com/1079261
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574281}
[modify] https://crrev.com/3bb7dfb7bf825213c3d489e2471fe3c8823675e9/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/3bb7dfb7bf825213c3d489e2471fe3c8823675e9/chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationHandler.java
[modify] https://crrev.com/3bb7dfb7bf825213c3d489e2471fe3c8823675e9/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java

Status: Fixed (was: Started)
Project Member

Comment 14 by bugdroid1@chromium.org, Jul 30

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

commit 9b3eab8e0314ea7090d0853aa1f9dbe54ede3b63
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Mon Jul 30 15:37:49 2018

Revert "Hide VR Intent Dispatcher from Android Default Browser list."

This reverts commit 3bb7dfb7bf825213c3d489e2471fe3c8823675e9.

Reason for revert: Breaks other chromium-based browsers.

Original change's description:
> Hide VR Intent Dispatcher from Android Default Browser list.
>
> The Android platform's Default Browser code has a bug where it allows
> the same package to show up multiple times despite not being able to
> differentiate between the multiple components within the package that
> caused it to detect multiple browsers within the package.
>
> We can trick it into thinking the VR intent handling code isn't default
> browser material by providing a wildcard host, making Android think
> the intent handler uses a URI authority when it really doesn't.
>
> Bug:  847921 
> Change-Id: I2b271fc1c99495a0e80e301ff432690dd5a2d0a6
> Reviewed-on: https://chromium-review.googlesource.com/1079261
> Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#574281}

TBR=yfriedman@chromium.org,mthiesse@chromium.org,tedchoc@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  847921 , 867583
Change-Id: I5580c3d43444a177e99f5f57fa93ca725312261d
Reviewed-on: https://chromium-review.googlesource.com/1155068
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579045}
[modify] https://crrev.com/9b3eab8e0314ea7090d0853aa1f9dbe54ede3b63/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/9b3eab8e0314ea7090d0853aa1f9dbe54ede3b63/chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationHandler.java
[modify] https://crrev.com/9b3eab8e0314ea7090d0853aa1f9dbe54ede3b63/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 30

Labels: merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d684d9418960d11303d3cfd664adbb7b3e62b3f7

commit d684d9418960d11303d3cfd664adbb7b3e62b3f7
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Mon Jul 30 19:09:12 2018

Revert "Hide VR Intent Dispatcher from Android Default Browser list."

This reverts commit 3bb7dfb7bf825213c3d489e2471fe3c8823675e9.

Reason for revert: Breaks other chromium-based browsers.

Original change's description:
> Hide VR Intent Dispatcher from Android Default Browser list.
>
> The Android platform's Default Browser code has a bug where it allows
> the same package to show up multiple times despite not being able to
> differentiate between the multiple components within the package that
> caused it to detect multiple browsers within the package.
>
> We can trick it into thinking the VR intent handling code isn't default
> browser material by providing a wildcard host, making Android think
> the intent handler uses a URI authority when it really doesn't.
>
> Bug:  847921 
> Change-Id: I2b271fc1c99495a0e80e301ff432690dd5a2d0a6
> Reviewed-on: https://chromium-review.googlesource.com/1079261
> Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
> Reviewed-by: Ted Choc <tedchoc@chromium.org>
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#574281}

TBR=mthiesse@chromium.org, tedchoc@chromium.org, yfriedman@chromium.org


(cherry picked from commit 9b3eab8e0314ea7090d0853aa1f9dbe54ede3b63)

Bug:  847921 , 867583
Change-Id: I5580c3d43444a177e99f5f57fa93ca725312261d
Reviewed-on: https://chromium-review.googlesource.com/1155068
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#579045}
Reviewed-on: https://chromium-review.googlesource.com/1155468
Cr-Commit-Position: refs/branch-heads/3497@{#226}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/d684d9418960d11303d3cfd664adbb7b3e62b3f7/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/d684d9418960d11303d3cfd664adbb7b3e62b3f7/chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationHandler.java
[modify] https://crrev.com/d684d9418960d11303d3cfd664adbb7b3e62b3f7/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappTabDelegate.java

Status: Started (was: Fixed)
Project Member

Comment 17 by bugdroid1@chromium.org, Aug 2

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

commit b1a0e71605a5d598eac7cbfea80bd105b2b027b8
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Thu Aug 02 22:08:17 2018

Merge 2D and VR intent filters.

This is enabled by three things:
1. We now trust that 2D intents should launch 2D Chrome even if the user
is in a VR headset so we don't have to use expensive APIs on launch.
2. Deep-linked apps and the associated launcher workarounds have been
removed.
3. Daydream has stopped removing the Daydream category from its intents
that target launching in VR.

This should mean that Chrome no longer shows up multiple times in the
default browser picker, and third-party ROMs won't get confused by
Chrome having multiple intent handlers that support View intents.

Bug:  847921 
Change-Id: I236fab049e00f64eb04d6fcb4a82f05f0d31f728
Reviewed-on: https://chromium-review.googlesource.com/1157272
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580344}
[modify] https://crrev.com/b1a0e71605a5d598eac7cbfea80bd105b2b027b8/chrome/android/java/AndroidManifest.xml
[delete] https://crrev.com/3e237d96560f3912f9a50c2158e0dcc89902305b/chrome/android/java/res_vr/values-v17/vr_styles.xml
[modify] https://crrev.com/b1a0e71605a5d598eac7cbfea80bd105b2b027b8/chrome/android/java/src/org/chromium/chrome/browser/LaunchIntentDispatcher.java
[modify] https://crrev.com/b1a0e71605a5d598eac7cbfea80bd105b2b027b8/chrome/android/java/src/org/chromium/chrome/browser/document/ChromeLauncherActivity.java
[modify] https://crrev.com/b1a0e71605a5d598eac7cbfea80bd105b2b027b8/chrome/android/java/src/org/chromium/chrome/browser/vr/VrDelegate.java
[modify] https://crrev.com/b1a0e71605a5d598eac7cbfea80bd105b2b027b8/chrome/android/java/src/org/chromium/chrome/browser/vr/VrDelegateFallback.java
[modify] https://crrev.com/b1a0e71605a5d598eac7cbfea80bd105b2b027b8/chrome/android/java/src/org/chromium/chrome/browser/vr/VrDelegateImpl.java
[modify] https://crrev.com/b1a0e71605a5d598eac7cbfea80bd105b2b027b8/chrome/android/java/src/org/chromium/chrome/browser/vr/VrIntentUtils.java
[delete] https://crrev.com/3e237d96560f3912f9a50c2158e0dcc89902305b/chrome/android/java/src/org/chromium/chrome/browser/vr/VrMainActivity.java
[modify] https://crrev.com/b1a0e71605a5d598eac7cbfea80bd105b2b027b8/chrome/android/java_sources.gni
[modify] https://crrev.com/b1a0e71605a5d598eac7cbfea80bd105b2b027b8/chrome/android/javatests/src/org/chromium/chrome/browser/vr/VrBrowserTransitionTest.java
[modify] https://crrev.com/b1a0e71605a5d598eac7cbfea80bd105b2b027b8/chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/VrBrowserTransitionUtils.java

Issue 870307 has been merged into this issue.
Status: Fixed (was: Started)
Issue 875850 has been merged into this issue.
Issue 879155 has been merged into this issue.
Cc: chelamcherla@chromium.org
This issue is fixed in latest canary 70.0.3537.0 but still seen in latest beta 69.0.3497.73 and stable 68.0.3440.91. As per comment#15 fix got merged into M-69.

@mthiesse: Please let us know which build of M-69 consists this fix.

Thanks!
This has not been merged into 69. The revert of the previous fix was merged into 69.
 Issue 880046  has been merged into this issue.
It seems that it was modified in version 70, but I have a question.

In version 67, the default browser is disabled, and the update is made from the playstore to version 70.
 
In this case, is Default browser set to Chrome again?
Updating chrome shouldn't impact any settings on default browser. Also I believe if the app is disabled it won't be updated (not sure though)
I will ask again.
In version 66, the default browser is set to chrome.
Updating to 67 or 68 will change default browser to none.
If you update to 70, it will ask you if chrome is set to default like 66.

Does the answer below tell me that the setting remains unchanged and remains at none?
"Updating chrome shouldn't impact any settings on default browser."
Sorry for my terse response and thanks for clarifying the question.

Regarding "Updating to 67 or 68 will change default browser to none." that's a surprise to me. You're saying merely taking the update from Play caused the default browser to be unset? That wasn't the original issue it was more that if the user was prompted to pick a default browser they would see chrome show twice. But had they set the default browser in 66, it should've stayed set in future versions.
I have additional questions to existing questions.
The preloaded version of 66 is Chrome as your default browser.
Preloaded version 67 is set to none as the default browser.

If you update to version 70.
Please confirm the operation of the model with 66 version and 67 version with preload.

If the version of the 67 version is updated to 70,
Does the default browser change from none to chrome?
Issue 886927 has been merged into this issue.

Comment 31 Deleted

Sign in to add a comment