bluetooth: Separate notification tests

bluetooth: Separate notification tests

We separate the tests so that we don't have to manually stop notifications
after each test is done. Because notifications state is kept in the frame
and the frame cleans up the state after navigation we know the tests
won't interfere with each other.

BUG=554240
Committed: https://crrev.com/af5b0e45b29290c7d013e3c5e01a86fcfa9d50a4
Cr-Commit-Position: refs/heads/master@{#387734}

[ortuno, 2016-04-13 22:18:40]

The CQ bit was checked by ortuno@chromium.org to run a CQ dry run

[ortuno, 2016-04-13 22:18:51]

ortuno@chromium.org changed reviewers:
+ jyasskin@chromium.org

[ortuno, 2016-04-13 22:18:52]

jyasskin: PTAL

[commit-bot: I haz the power, 2016-04-13 22:19:06]

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

[ortuno, 2016-04-13 22:19:12]

Description was changed from

==========
bluetooth: Separate notification tests

We separate the tests so taht we don't have to manually stop notifications
after each test is done. Because notifications state is kept in the frame
and the frame cleans up the state after navigation we know the tests
won't interfere with each other.

BUG=554240
==========

to

==========
bluetooth: Separate notification tests

We separate the tests so that we don't have to manually stop notifications
after each test is done. Because notifications state is kept in the frame
and the frame cleans up the state after navigation we know the tests
won't interfere with each other.

BUG=554240
==========

[commit-bot: I haz the power, 2016-04-13 22:22:38]

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

[commit-bot: I haz the power, 2016-04-13 22:22:39]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ortuno, 2016-04-13 22:35:49]

The CQ bit was checked by ortuno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-13 22:36:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1858103004/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1858103004/60001

[commit-bot: I haz the power, 2016-04-13 22:38:59]

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

[commit-bot: I haz the power, 2016-04-13 22:39:00]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)

[Jeffrey Yasskin, 2016-04-15 19:56:46]

Other than the comments and the red tests, LGTM.

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/notifications/event-after-starting.html
(right):

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/notifications/event-after-starting.html:17:
'after notifications are stopped.');
I don't see the assertions for "no events after notifications are stopped."

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/notifications/gc-with-pending-start.html
(right):

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/notifications/gc-with-pending-start.html:17:
// We itentionally don't return the promise so that 'characteristic' goes
sp: itentionally

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/notifications/gc-with-pending-start.html:22:
// Assert that notifications are not active.
This may also depend on https://crbug.com/495270.

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/notifications/gc-with-pending-stop.html
(right):

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/notifications/gc-with-pending-stop.html:18:
// We itentionally don't return the promise so that 'characteristic' goes
sp: itentionally

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/notifications/start-stop-start-stop.html
(right):

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/notifications/start-stop-start-stop.html:18:
.then(() => characteristic.stopNotifications());
Was there a good reason to run garbage collection before? Were we checking for a
crash or something? Or does GC automatically run at the end of tests, and
splitting this to its own file gets it automatically?

No comment needed; I'm just curious if the gc should come back.

[ortuno, 2016-04-15 20:59:06]

Thanks! I'm getting a patch error so I think I have to wait until the other
patch lands.

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/notifications/event-after-starting.html
(right):

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/notifications/event-after-starting.html:17:
'after notifications are stopped.');
On 2016/04/15 at 19:56:45, Jeffrey Yasskin wrote:
> I don't see the assertions for "no events after notifications are stopped."

Oops, got to excited about deleting code. Done.

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/notifications/gc-with-pending-start.html
(right):

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/notifications/gc-with-pending-start.html:17:
// We itentionally don't return the promise so that 'characteristic' goes
On 2016/04/15 at 19:56:45, Jeffrey Yasskin wrote:
> sp: itentionally

Done.

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/notifications/gc-with-pending-start.html:22:
// Assert that notifications are not active.
On 2016/04/15 at 19:56:45, Jeffrey Yasskin wrote:
> This may also depend on https://crbug.com/495270.

I guess we won't need the test once objects are not garbage collected. Added
comment.

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/notifications/gc-with-pending-stop.html
(right):

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/notifications/gc-with-pending-stop.html:18:
// We itentionally don't return the promise so that 'characteristic' goes
On 2016/04/15 at 19:56:46, Jeffrey Yasskin wrote:
> sp: itentionally

Done.

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/notifications/start-stop-start-stop.html
(right):

https://codereview.chromium.org/1858103004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/notifications/start-stop-start-stop.html:18:
.then(() => characteristic.stopNotifications());
On 2016/04/15 at 19:56:46, Jeffrey Yasskin wrote:
> Was there a good reason to run garbage collection before? Were we checking for
a crash or something? Or does GC automatically run at the end of tests, and
splitting this to its own file gets it automatically?
> 
> No comment needed; I'm just curious if the gc should come back.

After a test finishes the document is detached which unsubscribes all the
characteristics.

[ortuno, 2016-04-15 20:59:12]

The CQ bit was checked by ortuno@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-15 20:59:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1858103004/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1858103004/80001

[commit-bot: I haz the power, 2016-04-15 21:02:50]

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

[commit-bot: I haz the power, 2016-04-15 21:02:51]

Dry run: Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[ortuno, 2016-04-15 21:33:15]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2016-04-15 21:33:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
jyasskin@chromium.org
Link to the patchset: https://codereview.chromium.org/1858103004/#ps100001
(title: "Merge")

[commit-bot: I haz the power, 2016-04-15 21:33:39]

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

[commit-bot: I haz the power, 2016-04-15 22:48:07]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-04-15 22:49:44]

Description was changed from

==========
bluetooth: Separate notification tests

We separate the tests so that we don't have to manually stop notifications
after each test is done. Because notifications state is kept in the frame
and the frame cleans up the state after navigation we know the tests
won't interfere with each other.

BUG=554240
==========

to

==========
bluetooth: Separate notification tests

We separate the tests so that we don't have to manually stop notifications
after each test is done. Because notifications state is kept in the frame
and the frame cleans up the state after navigation we know the tests
won't interfere with each other.

BUG=554240

Committed: https://crrev.com/af5b0e45b29290c7d013e3c5e01a86fcfa9d50a4
Cr-Commit-Position: refs/heads/master@{#387734}
==========

[commit-bot: I haz the power, 2016-04-15 22:49:45]

Patchset 6 (id:??) landed as
https://crrev.com/af5b0e45b29290c7d013e3c5e01a86fcfa9d50a4
Cr-Commit-Position: refs/heads/master@{#387734}