New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 666412 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit 15 days ago
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: ----



Sign in to add a comment

Multiple users reporting Data Saver not working on Lenovo Zuk devices

Project Member Reported by mdw@chromium.org, Nov 17 2016

Issue description

Comment 1 by mdw@chromium.org, Nov 17 2016

In 16502746213, I see this in event_log.gz:

11-17 19:27:00.368 I/am_activity_launch_time( 1333): [0,43717074,com.android.chrome/org.chromium.chrome.browser.preferences.Preferences,181,181]
11-17 19:27:00.404 I/sf_frame_dur(  604): [com.android.chrome/org.chromium.chrome.browser.preferences.Preferences,18,2,1,1,2,0,1]
11-17 19:27:01.987 I/am_pss  ( 1333): [12068,10154,com.android.chrome,68027392,24481792]
11-17 19:27:02.020 I/am_crash( 1333): [12068,0,com.android.chrome,954973764,java.lang.NullPointerException,Attempt to invoke virtual method 'void android.widget.TextView.setText(java.lang.CharSequence)' on a null object reference,DataReductionStatsPreference.java,103]
11-17 19:27:02.027 I/am_finish_activity( 1333): [0,43717074,255,com.android.chrome/org.chromium.chrome.browser.preferences.Preferences,force-crash]

In 16485501641, I see this in event_log.gz:

11-17 16:22:18.620 I/am_crash( 1306): [18338,0,com.android.chrome,952876612,java.lang.NullPointerException,Attempt to invoke virtual method 'void android.widget.TextView.setText(java.lang.CharSequence)' on a null object reference,DataReductionStatsPreference.java,103]


Comment 2 by bengr@chromium.org, Nov 17 2016

Cc: -tbansal@chromium.org bengr@chromium.org
Labels: -Pri-2 M-56 Pri-1
Owner: tbansal@chromium.org

Comment 3 by bengr@chromium.org, Nov 17 2016

Cc: n...@chromium.org
I wonder if implementing onCreateView() before onBindView() would help. An immediate fix may be just guard it by null checks.
DataReductionStatsPreference.java is also NOT calling setContentView() which is suspicious.
http://stackoverflow.com/a/8164879 says it is important to call setContentView(). Not sure how correct is that information.

Comment 7 by n...@chromium.org, 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.

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.
Labels: Restrict-View-Google
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.
I ordered the phone last week. It should arrive sometime this week or early next week.

Comment 11 by mdw@chromium.org, Nov 30 2016

Labels: -Restrict-View-Google
Cc: megjab...@chromium.org
Status: Started (was: Assigned)
I received the Zuk2 device, and the bug repro'es. Yay!
Cc: -megjab...@chromium.org tbansal@chromium.org
Owner: megjab...@chromium.org
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/.
Labels: Merge-Request-56
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}

Comment 16 by mdw@chromium.org, 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?)?

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.

Comment 18 by dimu@chromium.org, Dec 7 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 19 by bugdroid1@chromium.org, Dec 7 2016

Labels: -merge-approved-56 merge-merged-2924
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

Status: Fixed (was: Started)

Sign in to add a comment