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

Issue 658498 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

https://wrong.host.badssl.com/ causes WebAPK to crash

Project Member Reported by pkotw...@chromium.org, Oct 22 2016

Issue description

https://wrong.host.badssl.com/ causes WebAPK to crash

 
Owner: pkotw...@chromium.org
Status: Assigned (was: Untriaged)
Owner: hanxi@chromium.org
Labels: -Pri-3 Pri-1
Owner: pkotw...@chromium.org
Mine!
The crash is caused by the Tab#goBack() call in WebappActivity#onDidAttachInterstitialPage() when the WebAPK's start URL is wrong.host.badssl.com because there is no page to go back to.
Cc: pkotw...@chromium.org
Owner: dfalcant...@chromium.org
Assigning to Dan for suggestions on how to fix the crash.

The crash is caused by the Tab#goBack() call in WebappActivity#onDidAttachInterstitialPage() when the WebAPK's start URL is wrong.host.badssl.com because there is no page to go back to.

WebappActivity#onDidAttachInterstitialPage() seems to now open a Chrome custom tab instead of the full blown browser. 
Cc: -pkotw...@chromium.org tedc...@chromium.org dfalcant...@chromium.org
Owner: pkotw...@chromium.org
I wonder if the Custom Tab is an Herb thing (Ted?).

Peter: you can stop the crash at least by adding a conditional to check if the Tab#canGoBack() before calling back on it.  I'm not sure what the correct behavior should be, though.  How often do you picture a user adding a WebAPK to a bad website?
The fact that it opens a Custom Tab does sound like Herb.  We could force Tabbed mode if we felt the need.

I guess if we want to be super proactive, we could remove the task in this case (on L+ at least), but it is such a rare case that not crashing is probably fine.
Dan: I am unsure how to fix things in order to address the concern in comment #4 in Issue 293717

There are two ways that I can foresee a user adding a WebAPK for a bad website
1) The user installs a WebAPK at the time that the site isn't bad. The site becomes bad over time.
2) The user installs a WebAPK. The Web Manifest is updated to point to a bad page on the site.

In general, I don't think that interstitials show often. I don't expect users to install WebAPKs for sites which show interstititals often either.

At the very least, the user should be forced out of the WebAPK into the main Chrome browser to show the interstitial because of how Chromium handles it: only one interstitial is shown per domain shown across all Activities, all other Activities/tabs showing the domain freeze until the interstitial is handled.  Firing the Intent should cause the interstitial to be opened up in the right spot, and Android should handle the Activity opening accordingly.

The problem here is that users weren't ever expected to trigger this on startup.  I wonder if the proper thing to do is to finish the WebappActivity when the user can't go back?  It's probably the least terrible course of action.
Owner: hanxi@chromium.org
Assigning to Xi. I don't think that I will get to this bug this week. (And it should probably be fixed this week)

Comment #6 has the source of the crash
Owner: pkotw...@chromium.org
Mine!
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 17 2017

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

commit 33060182ee896b89c54ec758ac7fccc372e25c1a
Author: pkotwicz <pkotwicz@chromium.org>
Date: Fri Mar 17 04:19:10 2017

[WebAPKs] Don't crash if WebAPK starts on page which displays interstitial

This CL:
- Fixes "launch Chrome" intent to actually launch Chrome instead of Herb Chrome
  Custom Tab.
- Finishes the WebAPK if there is no URL to navigate back to.

BUG= 658498 

Review-Url: https://codereview.chromium.org/2752133002
Cr-Commit-Position: refs/heads/master@{#457682}

[modify] https://crrev.com/33060182ee896b89c54ec758ac7fccc372e25c1a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java

Should we merge this back to 58?
Sam: Yes
Labels: Merge-Request-58
+merge-request -- this fixes a (rare) crash and IMO has low merge risk (from a quick read-over of the patch.)
Project Member

Comment 17 by sheriffbot@chromium.org, Mar 28 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 18 by bugdroid1@chromium.org, Apr 1 2017

Labels: -merge-approved-58 merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6db2798ee0ed98e89300a14e10a25cfe2659ff33

commit 6db2798ee0ed98e89300a14e10a25cfe2659ff33
Author: Peter Kotwicz <pkotwicz@google.com>
Date: Sat Apr 01 01:27:59 2017

Merge M58 [WebAPKs] Don't crash if WebAPK starts on page which displays interstitial

This CL:
- Fixes "launch Chrome" intent to actually launch Chrome instead of Herb Chrome
  Custom Tab.
- Finishes the WebAPK if there is no URL to navigate back to.

BUG= 658498 

Review-Url: https://codereview.chromium.org/2752133002
Cr-Commit-Position: refs/heads/master@{#457682}
(cherry picked from commit 33060182ee896b89c54ec758ac7fccc372e25c1a)

Review-Url: https://codereview.chromium.org/2789883004 .
Cr-Commit-Position: refs/branch-heads/3029@{#524}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/6db2798ee0ed98e89300a14e10a25cfe2659ff33/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActivity.java

Steps for test team:
1) Navigate to https://webapk-test.appspot.com/static/navigator.html
2) Tap "Add to Home screen" in the app menu
3) Open the installed WebAPK and enter "wrong.host.badssl.com/" in the text field. and tap the "Navigate" button
Expectations:
- Chrome should open and an interstitial should be shown.
- The WebAPK should not have navigated and still be at "the page with the text field and Navigate button".

Status: Fixed (was: Assigned)

Sign in to add a comment