Issue metadata
Sign in to add a comment
|
chrome.notifications.create causes notification blocking in Chrome 66+
Reported by
woshenme...@blackglory.me,
Apr 26 2018
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.117 Safari/537.36 Steps to reproduce the problem: 1. Create a Chrome extension with Notification permission (the attachment). 2. Invoke the API to create notifications that exceed the maximum number of displays at one time, such as 10. 3. You will notice that first batch pop-up notifications are complete (3 in my case), the subsequent notifications will not be displayed until the create method is called again, blocking will affect all extensions. What is the expected behavior? What went wrong? Chrome should continue to pop up notifications. Please refer to Chrome 65 and previous versions of the same code run results. I think maybe Chrome's notification bus has changed on Chrome 66, which has caused this problem. Did this work before? Yes Chrome 65 Does this work in other browsers? Yes Chrome version: 66.0.3359.117 Channel: stable OS Version: 10.0 Flash Version: This problem has been verified by me on Chrome Stable and Beta 66.0.3359.117, Canary 68.0.3406.0, both 32bit and 64bit versions, Windows and Linux platforms.
,
Apr 27 2018
,
Apr 27 2018
woshenmedoubuzhidao@ Thanks for the issue. Able to reproduce this issue on Windows 10, Mac OS 10.12.6 and Ubuntu 14.04 on the latest Canary 68.0.3409.0 and latest Stable 66.0.3359.117 as per the original comment. Bisect Information: =================== Good Build: 66.0.3342.0 (Revision - 534887) Bad Build : 66.0.3343.0 (Revision - 535276) On executing the per-revision bisect script, below is the Changelog URL: https://chromium.googlesource.com/chromium/src/+log/48836201498b4e560c1687dc5b33bd7c0880f588..dfa3260ac4018426e5f6b63911d17e500e7c6380 From the above Changelog, suspecting the below change: Reviewed-on: https://chromium-review.googlesource.com/857814 dpapad@ Please check and confirm if this issue is related to your change, else help us in assigning to the right owner. Adding ReleaseBlock-Stable for M-66 as this is a recent regression. Please feel to remove if this is not applicable. Thanks.
,
Apr 27 2018
,
Apr 27 2018
,
Apr 27 2018
Over to peter@ for notifications API.
,
Apr 27 2018
,
Apr 27 2018
,
May 2 2018
*** Bulk Edit *** M67 Stable promotion is coming soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. If fix is already merged to M67 and nothing else is pending, pls mark the bug as fixed. Thank you.
,
May 7 2018
*** Bulk Edit *** M67 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. If fix is already merged to M67 and nothing else is pending, pls mark the bug as fixed. Thank you.
,
May 8 2018
+tetsui, yoshiki This sounds like it might be related to the queuing issues we're seeing on Chrome OS. WDYT?
,
May 9 2018
Also the bisect in #3 is wrong. Both 48836201498b4e560c1687dc5b33bd7c0880f588 and dfa3260ac4018426e5f6b63911d17e500e7c638 are good. I'll bisect again.
,
May 9 2018
Sorry I wrote something confusing in #12. Please discard. https://crrev.com/c/967094 (https://crbug.com/814495#c40) seems to be causing this regression... It reproduces on Linux with --disable-features=NativeNotifications, didn't reproduce on Chrome OS.
,
May 9 2018
,
May 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/62ea842e987a1885b5115d137dd3f07f975007da commit 62ea842e987a1885b5115d137dd3f07f975007da Author: Tetsui Ohkubo <tetsui@chromium.org> Date: Thu May 10 01:11:31 2018 Fix desktop views notification blocking. When notifications more than 3 are created on desktop notification at once, they should be shown in the order after timeouts. https://crrev.com/c/967094 worked fine on Chrome OS, but didnt' work on desktop chrome, because MessageCenterImpl::MarkSinglePopupAsShown takes different codepaths by ifdef-ing OS_CHROMEOS. To fix this, we have to always call DoUpdate in OnNotificationRemoved. TEST=chrome --disable-features=NativeNotifications with extension attached in #0 of https://crbug.com/837119 BUG= 837119 Change-Id: Ie287045fe702736fcc449a6df1b3f7ed2b077abf Reviewed-on: https://chromium-review.googlesource.com/1049989 Reviewed-by: Evan Stade <estade@chromium.org> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> Cr-Commit-Position: refs/heads/master@{#557404} [modify] https://crrev.com/62ea842e987a1885b5115d137dd3f07f975007da/ui/message_center/views/message_popup_collection.cc
,
May 10 2018
,
May 10 2018
,
May 10 2018
,
May 10 2018
Pls update the bug with canary result tomorrow. As this is regressed in M66 which is already in stable, how critical and safe it is to merge to M67?
,
May 10 2018
> Pls update the bug with canary result tomorrow. Ack > As this is regressed in M66 which is already in stable, how critical and safe it is to merge to M67? It affects all the platforms including Win 10 right now, and users can miss notifications in some cases, so very critical. The change is small and trivial, so it's safe to merge to M67.
,
May 10 2018
Also forgot to mention, not only extensions, web notifications will also be affected by this.
,
May 10 2018
Ok, thank you. Pls update the bug with canary result tomorrow.
,
May 10 2018
Able to reproduce this issue on Windows 10, Mac OS 10.12.6 and Ubuntu 14.04 on the reported version 66.0.3359.117 and the issue is fixed on the latest Canary 68.0.3426.0 as per the original comment. On adding the extension given in the original comment, can observe that notifications are popping up. Attached is the screen cast for reference. Hence adding TE verified labels as the fix is working as intended. Thanks..
,
May 10 2018
The NextAction date has arrived: 2018-05-10
,
May 10 2018
Approving merge to M67 branch 3396 based on comment #22, #23 and #25. Please merge ASAP and mark the bug as fixed if nothing else is pending after the merge. Thank you.
,
May 10 2018
*** Bulk Edit *** M67 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. If fix is already merged to M67 and nothing else is pending, pls mark the bug as fixed. Thank you.
,
May 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/70aadd3ce45c6ada4105538a55eea153611ee38e commit 70aadd3ce45c6ada4105538a55eea153611ee38e Author: Tetsui Ohkubo <tetsui@chromium.org> Date: Fri May 11 01:25:36 2018 Fix desktop views notification blocking. When notifications more than 3 are created on desktop notification at once, they should be shown in the order after timeouts. https://crrev.com/c/967094 worked fine on Chrome OS, but didnt' work on desktop chrome, because MessageCenterImpl::MarkSinglePopupAsShown takes different codepaths by ifdef-ing OS_CHROMEOS. To fix this, we have to always call DoUpdate in OnNotificationRemoved. TEST=chrome --disable-features=NativeNotifications with extension attached in #0 of https://crbug.com/837119 BUG= 837119 TBR=estade@chromium.org Change-Id: Ie287045fe702736fcc449a6df1b3f7ed2b077abf Reviewed-on: https://chromium-review.googlesource.com/1049989 Reviewed-by: Evan Stade <estade@chromium.org> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#557404}(cherry picked from commit 62ea842e987a1885b5115d137dd3f07f975007da) Reviewed-on: https://chromium-review.googlesource.com/1053930 Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#563} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/70aadd3ce45c6ada4105538a55eea153611ee38e/ui/message_center/views/message_popup_collection.cc
,
May 11 2018
,
May 15 2018
Let's target this for M67 (already merged).
,
May 24 2018
,
May 25 2018
Issue 846245 has been merged into this issue. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by dtapu...@chromium.org
, Apr 26 2018