Multiple users reporting Data Saver not working on Lenovo Zuk devices |
|||||||||||
Issue descriptionFrom this thread, a number of users are reporting problems enabling Data Saver on Lenovo Zuk Z2 Pro (and possibly other Zuk model) phones: http://zukfans.eu/community/threads/chrome-data-saver.1441/ I asked a few of these users to file bug reports: https://feedback.corp.google.com/product/282/neutron?lView=rd&lRSort=1&lROrder=2&lRFilter=1&lReportSearch=Data%20Saver&lReport=16502746213 https://feedback.corp.google.com/product/282/neutron?lView=rd&lRSort=1&lROrder=2&lRFilter=1&lReportSearch=Data%20Saver&lReport=16485501641
,
Nov 17 2016
,
Nov 17 2016
,
Nov 17 2016
I wonder if implementing onCreateView() before onBindView() would help. An immediate fix may be just guard it by null checks.
,
Nov 17 2016
DataReductionStatsPreference.java is also NOT calling setContentView() which is suspicious.
,
Nov 17 2016
http://stackoverflow.com/a/8164879 says it is important to call setContentView(). Not sure how correct is that information.
,
Nov 17 2016
Some thoughts: The crash message is pretty clear that mOriginalSizeTextView is null: "Attempt to invoke virtual method 'void android.widget.TextView.setText(java.lang.CharSequence)' on a null object". This would happen if the view passed into onBindView() is the wrong view (say, the default preference view), or possibly if the view passed into onBindView() is null. Since this is device-specific, my guess is that Preference or PreferenceCategory or some related file has been modified on Zuk devices and somehow isn't respecting the android:widgetLayout attribute in data_reduction_preferences.xml, or is calling onBind() at the wrong time (or twice). Do we need to call setContentView? No... Preference views are handled via other means. The method Preference.setContentView() doesn't exist. Preference views are inflated by the preference framework and passed to YourPreferenceSubclass.onBindView(). The view for DataReductionStatsPreference is specified via the attribute android:widgetLayout="@layout/data_reduction_stats_layout" in data_reduction_preferences.xml. But for some reason, that view isn't being inflated and passed to onBindView(). Instead, I'd guess, the default preference view is being passed to onBindView(). Avenues for investigation (on a Zuk device): - Log information about the view that's being passed to onBindView(). Is it the view from data_reduction_stats_layout.xml, or the default preference view? - Is onBindView() called multiple times perhaps? Once with the default view, once with the correct view? (I can imagine a sloppy implementation doing this) - Do the three other preferences where we used android:widgetLayout work correctly? (usb_device_preferences.xml, single_website_preferences.xml, autofill_server_profile_preferences.xml) - Try this potential workaround: set the widget layout in code rather than in xml. i.e. call setWidgetLayoutResource() in the DataReductionStatsPreference constructor, and remove the android:widgetLayout attribute from data_reduction_preferences.xml.
,
Nov 17 2016
newt, thanks for the explanation. I tried repro'ing on Svelte device but could not. I will get hold of a Lenovo device to repro this.
,
Nov 18 2016
Status update: candrada@ reported that they could not repro the issue on any of the Lenovo devices that they have: Lenovo K3 Note (K50-T3S) / 6.0 Lenovo K3 Note (K50A40) / 5.0 Lenovo A6000 / 5.0 Lenovo Tab 2 A7 / 4.4.2 Lenovo A369i / JDQ39 I will get a Zuk device, and see if I can repro.
,
Nov 29 2016
I ordered the phone last week. It should arrive sometime this week or early next week.
,
Nov 30 2016
,
Dec 5 2016
I received the Zuk2 device, and the bug repro'es. Yay!
,
Dec 6 2016
Assigning to megjbalon based on offline discussion. Here is a CL that fixes the problem but there might be better ways out: https://codereview.chromium.org/2549243002/.
,
Dec 7 2016
,
Dec 7 2016
Apparently the bug didn't auto update yet: https://chromiumcodereview.appspot.com/2554723002/ Fix Data Saver settings preference crash on Lenovo Zuk devices The Data Saver settings menu is crashing for Lenovo Zuk devices due to findViewById returning null. Since we don't use the title in the PreferenceCategory, make the DataReductionStatsPreference a Preference and set the widget layout resource dynamically. Also, set the preference to not be selectable so that it does not have dividers and cannot be clicked. BUG= 666412 Committed: https://crrev.com/c8b5b01012a5b4245eddb891c2557e7c6e3e64b6 Cr-Commit-Position: refs/heads/master@{#436686}
,
Dec 7 2016
Thanks Megan for the quick fix! Do we understand why this would be broken on Zuk devices? Is this simply bad programming practice (assuming the resources will be there?)?
,
Dec 7 2016
onBindView wasn't getting called multiple times so I believe it's due to newt's first suggestion that we are getting the default preference view instead of the view from data_reduction_stats_layout. I was able to switch the class to a Preference and set the widget layout resource dynamically to fix it.
,
Dec 7 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Dec 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a4e3e0ff2a88903e6636bb3f782adb51060a1ebc commit a4e3e0ff2a88903e6636bb3f782adb51060a1ebc Author: Megan Jablonski <megjablon@google.com> Date: Wed Dec 07 23:44:16 2016 Fix Data Saver settings preference crash on Lenovo Zuk devices The Data Saver settings menu is crashing for Lenovo Zuk devices due to findViewById returning null. Since we don't use the title in the PreferenceCategory, make the DataReductionStatsPreference a Preference and set the widget layout resource dynamically. Also, set the preference to not be selectable so that it does not have dividers and cannot be clicked. BUG= 666412 Review-Url: https://codereview.chromium.org/2554723002 Cr-Commit-Position: refs/heads/master@{#436686} (cherry picked from commit c8b5b01012a5b4245eddb891c2557e7c6e3e64b6) Review URL: https://codereview.chromium.org/2552123007 . Cr-Commit-Position: refs/branch-heads/2924@{#395} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/a4e3e0ff2a88903e6636bb3f782adb51060a1ebc/chrome/android/java/res/xml/data_reduction_preferences.xml [modify] https://crrev.com/a4e3e0ff2a88903e6636bb3f782adb51060a1ebc/chrome/android/java/src/org/chromium/chrome/browser/preferences/datareduction/DataReductionStatsPreference.java
,
Dec 7 2016
|
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by mdw@chromium.org
, Nov 17 2016