Extension SW - add basic push messaging tests.

Extension SW - add basic push messaging tests.

This test checks that self.onpush() works correctly from an
  extension ServiceWorker when a gcm message is sent.

Caveat:
PushMessagingDispatcher::DoSubscribe() only looks at the web
  manifest, so we list the manifest in the extension page
  (page.html). However, this also means we cannot use
  PushMessaging from a service worker of a background page.

BUG=545535
Committed: https://crrev.com/561b7deff9073356ef8e4ed7238b5c7962f022dd
Cr-Commit-Position: refs/heads/master@{#360640}

[lazyboy, 2015-11-13 01:31:56]

Description was changed from

==========
[WIP] Extension SW - push messaging tests.

This is not ready for review yet.
I'm trying to figure out why I needed to list link rel="manifest"
  in the extension page's html.

BUG=
==========

to

==========
Extension SW - basic push messaging tests.

This test checks that self.onpush() works correctly from an
  extension ServiceWorker when a gcm message is sent.

Caveat:
PushMessagingDispatcher::DoSubscribe() only looks at the web
  manifest, so we list the manifest in the extension page
  (page.html). However, this also means we cannot use
  PushMessaging from a service worker of a background page.

BUG=545535
==========

[lazyboy, 2015-11-13 01:32:21]

Description was changed from

==========
Extension SW - basic push messaging tests.

This test checks that self.onpush() works correctly from an
  extension ServiceWorker when a gcm message is sent.

Caveat:
PushMessagingDispatcher::DoSubscribe() only looks at the web
  manifest, so we list the manifest in the extension page
  (page.html). However, this also means we cannot use
  PushMessaging from a service worker of a background page.

BUG=545535
==========

to

==========
Extension SW - add basic push messaging tests.

This test checks that self.onpush() works correctly from an
  extension ServiceWorker when a gcm message is sent.

Caveat:
PushMessagingDispatcher::DoSubscribe() only looks at the web
  manifest, so we list the manifest in the extension page
  (page.html). However, this also means we cannot use
  PushMessaging from a service worker of a background page.

BUG=545535
==========

[lazyboy, 2015-11-13 01:34:19]

lazyboy@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[lazyboy, 2015-11-18 23:28:22]

Ping.

[Devlin, 2015-11-18 23:42:06]

Sorry for the delay.

The integration between these two is pretty rough, but I'm okay with it for now.
 Ideally it'd be simpler - but with the gcm api, I think we're probably alright.
 Might be worth adding a TODO or filing a bug that it could be better, though.

lgtm.

https://chromiumcodereview.appspot.com/1440873004/diff/80001/chrome/test/data...
File
chrome/test/data/extensions/api_test/service_worker/push_messaging/page.html
(right):

https://chromiumcodereview.appspot.com/1440873004/diff/80001/chrome/test/data...
chrome/test/data/extensions/api_test/service_worker/push_messaging/page.html:3:
However, this also means we cannot use PushMessaging from a service
Hmm... not strictly true, right, as long as the background page had an
associated html page (and not a generated_background_page)?

https://chromiumcodereview.appspot.com/1440873004/diff/80001/chrome/test/data...
File chrome/test/data/extensions/api_test/service_worker/push_messaging/sw.js
(right):

https://chromiumcodereview.appspot.com/1440873004/diff/80001/chrome/test/data...
chrome/test/data/extensions/api_test/service_worker/push_messaging/sw.js:8:
this.onpush = function(e) {
nit: I think this can be a little cleaner:

var port;
var pushData;

function maybeFinish() {
  if (port && pushData)
    port.postMessage(pushData);
}

this.onpush = function(e) {
  pushData = e.data.text();
  maybeFinish();
};

this.onmessage = function(e) {
  ...
  port = e.ports[0];
  maybeFinish();
}

[lazyboy, 2015-11-19 02:40:28]

Filed crbug.com/558173

https://chromiumcodereview.appspot.com/1440873004/diff/80001/chrome/test/data...
File
chrome/test/data/extensions/api_test/service_worker/push_messaging/page.html
(right):

https://chromiumcodereview.appspot.com/1440873004/diff/80001/chrome/test/data...
chrome/test/data/extensions/api_test/service_worker/push_messaging/page.html:3:
However, this also means we cannot use PushMessaging from a service
On 2015/11/18 23:42:06, Devlin wrote:
> Hmm... not strictly true, right, as long as the background page had an
> associated html page (and not a generated_background_page)?

That's right, updated the comment.

https://chromiumcodereview.appspot.com/1440873004/diff/80001/chrome/test/data...
File chrome/test/data/extensions/api_test/service_worker/push_messaging/sw.js
(right):

https://chromiumcodereview.appspot.com/1440873004/diff/80001/chrome/test/data...
chrome/test/data/extensions/api_test/service_worker/push_messaging/sw.js:8:
this.onpush = function(e) {
On 2015/11/18 23:42:06, Devlin wrote:
> nit: I think this can be a little cleaner:
> 
> var port;
> var pushData;
> 
> function maybeFinish() {
>   if (port && pushData)
>     port.postMessage(pushData);
> }
> 
> this.onpush = function(e) {
>   pushData = e.data.text();
>   maybeFinish();
> };
> 
> this.onmessage = function(e) {
>   ...
>   port = e.ports[0];
>   maybeFinish();
> }

Thanks. Done.

[lazyboy, 2015-11-19 07:11:24]

The CQ bit was checked by lazyboy@chromium.org

[lazyboy, 2015-11-19 07:11:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/1440873004/#ps100001 (title: "update")

[commit-bot: I haz the power, 2015-11-19 07:11:40]

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

[commit-bot: I haz the power, 2015-11-19 07:16:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-19 07:16:17]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[lazyboy, 2015-11-19 19:18:29]

The CQ bit was checked by lazyboy@chromium.org

[lazyboy, 2015-11-19 19:18:29]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/1440873004/#ps120001 (title: "sync @
tott")

[commit-bot: I haz the power, 2015-11-19 19:20:16]

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

[commit-bot: I haz the power, 2015-11-19 19:27:38]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-11-19 19:28:32]

Patchset 7 (id:??) landed as
https://crrev.com/561b7deff9073356ef8e4ed7238b5c7962f022dd
Cr-Commit-Position: refs/heads/master@{#360640}