New issue
Advanced search Search tips

Issue 715934 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Crash with Notification API

Reported by jm.acun...@gmail.com, Apr 27 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36

Steps to reproduce the problem:
Google Chrome Version 60.0.3081.6 (Build oficial) canary (64 bits)
ID 6481f1bb-7bf2-4130-bfcf-baca54f5b4d3 (Server: 03ba8c6e80000000)

Google Chrome Version 58.0.3029.81
ID 49685d41-81fe-458f-9d6f-c5704b5fdc79 (Sever: 67f6cda330000000)

1- go to http://createcharts.esy.es/notification-crash.html
2- click button Test
3- click button Allow
4- click right button on notification: Disable notifications of createcharts.esy.es
5- crash

(If in step 5 no crash occurs, repeat steps 3, 4)

What is the expected behavior?

What went wrong?
Crash

Crashed report ID: 

How much crashed? Just one tab

Is it a problem with a plugin? N/A 

Did this work before? N/A 

Chrome version: 58.0.3029.81  Channel: stable
OS Version: 6.3
Flash Version:
 
ice_video_20170427-112024.webm
1.2 MB View Download
I have verified that the crash occurs between request 250 and 300.
That is, we could limit the code in this way with the same result:

if (typeof Notification === 'function' && count <= 300) {
Labels: Needs-Triage-M58

Comment 3 by rbyers@chromium.org, Apr 27 2017

Components: UI>Notifications Blink>PushAPI
I can't seem to find any crash reports with those crash IDs.

This is the Notification API which I believe is separate from the push notification API, but there doesn't seem to be a component for the former and hopefully the same team owns both?

Comment 4 by rbyers@chromium.org, Apr 27 2017

Cc: rbyers@chromium.org
Reproduced on Chrome Linux 59.0.3063.4 
Crash ID a94eff3330000000 (hasn't processed yet so I can't grab the stack right now).

Comment 5 by rbyers@chromium.org, Apr 27 2017

Cc: peter@chromium.org
Status: Untriaged (was: Unconfirmed)
Relevant callstack:

content::BrowserMessageFilter::ShutdownForBadMessage()
...
content::NotificationMessageFilter::OnMessageReceived(IPC::Message const&)

I see a few hits for this crash back to at least Chrome 55, so doesn't appear to be a regression.

Comment 6 by peter@chromium.org, Jun 28 2017

Cc: -peter@chromium.org
Components: -Blink>PushAPI
Owner: peter@chromium.org
Status: Started (was: Untriaged)
This is an unfortunate race condition indeed -- we're probably better off just dropping such calls on the floor rather than marking the renderer as compromised.

Uploaded a CL doing just that:
https://chromium-review.googlesource.com/552553
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 28 2017

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

commit 701df4bee16051b0560daecb23eaaa32dfb1ba3c
Author: Peter Beverloo <peter@chromium.org>
Date: Wed Jun 28 19:13:10 2017

Don't tag a renderer as bad when there's no notification permission

There's a race where the user might disable notification permission
while some of the notification API functions are still executing, for
example because they were still fetching resources. We shouldn't crash
the renderer in those cases, but just silently drop it.

BUG= 715934 

Change-Id: I6c4c02cb757db22999b9fbac3d716876fee46d07
Reviewed-on: https://chromium-review.googlesource.com/552553
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#483094}
[modify] https://crrev.com/701df4bee16051b0560daecb23eaaa32dfb1ba3c/content/browser/bad_message.h
[modify] https://crrev.com/701df4bee16051b0560daecb23eaaa32dfb1ba3c/content/browser/notifications/notification_message_filter.cc
[modify] https://crrev.com/701df4bee16051b0560daecb23eaaa32dfb1ba3c/content/browser/notifications/notification_message_filter.h

Comment 8 by peter@chromium.org, Jun 28 2017

Status: Fixed (was: Started)

Sign in to add a comment