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

Issue 1125133005: bluetooth: Move testing IPC from BluetoothDispatcher to BlinkTestRunner (Closed)

Created:
5 years, 7 months ago by ortuno
Modified:
5 years, 7 months ago
Reviewers:
Tom Sepez, scheib, jam
CC:
chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, jam, jochen+watch_chromium.org, mkwst+moarreviews-shell_chromium.org, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, scheib+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

bluetooth: Move testing IPC from BluetoothDispatcher to BlinkTestRunner This patch removes the testing IPC from BluetoothDispatcher and BluetoothDispatcherHost. This patch also changes the way the mock BluetoothAdapter was set in BluetoothDispatcherHost. Instead of receiving an IPC to set the adapter, BluetoothDispatcherHost exposes a function to directly set the adapter. This function is used by LayoutTestSupport to set the adapter. Mock adapter flow before: BlinkTestRunner -> LayoutTestSupport -> BluetoothDispatcher --IPC--> BluetoothDispatcherHost (Mock constructed here) After: BlinkTestRunner --IPC--> LayoutTestMessages(Mock constructed here) -> LayoutTestSupport -> BluetoothDispatcherHost This is the first of two patches to remove testing from BluetoothDispatcher and BluetoothDispatcherHost: [1] This patch. [2] http://crrev.com/1132943002 BUG=436284 Committed: https://crrev.com/3f7142d0acf5e930743cbe5d754084c464ac3c85 Cr-Commit-Position: refs/heads/master@{#330647} Committed: https://crrev.com/a58d60c749f55046a8a85c6398864759fd877266 Cr-Commit-Position: refs/heads/master@{#330696}

Patch Set 1 : #

Total comments: 12

Patch Set 2 : Fixes based on comments" #

Patch Set 3 : Fix for stable release mode tests #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+60 lines, -53 lines) Patch
M content/browser/bluetooth/bluetooth_dispatcher_host.h View 3 chunks +3 lines, -2 lines 0 comments Download
M content/browser/bluetooth/bluetooth_dispatcher_host.cc View 1 2 chunks +17 lines, -19 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.h View 3 chunks +5 lines, -0 lines 0 comments Download
M content/browser/renderer_host/render_process_host_impl.cc View 2 chunks +6 lines, -1 line 0 comments Download
M content/child/blink_platform_impl.h View 1 chunk +0 lines, -2 lines 0 comments Download
M content/child/bluetooth/bluetooth_dispatcher.h View 1 chunk +0 lines, -1 line 0 comments Download
M content/child/bluetooth/bluetooth_dispatcher.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M content/child/bluetooth/web_bluetooth_impl.h View 1 chunk +0 lines, -3 lines 0 comments Download
M content/child/bluetooth/web_bluetooth_impl.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M content/common/bluetooth/bluetooth_messages.h View 1 chunk +0 lines, -5 lines 0 comments Download
M content/public/test/layouttest_support.h View 1 chunk +2 lines, -2 lines 0 comments Download
M content/shell/browser/layout_test/layout_test_message_filter.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M content/shell/browser/layout_test/layout_test_message_filter.cc View 4 chunks +11 lines, -1 line 0 comments Download
M content/shell/common/layout_test/layout_test_messages.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M content/shell/renderer/layout_test/blink_test_runner.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/test/layouttest_support.cc View 1 2 2 chunks +12 lines, -6 lines 1 comment Download

Messages

