New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 673147 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug

Blocked on:
issue 633191

Blocking:
issue 510287



Sign in to add a comment

Enable WebBluetooth tests on MacOS

Project Member Reported by dougt@chromium.org, Dec 11 2016

Issue description

Currently some of the tests for WebBluetooth are disabled on the MacOS, specifically the ones dealing with notifications. 

https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/TestExpectations?q=LayoutTests/Test&sq=package:chromium&l=1640

I couldn't think of a really good reason for them to be disabled.  I think we should flip them back on, and remove the #ifdef preventing notifications from working on MacOS.

Thoughts?
 

Comment 1 by dougt@chromium.org, Dec 11 2016

This was added in:

https://codereview.chromium.org/1940923002

Comment 2 by ortuno@chromium.org, Dec 11 2016

Blockedon: 633191
Blocking: 510287
Cc: fbeaufort@chromium.org
 Issue 607822  has some context. fbeaufort: Since you requested the change, wdyt?
IIRC I've disabled these tests on macOS because they were failing since we were adding rejecting all notifications Web Bluetooth promises: https://chromium.googlesource.com/chromium/src.git/+/4ae4c0730e8984a7e18a0b2e170deec765b1a600%5E%21/#F1
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 13 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b64f718b6b3f1bc5cae3a0552fb2ce63e7b6e300

commit b64f718b6b3f1bc5cae3a0552fb2ce63e7b6e300
Author: dougt <dougt@chromium.org>
Date: Tue Dec 13 01:12:33 2016

Enable WebBluetooth LayoutTests for notifications on MacOS.

In  Bug 607822 , many MacOS tests were disabled because the underlying implementation not ready. We have since fixed many bugs, and notifications seem to be working fine. This patch removes all of web bluetooth TestExceptions and re-enables notifications on the MacOS

BUG= 673147 

Review-Url: https://codereview.chromium.org/2569593002
Cr-Commit-Position: refs/heads/master@{#437975}

[modify] https://crrev.com/b64f718b6b3f1bc5cae3a0552fb2ce63e7b6e300/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/b64f718b6b3f1bc5cae3a0552fb2ce63e7b6e300/third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTCharacteristic.cpp

dougt@ stopNotifications are still NOT working on macOS. See https://github.com/WebBluetoothCG/web-bluetooth/blob/gh-pages/implementation-status.md#chrome

FYI, I've added this "Promise.reject" because I've witnessed many developers confused during codelabs events. I wish we would implement stopNotifications first on macOS before re-enabling WebBluetooth LayoutTests.

Comment 7 by dougt@chromium.org, Dec 13 2016

Yes, the bug about stop notifications is 633191.

My thinking is roughly: having notifications that you can't stop is better than having no notifications.  That is, you can listen to value changes which is important for many use cases without having to poll.  Yes, it's sad that you can't stop notifications, but it's not a deal breaker for many applications.

Also, the same problem exists on Windows.
Cc: jlebel@chromium.org
having notifications that you can't stop is better than having no notifications. We agree. But if stopNotifications() actually tells me it works (because it didn't fail), I'm in much more trouble.

The original patch was added so that developers are aware that stopNotifications() don't work yet.

I'd argue we should fix https://bugs.chromium.org/p/chromium/issues/detail?id=633191 before enabling all WebBluetooth LayoutTests for notifications on MacOS.

Good news is that jlebel@ is already working on it ;)



Comment 9 by dougt@chromium.org, Jan 27 2017

Status: Fixed (was: Assigned)

Sign in to add a comment