PushEvent.data should be NULL for empty payloads

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
Committed: https://crrev.com/72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7
Cr-Commit-Position: refs/heads/master@{#380258}

[Peter Beverloo, 2016-03-09 21:01:10]

Patchset #1 (id:1) has been deleted

[Peter Beverloo, 2016-03-09 21:12:23]

peter@chromium.org changed reviewers:
+ dgn@chromium.org, harkness@chromium.org

[Peter Beverloo, 2016-03-09 21:13:25]

Description was changed from

==========
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=578036
==========

to

==========
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
==========

[dgn, 2016-03-09 21:22:26]

https://codereview.chromium.org/1784583002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/push_messaging/resources/pushmessagedata-worker.js
(right):

https://codereview.chromium.org/1784583002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/push_messaging/resources/pushmessagedata-worker.js:32:
}, 'PushMessageData is empty by default.');
null instead of empty?

https://codereview.chromium.org/1784583002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/push_messaging/PushEvent.cpp (right):

https://codereview.chromium.org/1784583002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/push_messaging/PushEvent.cpp:12: :
m_data(PushMessageData::create())
Isn't that always creating empty data? shouldn't it be create with a null
string? or leave it null? 
The description does not match currently.

[Peter Beverloo, 2016-03-09 21:28:29]

https://codereview.chromium.org/1784583002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/push_messaging/resources/pushmessagedata-worker.js
(right):

https://codereview.chromium.org/1784583002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/push_messaging/resources/pushmessagedata-worker.js:32:
}, 'PushMessageData is empty by default.');
On 2016/03/09 21:22:25, dgn wrote:
> null instead of empty?

Done.

https://codereview.chromium.org/1784583002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/push_messaging/PushEvent.cpp (right):

https://codereview.chromium.org/1784583002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/push_messaging/PushEvent.cpp:12: :
m_data(PushMessageData::create())
On 2016/03/09 21:22:25, dgn wrote:
> Isn't that always creating empty data? shouldn't it be create with a null
> string? or leave it null? 
> The description does not match currently.

There's one call-site that can return a nullptr immediately indeed, so I removed
this.

[dgn, 2016-03-09 21:30:02]

lgtm

[harkness, 2016-03-09 21:31:22]

On 2016/03/09 21:30:02, dgn wrote:
> lgtm

lgtm

[Peter Beverloo, 2016-03-09 21:31:41]

The CQ bit was checked by peter@chromium.org

[commit-bot: I haz the power, 2016-03-09 21:32:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1784583002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1784583002/40001

[commit-bot: I haz the power, 2016-03-09 23:33:09]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-03-09 23:33:10]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-03-09 23:34:07]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7
Cr-Commit-Position: refs/heads/master@{#380258}
==========

[commit-bot: I haz the power, 2016-03-09 23:34:09]

Patchset 2 (id:??) landed as
https://crrev.com/72d7385fd0dfbd77ee41dd53cb0ef7ac16d118d7
Cr-Commit-Position: refs/heads/master@{#380258}