Issue metadata
Sign in to add a comment
|
Regression: Chrome crashes while opening Chrome canvas app in guest user |
||||||||||||||||||||
Issue descriptionChrome Version: 73.0.3654.0/11512.0.0 dev channel Daisy,Kip,Reks OS: Chrome OS What steps will reproduce the problem? (1) In guest user>> Open Chrome Canvas app and observe chrome crashes Actual: Chrome crashes while opening Chrome canvas app in guest user Expected: No such issue should be seen This is a Regression issue as same is working fine on 73.0.3644.0/11508.0.0 dev NOTE: As it's guest user, unable to get crash ids. Attaching screencast for reference..
,
Jan 4
Is anyone here a potential owner for this issue? This issue is currently blocking M73 Dev which is targeted for next week.
,
Jan 7
Is this still an issue in the latest version? It seems like a small edge case, can https://canvas.apps.chrome/ be used successfully?
,
Jan 7
+benwells would it be possible to prevent Canvas from installing in Guest Mode? It looks like users don't have the option to install PWAs at all right now. We also don't have any other web apps that rely on a google account pre-installed in Guest Mode (Chrome, Files, Settings, Get Help).
,
Jan 8
Yury, will your change prevent Canvas from being installed in guest mode?
,
Jan 8
no, default apps have no limits to be visible in guest mode. And canvas crashed my test device in guest mode. I think this is aligned with conversation "For web apps, it would be simpler to just have a field in each app's json defining what profiles it works on."
,
Jan 9
No we have an approach for this? Does this qualify as a RBD issue, if so, the release will be blocked this week.
,
Jan 9
CLs are in review. Let see how this goes. crrev.com/c/1401508 + crrev.com/i/778186
,
Jan 9
Yury is preventing this from being installed in guest mode but I'm not sure if that is a change we'll want to merge into 72. I'd like to also fix the crash and then figure out which is safer to merge, hopefully that is a pretty simple thing to fix.
,
Jan 9
Oh hang on, is this 73 only? In this case we can just live with the changes from Yury. Would be good to know why it crashes though....
,
Jan 9
That's kind of odd. The whole browser doesn't crash, so we're not hitting a null pointer exception or CHECK. The window just closes. So is there some piece of code somewhere that decides to close the app window if the profile doesn't have what it's looking for?
,
Jan 9
Canvas works fine in Guest mode in M72. I've now got an M73 build, and it is actually a full browser crash. I'm assuming then that this is a null pointer dereference, likely of some sort of tab helper or other thing we expect to be present when we launch a bookmark app in windowed mode.
,
Jan 9
Bisected down to https://chromium-review.googlesource.com/c/chromium/src/+/1356129. +beccahughes. I believe that CL introduced a null pointer dereference or something similar that crashes the whole browser when a web app is run in guest mode. Prior to that CL, guest mode could be use to run a default-installed web app without a crash. The crash is reproable on a Chrome OS device with a build that picks up the Chrome Canvas autoinstall when guest mode is started (you'll see Chrome Canvas appear in the launcher a few seconds after starting guest mode). We can probably find a fix in SYD, but if you can find it and fix it faster, that would be great. :) Also +cc loyso: we need to watch out for more potential regressions like this in guest profiles with the ongoing BMO work. Even though we block user-triggered installation, default installations can still happen and should (apparently) still work (I'm guessing it simply reinstalls at each guest session startup).
,
Jan 9
I have a CL out to fix: https://chromium-review.googlesource.com/c/chromium/src/+/1403089
,
Jan 10
It looks like there is a workaround #9 and I am not going to block dev for this, if anyone disagrees, please let me know. Otherwise I am moving forward with dev. Thanks.
,
Jan 11
,
Jan 14
,
Jan 15
Formally, extensions are disabled in guest mode on ChromeOS. But it could work by occasion. I think we should disable WebAppProvider for any guest/incognito profiles for now. In the future, extension-independent web apps may be enabled for guest sessions.
,
Jan 15
What does "formally, extensions are disabled" mean? Does it mean the whole system is disabled (unlikely if they still work), or something else?
,
Jan 15
It means that --disable-extensions flag is passed. But many subsystems (external web apps and pending bookmark app manager) don't check that. They use extensions anyway.
,
Jan 15
IIUC some extensions (at least default apps) might not follow this flag as well. I could see that extension based default apps are not in guest mode due this logic: https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_manager.cc?l=1296 I could find any disable-extensions handling there. Probably this flag can be used somewhere else to decline loading request for particular app but top logic is controlled at link above. Probably we might want to enable some default apps for guest modes.
,
Jan 16
Well, the Canvas bookmark app (extension-based) works in guest mode (but disabled by khmel@'s second CLs). Do I correctly understand that we want to preserve this ability (so functionality stays in the binary but controlled in external json)? We can do that. A couple of fixes needed and cleanups.
,
Jan 16
+devlin - can you explain what --disable-extensions is meant to do? E.g. is it meant to disable the entire extensions system, or just prevent user installation of extensions (but let policy installed ones still work, for example), or something else? As to what we should do, I think having the ability to have web apps pre-installed in guest mode is a good thing. The same is probably true for other app and extension types, but there might be privacy aspects to it. For now making it not crash would be a great thing for Becca to do and we can take it from there.
,
Jan 18
(5 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/21eed52bba818d93072db66b6cef839b89ae7739 commit 21eed52bba818d93072db66b6cef839b89ae7739 Author: Becca Hughes <beccahughes@chromium.org> Date: Fri Jan 18 01:39:10 2019 Fix regression in WebAppProviderFactory Adds support for WebAppProviderFactory to handle guest and incognito profiles. BUG=918786 Change-Id: I8338bce9b1291a25b98d16cc61f0162bf83140e2 Reviewed-on: https://chromium-review.googlesource.com/c/1403089 Reviewed-by: Alexey Baskakov <loyso@chromium.org> Commit-Queue: Becca Hughes <beccahughes@chromium.org> Cr-Commit-Position: refs/heads/master@{#623946} [modify] https://crrev.com/21eed52bba818d93072db66b6cef839b89ae7739/chrome/browser/web_applications/web_app_provider_factory.cc [modify] https://crrev.com/21eed52bba818d93072db66b6cef839b89ae7739/chrome/browser/web_applications/web_app_provider_factory.h |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by manucornet@chromium.org
, Jan 3