New issue
Advanced search Search tips

Issue 654705 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: ----



Sign in to add a comment

NPE in CustomTabsService#warmup()

Project Member Reported by lizeb@chromium.org, Oct 11 2016

Issue description

NullPointerException is thrown on some devices from warmup(). This comes from internal crash reports from AGSA (bug 31861380 in the internal bugtracker).

Unfortunately, since the exception is re-thrown by Parcel.java on the GSA side, the Chrome stack trace is not available in the GSA bug reports.

 

Comment 1 by lizeb@chromium.org, Oct 11 2016

From the internal bug:

I didn't find matching Chrome crash reports, but from the initial investigation:
- All the crash reports I looked at where from Android 5.0.x, API Level 21, no matter the phone model.
- There is very little code executed synchronously in warmup(), narrowing down the possibilities

Basically, what warmup() does synchronously is:
1. Check whether the first run experience should be ran. This involves a special check for the intent creator for GSA
2. Check whether the calling app is foreground
3. Post a task to the main thread

(3) cannot be the issue, since posting a task to the main looper is always OK, and the code executing in the task is asynchronous.
(2) has two different code paths, depending on whether Chrome is running on a system with API level <= 21 or not. However this code hasn't changed since M45.
(1) is somewhat new, at least part of it.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 11 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f974fe4e5a32803b83592a014a82db683cb485f7

commit f974fe4e5a32803b83592a014a82db683cb485f7
Author: lizeb <lizeb@chromium.org>
Date: Tue Oct 11 11:56:33 2016

customtabs: Add paranoia to isCallerForegroundOrSelf() due to NPEs in the wild.

We have a number of NPEs reported from devices in the
wild. Unfortunately, we only know that they happen on L pre-MR1, and
from warmup(). This commits adds null checks to prevent these from
crashing the client (reported by GSA).

BUG= 654705 

Review-Url: https://codereview.chromium.org/2404183004
Cr-Commit-Position: refs/heads/master@{#424406}

[modify] https://crrev.com/f974fe4e5a32803b83592a014a82db683cb485f7/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java

Comment 3 by lizeb@chromium.org, Oct 12 2016

Labels: Merge-Request-55
Requesting a merge to M55. Rationale: crash visible to users.

Comment 4 by dimu@chromium.org, Oct 12 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 12 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d5357e02cda8ed82b7788b76e036937a329460f

commit 1d5357e02cda8ed82b7788b76e036937a329460f
Author: Benoit Lize <lizeb@chromium.org>
Date: Wed Oct 12 14:05:15 2016

customtabs: Add paranoia to isCallerForegroundOrSelf() due to NPEs in the wild.

We have a number of NPEs reported from devices in the
wild. Unfortunately, we only know that they happen on L pre-MR1, and
from warmup(). This commits adds null checks to prevent these from
crashing the client (reported by GSA).

BUG= 654705 

Review-Url: https://codereview.chromium.org/2404183004
Cr-Commit-Position: refs/heads/master@{#424406}
(cherry picked from commit f974fe4e5a32803b83592a014a82db683cb485f7)

Review URL: https://codereview.chromium.org/2415573002 .

Cr-Commit-Position: refs/branch-heads/2883@{#59}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/1d5357e02cda8ed82b7788b76e036937a329460f/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java

Comment 6 by lizeb@chromium.org, Oct 17 2016

Status: Fixed (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1d5357e02cda8ed82b7788b76e036937a329460f

commit 1d5357e02cda8ed82b7788b76e036937a329460f
Author: Benoit Lize <lizeb@chromium.org>
Date: Wed Oct 12 14:05:15 2016

customtabs: Add paranoia to isCallerForegroundOrSelf() due to NPEs in the wild.

We have a number of NPEs reported from devices in the
wild. Unfortunately, we only know that they happen on L pre-MR1, and
from warmup(). This commits adds null checks to prevent these from
crashing the client (reported by GSA).

BUG= 654705 

Review-Url: https://codereview.chromium.org/2404183004
Cr-Commit-Position: refs/heads/master@{#424406}
(cherry picked from commit f974fe4e5a32803b83592a014a82db683cb485f7)

Review URL: https://codereview.chromium.org/2415573002 .

Cr-Commit-Position: refs/branch-heads/2883@{#59}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/1d5357e02cda8ed82b7788b76e036937a329460f/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java

Comment 8 by dimu@google.com, Nov 4 2016

[Automated comment] removing mislabelled merge-merged-2840

Comment 9 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment