PushEvent.data getter should not force-create a data object |
||||||
Issue descriptionEven when a NULL payload has been passed and PushMessageData::create() returns a nullptr, the `data` getter of the PushEvent will force-create a PushMessageData object as we used to initialize it lazily. This means that Chrome's returning a PushMessageData object for NULL payloads after all. We shouldn't.
,
Mar 9 2016
Got it. I'll have a look at your library too, thanks!
In the mean time, the fix is about to land per the following CL:
https://codereview.chromium.org/1784583002/
,
Mar 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7 commit 72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7 Author: peter <peter@chromium.org> Date: Wed Mar 09 23:33:01 2016 PushEvent.data should be NULL for empty payloads There was code in the PushEvent that would create an empty data object even when no data was available. Remove it, and add an end-to-end test. BUG= 593454 Review URL: https://codereview.chromium.org/1784583002 Cr-Commit-Position: refs/heads/master@{#380258} [modify] https://crrev.com/72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7/chrome/browser/push_messaging/push_messaging_browsertest.cc [modify] https://crrev.com/72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7/chrome/test/data/push_messaging/service_worker.js [modify] https://crrev.com/72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7/third_party/WebKit/LayoutTests/http/tests/push_messaging/resources/pushmessagedata-worker.js [modify] https://crrev.com/72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7/third_party/WebKit/Source/modules/push_messaging/PushEvent.cpp [modify] https://crrev.com/72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7/third_party/WebKit/Source/modules/push_messaging/PushMessageData.cpp [modify] https://crrev.com/72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7/third_party/WebKit/Source/modules/push_messaging/PushMessageData.h
,
Mar 9 2016
Requesting merge. Presuming this missed the first beta, I'd be happy to let it simmer for a few days.
,
Mar 10 2016
Your change meets the bar and is auto-approved for M50 (branch: 2661)
,
Mar 10 2016
Please merge your change to M50 branch 2661 once you think it has enough baking time. If merge happens by Monday, we can take it in for next week beta. Thank you.
,
Mar 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0ca7be84ec95b002d722b1c46acdc9d0e76242b1 commit 0ca7be84ec95b002d722b1c46acdc9d0e76242b1 Author: Peter Beverloo <peter@chromium.org> Date: Fri Mar 11 17:51:00 2016 PushEvent.data should be NULL for empty payloads There was code in the PushEvent that would create an empty data object even when no data was available. Remove it, and add an end-to-end test. BUG= 593454 Review URL: https://codereview.chromium.org/1784583002 Cr-Commit-Position: refs/heads/master@{#380258} (cherry picked from commit 72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7) Review URL: https://codereview.chromium.org/1786733003 . Cr-Commit-Position: refs/branch-heads/2661@{#195} Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081} [modify] https://crrev.com/0ca7be84ec95b002d722b1c46acdc9d0e76242b1/chrome/browser/push_messaging/push_messaging_browsertest.cc [modify] https://crrev.com/0ca7be84ec95b002d722b1c46acdc9d0e76242b1/chrome/test/data/push_messaging/service_worker.js [modify] https://crrev.com/0ca7be84ec95b002d722b1c46acdc9d0e76242b1/third_party/WebKit/LayoutTests/http/tests/push_messaging/resources/pushmessagedata-worker.js [modify] https://crrev.com/0ca7be84ec95b002d722b1c46acdc9d0e76242b1/third_party/WebKit/Source/modules/push_messaging/PushEvent.cpp [modify] https://crrev.com/0ca7be84ec95b002d722b1c46acdc9d0e76242b1/third_party/WebKit/Source/modules/push_messaging/PushMessageData.cpp [modify] https://crrev.com/0ca7be84ec95b002d722b1c46acdc9d0e76242b1/third_party/WebKit/Source/modules/push_messaging/PushMessageData.h
,
Mar 11 2016
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by mcastell...@mozilla.com
, Mar 9 2016