https://wrong.host.badssl.com/ causes WebAPK to crash |
||||||||||||
Issue descriptionhttps://wrong.host.badssl.com/ causes WebAPK to crash
,
Oct 31 2016
,
Dec 19 2016
,
Jan 16 2017
Mine!
,
Jan 18 2017
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.
,
Jan 18 2017
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.
,
Jan 18 2017
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?
,
Jan 18 2017
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.
,
Jan 18 2017
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.
,
Jan 18 2017
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.
,
Jan 24 2017
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
,
Mar 15 2017
Mine!
,
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
,
Mar 17 2017
Should we merge this back to 58?
,
Mar 17 2017
Sam: Yes
,
Mar 28 2017
+merge-request -- this fixes a (rare) crash and IMO has low merge risk (from a quick read-over of the patch.)
,
Mar 28 2017
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
,
Apr 1 2017
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
,
Apr 3 2017
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".
,
Apr 3 2017
|
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by pkotw...@chromium.org
, Oct 29 2016Status: Assigned (was: Untriaged)