Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(267)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2564283004: bluetooth: web: Include the UUID in the error message. (Closed)

Created:
4 years ago by perja
Modified:
3 years, 10 months ago
Reviewers:
haraken, scheib, ortuno
CC:
blink-reviews, chromium-reviews, haraken, ortuno+watch_chromium.org, scheib+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

bluetooth: web: Include the UUID in the error message. Show the UUID in the error message when services or characteristics can't be found. BUG=619932

Patch Set 1 #

Patch Set 2 : Only use the custom error message for the WebBluetoothResult error it was intended for. #

Total comments: 4

Patch Set 3 : Addressed issues in code review. #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+78 lines, -18 lines) Patch
M third_party/WebKit/LayoutTests/bluetooth/server/getPrimaryService/delayed-discovery-service-not-found.html View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/bluetooth/server/getPrimaryService/service-not-found.html View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/bluetooth/server/getPrimaryServices/delayed-discovery-service-with-uuid-not-found.html View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/bluetooth/server/getPrimaryServices/services-not-found-with-uuid.html View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/bluetooth/service/getCharacteristic/characteristic-not-found.html View 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/bluetooth/service/getCharacteristics/characteristics-not-found-with-uuid.html View 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/BluetoothError.h View 1 2 2 chunks +12 lines, -0 lines 2 comments Download
M third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp View 1 2 2 chunks +22 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTServer.cpp View 1 3 chunks +17 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTService.cpp View 1 3 chunks +19 lines, -5 lines 0 comments Download

Messages

Total messages: 24 (16 generated)
perja
@ortuno: PTAL.
4 years ago (2016-12-14 08:56:00 UTC) #10
ortuno
I'm currently OOO. Adding scheib instead.
4 years ago (2016-12-14 16:10:04 UTC) #12
scheib
LGTM, with comment requests: Doug, you'll likely land after this, and it would be good ...
4 years ago (2016-12-14 18:22:46 UTC) #14
perja
https://codereview.chromium.org/2564283004/diff/20001/third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp File third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp (right): https://codereview.chromium.org/2564283004/diff/20001/third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp#newcode159 third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp:159: bool BluetoothError::isSameError(int32_t error, On 2016/12/14 18:22:46, scheib wrote: > ...
4 years ago (2016-12-15 10:54:01 UTC) #17
dougt
I created bug 676720 to add uuid error messages to descriptors.
4 years ago (2016-12-22 21:54:40 UTC) #20
perja
Looks like this never got pushed to master. I've created a new review here: https://codereview.chromium.org/2680783002
3 years, 10 months ago (2017-02-07 15:00:21 UTC) #21
haraken
Just a drive-by: https://codereview.chromium.org/2564283004/diff/40001/third_party/WebKit/Source/modules/bluetooth/BluetoothError.h File third_party/WebKit/Source/modules/bluetooth/BluetoothError.h (right): https://codereview.chromium.org/2564283004/diff/40001/third_party/WebKit/Source/modules/bluetooth/BluetoothError.h#newcode9 third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:9: #include "third_party/WebKit/public/platform/modules/bluetooth/web_bluetooth.mojom-blink.h" Nit: "third_party/WebKit" is not ...
3 years, 10 months ago (2017-02-07 15:22:52 UTC) #23
perja
3 years, 10 months ago (2017-02-07 15:41:52 UTC) #24 
haraken: I will drop this review. New review:
https://codereview.chromium.org/2680783002

On 2017/02/07 15:22:52, haraken wrote:
> Just a drive-by:
> 
>
https://codereview.chromium.org/2564283004/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/bluetooth/BluetoothError.h (right):
> 
>
https://codereview.chromium.org/2564283004/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:9: #include
>
"third_party/WebKit/public/platform/modules/bluetooth/web_bluetooth.mojom-blink.h"
> 
> Nit: "third_party/WebKit" is not needed.
> 
>
https://codereview.chromium.org/2564283004/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/bluetooth/BluetoothError.h:40:
> mojom::blink::WebBluetoothResult webError);
> 
> Can we use a type mapping for this? (I don't know if we can map a mojom type
to
> a primitive type though.)

Powered by Google App Engine
This is Rietveld 408576698