Web Share Target: If manifest url is invalid, crashes |
||
Issue descriptionDCHECK fail in share_service_impl.cc:155 if any manifest_url from the Preferences 'profile.web_share.visited_targets' key is blank or invalid. I don't know how this can happen, but it somehow happened to my Preferences file. We shouldn't crash when the users Preferences is corrupted, if possible, so let's just continue gracefully.
,
Sep 26 2017
Looks like the last 3 call sites (ManifestManagerHost::RenderFrameDeleted, ManifestManager::GetManifest, ManifestManager::ResolveCallbacks) are either passing empty GURL because it's legitimately a missing manifest, or they've since been fixed. It's just BookmarkAppHelper::CreateFromAppBanner that's a problem.
,
Sep 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e0739ed09afe6042c36e9a76da3bc58431b75b71 commit e0739ed09afe6042c36e9a76da3bc58431b75b71 Author: Matt Giuca <mgiuca@chromium.org> Date: Tue Sep 26 09:28:22 2017 Web Share Target (desktop): Fail gracefully if manifest URL is blank. Previously it would DCHECK if any share target in the Preferences file had a blank or invalid manifest URL, which could be caused if the share target was previously installed using an app banner. Since this is corrupted user data, we should recover, not DCHECK, in these cases. Also adds a DCHECK to catch cases that would write this corrupted data into the preferences. This doesn't yet fix the issue, but it does report it to developers. The DCHECK only fires if the manifest has a share_target and the --enable-experimental-web-platform-features flag is enabled (so it won't be too annoying). Bug: 762388 Change-Id: Ibd800dc529510d4755068f7bb0c55f8ca083bc7e Reviewed-on: https://chromium-review.googlesource.com/652011 Commit-Queue: Matt Giuca <mgiuca@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Cr-Commit-Position: refs/heads/master@{#504325} [modify] https://crrev.com/e0739ed09afe6042c36e9a76da3bc58431b75b71/chrome/browser/webshare/share_service_impl.cc [modify] https://crrev.com/e0739ed09afe6042c36e9a76da3bc58431b75b71/chrome/browser/webshare/share_service_impl_unittest.cc [modify] https://crrev.com/e0739ed09afe6042c36e9a76da3bc58431b75b71/chrome/browser/webshare/share_target_pref_helper.cc
,
Sep 27 2017
That CL I landed (r504325) makes the steps to reproduce the crash far more direct (in a DCHECK-enabled build on Linux): 1. Run Chrome with --enable-experimental-web-platform-features --enable-features=AppBanners --bypass-app-banner-engagement-checks 2. Visit: https://wicg.github.io/web-share-target/demos/sharetarget.html 3. Should see an app install banner. Click "Add" to install it through the banner. *crash* [409:409:0927/132726.288965:FATAL:share_target_pref_helper.cc(30)] Check failed: manifest_url.is_valid(). I've got another CL to pass the correct URL through: http://crrev.com/c/686084
,
Sep 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9e58d5c52840a09b62ec32afd0f3153c0be21b70 commit 9e58d5c52840a09b62ec32afd0f3153c0be21b70 Author: Matt Giuca <mgiuca@chromium.org> Date: Thu Sep 28 01:54:54 2017 Fix invalid Web share targets when app added via install banner. Apps added via the install banner were being installed with an empty manifest URL. The correct manifest URL is now passed through to BookmarkAppHelper::OnDidGetManifest so the share target is recorded correctly. Bug: 762388 Change-Id: I705c1f067946d3e7349c0d3be8e06c77b096be72 Reviewed-on: https://chromium-review.googlesource.com/686084 Reviewed-by: Dominick Ng <dominickn@chromium.org> Reviewed-by: Finnur Thorarinsson <finnur@chromium.org> Commit-Queue: Dominick Ng <dominickn@chromium.org> Cr-Commit-Position: refs/heads/master@{#504864} [modify] https://crrev.com/9e58d5c52840a09b62ec32afd0f3153c0be21b70/chrome/browser/banners/app_banner_infobar_delegate_desktop.cc [modify] https://crrev.com/9e58d5c52840a09b62ec32afd0f3153c0be21b70/chrome/browser/banners/app_banner_infobar_delegate_desktop.h [modify] https://crrev.com/9e58d5c52840a09b62ec32afd0f3153c0be21b70/chrome/browser/banners/app_banner_manager_desktop.cc [modify] https://crrev.com/9e58d5c52840a09b62ec32afd0f3153c0be21b70/chrome/browser/extensions/bookmark_app_helper.cc [modify] https://crrev.com/9e58d5c52840a09b62ec32afd0f3153c0be21b70/chrome/browser/extensions/bookmark_app_helper.h [modify] https://crrev.com/9e58d5c52840a09b62ec32afd0f3153c0be21b70/chrome/browser/extensions/bookmark_app_helper_unittest.cc
,
Sep 28 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by mgiuca@chromium.org
, Sep 25 2017