Total messages: 47 (27 generated)
ortuno
scheib: PTAL
5 years, 7 months ago (2015-05-15 23:26:11 UTC) #16
scheib
https://codereview.chromium.org/1125133005/diff/230001/content/browser/bluetooth/bluetooth_dispatcher_host.cc File content/browser/bluetooth/bluetooth_dispatcher_host.cc (right): https://codereview.chromium.org/1125133005/diff/230001/content/browser/bluetooth/bluetooth_dispatcher_host.cc#newcode136 content/browser/bluetooth/bluetooth_dispatcher_host.cc:136: void BluetoothDispatcherHost::SetBluetoothAdapterForTesting( Move to match declaration order. https://codereview.chromium.org/1125133005/diff/230001/content/browser/bluetooth/bluetooth_dispatcher_host.h File ...
5 years, 7 months ago (2015-05-16 01:00:35 UTC) #17
jam
lgtm https://codereview.chromium.org/1125133005/diff/230001/content/test/layouttest_support.cc File content/test/layouttest_support.cc (right): https://codereview.chromium.org/1125133005/diff/230001/content/test/layouttest_support.cc#newcode326 content/test/layouttest_support.cc:326: DCHECK(bdh); nit: remove all dchecks from this method. ...
5 years, 7 months ago (2015-05-18 15:37:58 UTC) #19
scheib
LGTM https://codereview.chromium.org/1125133005/diff/230001/content/browser/bluetooth/bluetooth_dispatcher_host.h File content/browser/bluetooth/bluetooth_dispatcher_host.h (right): https://codereview.chromium.org/1125133005/diff/230001/content/browser/bluetooth/bluetooth_dispatcher_host.h#newcode37 content/browser/bluetooth/bluetooth_dispatcher_host.h:37: void SetBluetoothAdapterForTesting(const std::string& name); On 2015/05/16 01:00:35, scheib ...
5 years, 7 months ago (2015-05-18 16:43:30 UTC) #20
ortuno
https://codereview.chromium.org/1125133005/diff/230001/content/browser/bluetooth/bluetooth_dispatcher_host.cc File content/browser/bluetooth/bluetooth_dispatcher_host.cc (right): https://codereview.chromium.org/1125133005/diff/230001/content/browser/bluetooth/bluetooth_dispatcher_host.cc#newcode136 content/browser/bluetooth/bluetooth_dispatcher_host.cc:136: void BluetoothDispatcherHost::SetBluetoothAdapterForTesting( On 2015/05/16 at 01:00:34, scheib wrote: > ...
5 years, 7 months ago (2015-05-18 17:40:17 UTC) #21
ortuno
5 years, 7 months ago (2015-05-18 17:40:18 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1125133005/250001
5 years, 7 months ago (2015-05-18 17:53:20 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1125133005/250001
5 years, 7 months ago (2015-05-19 22:07:35 UTC) #28
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/64754)
5 years, 7 months ago (2015-05-19 22:16:01 UTC) #30
ortuno
@tsepez: Please OWNERS review for content/common/bluetooth/bluetooth_messages.h
5 years, 7 months ago (2015-05-19 22:18:12 UTC) #32
Tom Sepez
lgtm
5 years, 7 months ago (2015-05-19 22:45:17 UTC) #33
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1125133005/250001
5 years, 7 months ago (2015-05-19 22:45:57 UTC) #35
commit-bot: I haz the power
Committed patchset #2 (id:250001)
5 years, 7 months ago (2015-05-19 22:56:01 UTC) #36
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/3f7142d0acf5e930743cbe5d754084c464ac3c85 Cr-Commit-Position: refs/heads/master@{#330647}
5 years, 7 months ago (2015-05-19 22:56:58 UTC) #37
leviw_travelin_and_unemployed
A revert of this CL (patchset #2 id:250001) has been created in https://codereview.chromium.org/1142303002/ by leviw@chromium.org. ...
5 years, 7 months ago (2015-05-20 00:18:09 UTC) #38
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1125133005/270001
5 years, 7 months ago (2015-05-20 02:46:20 UTC) #42
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1125133005/270001
5 years, 7 months ago (2015-05-20 02:47:17 UTC) #44
scheib
https://codereview.chromium.org/1125133005/diff/270001/content/test/layouttest_support.cc File content/test/layouttest_support.cc (right): https://codereview.chromium.org/1125133005/diff/270001/content/test/layouttest_support.cc#newcode324 content/test/layouttest_support.cc:324: if (dispatcher_host != NULL) Just "if (dispatcher_host)" in C++
5 years, 7 months ago (2015-05-20 04:21:27 UTC) #45
commit-bot: I haz the power
Committed patchset #3 (id:270001)
5 years, 7 months ago (2015-05-20 04:42:23 UTC) #46
commit-bot: I haz the power
5 years, 7 months ago (2015-05-20 04:43:15 UTC) #47
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/a58d60c749f55046a8a85c6398864759fd877266
Cr-Commit-Position: refs/heads/master@{#330696}

Powered by Google App Engine
This is Rietveld 408576698