New issue
Advanced search Search tips

Issue 686692 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 686687



Sign in to add a comment

Eliminate vibration_browsertest.cc injecting fake VibrationManager impl directly into RFH's InterfaceRegistry

Project Member Reported by blundell@chromium.org, Jan 30 2017

Issue description

vibration_browsertest.cc tests vibration integration between Blink and the browser, e.g.:
- Checking that executing navigator.vibrate() in JS causes a host-side vibration request
- Checking that an outstanding vibration is cancelled if the main frame is reloaded

It does this by injecting a fake VibrationManager impl into RenderFrameHost's InterfaceRegistry. However, there is no easy mechanism to support that once VibrationManager is hosted in the Device Service. Thus, we first need to eliminate this mechanism. Possible approaches:

- Expose a test-only interface on VibrationManager wherein it can receive a VibrationManager to which it will forward all requests. Access to this interface could be guarded by manifest specification such that it is only accessible from //content/browser.

- Expose the ability to inject a fake service impl. Perhaps this could just be done at the //content level?

- Replace these browsertests entirely with layout tests. The browsertests are not testing the real impl in any case. Would need to map the actions being tested (e.g., reloading main frame) to flows to take in layout tests.
 

Comment 1 by leon....@intel.com, Feb 15 2017

Owner: leon....@intel.com

Comment 2 by leon....@intel.com, Feb 16 2017

Status: Started (was: Available)

Comment 3 by leon....@intel.com, Feb 16 2017

Is investigating how to create layout tests to cover what vibration_browsertest.cc does currently.
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 17 2017

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

commit 5033377133364937998758c5b77bf63e95824e73
Author: leon.han <leon.han@intel.com>
Date: Fri Mar 17 07:43:52 2017

[DeviceService] Replace vibration_browsertest.cc with layout tests.

vibration_browsertest.cc tests vibration integration between Blink and
the browser. It does this by injecting a fake VibrationManager impl
into RenderFrameHost's InterfaceRegistry.
However, this blocks us to port VibrationManager to be hosted in
Device Service because after that there is no such mechanism to do this.

So this CL replaces these browsertests with layout tests, with a TODO
to add 'main frame reload' tests later, because for now we consider that
'sub frame reload' tests have actually covered what we want to verify
for lifecycle of NavigatorVibration.

BUG= 686692 
TEST=blink_tests

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

[delete] https://crrev.com/a6660c0cd2517e9da95f099211042d0c39fe8055/content/browser/vibration_browsertest.cc
[modify] https://crrev.com/5033377133364937998758c5b77bf63e95824e73/content/test/BUILD.gn
[add] https://crrev.com/5033377133364937998758c5b77bf63e95824e73/third_party/WebKit/LayoutTests/vibration/resources/vibrate-from-iframe.html
[add] https://crrev.com/5033377133364937998758c5b77bf63e95824e73/third_party/WebKit/LayoutTests/vibration/resources/vibration-helpers.js
[add] https://crrev.com/5033377133364937998758c5b77bf63e95824e73/third_party/WebKit/LayoutTests/vibration/vibration-iframe.html
[add] https://crrev.com/5033377133364937998758c5b77bf63e95824e73/third_party/WebKit/LayoutTests/vibration/vibration.html

Comment 5 by leon....@intel.com, Mar 17 2017

Status: Fixed (was: Started)
Components: Internals>Services>Device

Sign in to add a comment