[Android] SiteSettings crash with NPE when both autoplay and protected content are enabled |
|||||||
Issue descriptionEnable autoplay setting via FieldTrials or some other way (CL to enable via --enable-autoplay-muted-video is coming). 1. Go to Chrome settings > Site settings OR: NPE at https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SiteSettingsPreferences.java?rcl=0&l=205 ER: No NPE. The code removes both preferences from the view in this case but still tries to find the AUTOPLAY one in the Site Settings view. Then findPreference() returns null.
,
Jun 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf6c1f850a7b75b5992d3f99d7a8b3d8f0488b24 commit bf6c1f850a7b75b5992d3f99d7a8b3d8f0488b24 Author: avayvod <avayvod@chromium.org> Date: Thu Jun 09 16:35:34 2016 [Android, Settings] Fix NPE when both media related site settings are enabled Only look for AUTOPLAY preference if it's the only one enabled. BUG= 618439 TEST=manual Review-Url: https://codereview.chromium.org/2052613002 Cr-Commit-Position: refs/heads/master@{#398908} [modify] https://crrev.com/bf6c1f850a7b75b5992d3f99d7a8b3d8f0488b24/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SiteSettingsPreferences.java
,
Jun 9 2016
,
Jun 9 2016
Finnur, do we need to merge the fix to M52?
,
Jun 9 2016
Nope. To repro you'd need a really old device (that doesn't support Protected Content) with a special flag enabled (autoplay) that we control and haven't enabled yet.
,
Jun 9 2016
But the crash reproduces when both settings are available, so it should be a not really old device with the autoplay experiment. Enabling autoplay on 52 and before would cause Settings to crash.
,
Jun 9 2016
Well, then it is up to the fate of the flag. Will we enable it on 52? If so, you might need to merge. Mounir is better equipped at answering that.
,
Jun 10 2016
Per offline discussion with Mounir, will merge to M52 just in case.
,
Jun 10 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jun 10 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/06afa04922c0f13eca0576d3fa2cce3758307ee5 commit 06afa04922c0f13eca0576d3fa2cce3758307ee5 Author: Anton Vayvod <avayvod@google.com> Date: Fri Jun 10 12:11:42 2016 [Android, Settings] Fix NPE when both media related site settings are enabled Only look for AUTOPLAY preference if it's the only one enabled. BUG= 618439 TEST=manual Review-Url: https://codereview.chromium.org/2052613002 Cr-Commit-Position: refs/heads/master@{#398908} (cherry picked from commit bf6c1f850a7b75b5992d3f99d7a8b3d8f0488b24) Review URL: https://codereview.chromium.org/2057143002 . Cr-Commit-Position: refs/branch-heads/2743@{#314} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/06afa04922c0f13eca0576d3fa2cce3758307ee5/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SiteSettingsPreferences.java
,
Jun 10 2016
,
Jun 14 2016
verified in M53-53.0.2767.3 and not see any crash
,
Jun 14 2016
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf6c1f850a7b75b5992d3f99d7a8b3d8f0488b24 commit bf6c1f850a7b75b5992d3f99d7a8b3d8f0488b24 Author: avayvod <avayvod@chromium.org> Date: Thu Jun 09 16:35:34 2016 [Android, Settings] Fix NPE when both media related site settings are enabled Only look for AUTOPLAY preference if it's the only one enabled. BUG= 618439 TEST=manual Review-Url: https://codereview.chromium.org/2052613002 Cr-Commit-Position: refs/heads/master@{#398908} [modify] https://crrev.com/bf6c1f850a7b75b5992d3f99d7a8b3d8f0488b24/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SiteSettingsPreferences.java
,
Jun 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/06afa04922c0f13eca0576d3fa2cce3758307ee5 commit 06afa04922c0f13eca0576d3fa2cce3758307ee5 Author: Anton Vayvod <avayvod@google.com> Date: Fri Jun 10 12:11:42 2016 [Android, Settings] Fix NPE when both media related site settings are enabled Only look for AUTOPLAY preference if it's the only one enabled. BUG= 618439 TEST=manual Review-Url: https://codereview.chromium.org/2052613002 Cr-Commit-Position: refs/heads/master@{#398908} (cherry picked from commit bf6c1f850a7b75b5992d3f99d7a8b3d8f0488b24) Review URL: https://codereview.chromium.org/2057143002 . Cr-Commit-Position: refs/branch-heads/2743@{#314} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/06afa04922c0f13eca0576d3fa2cce3758307ee5/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SiteSettingsPreferences.java |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by avayvod@chromium.org
, Jun 8 2016