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

Issue 717377 link

Starred by 1 user
Status: Fixed
Owner:
leon....@intel.com
Closed: May 2017
Cc:
█ ke...@intel.com
█ roc...@chromium.org
blundell@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature

Blocking:
issue 612328
issue 715985
issue 717378
issue 717379



Sign in to add a comment

Need an infra supporting end-to-end browsertests for Services

Project Member Reported by leon....@intel.com, May 2 2017 
In the process of Device Service development, while we're removing some browsertests for device features(battery, vibration etc.), we noticed that some end-to-end tests are still necessary. Although we have blink layout tests to verify JS API behavior inside renderer and service tests to verify mojo interfaces implemented correctly inside Device Service, there is still a gap: 
We're lack of verifying those mojo interfaces are really registered/exposed to their clients correctly in a real browser environment, that is to say, whether clients can always connect to those mojo interfaces well even if sometimes service manager infra would change. And of course, such end-to-end test should not depend on any internal implementations of Device Service.

We need an infra to support such end-to-end test, not only for Device Service, but for other services in general.

Discussions detail: https://groups.google.com/a/chromium.org/forum/#!topic/services-dev/lJCKAElWz-E

Comment 1 by leon....@intel.com, May 2 2017

Blocking: 612328

Comment 2 by leon....@intel.com, May 2 2017

Blocking: 717378

Comment 3 by leon....@intel.com, May 2 2017

Blocking: 717379

Comment 4 by leon....@intel.com, May 2 2017

Cc: -leon....@intel.com
Owner: leon....@intel.com
Status: Started (was: Untriaged)

Comment 5 by leon....@intel.com, May 2 2017

Blocking: 715985
Project Member

Comment 6 by bugdroid1@chromium.org, May 12 2017 

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

commit d25e37df0f08a754fbc11041451a5ce7ca7d043d
Author: leon.han <leon.han@intel.com>
Date: Fri May 12 04:16:16 2017

Enable overriding interface binders for any services running in current process.

This CL
  - holds a 'service name <--> binder registry' map inside
    service manager client library, at run time it acts as a global
    variable in any process linking with service manager client library.
  - lets ServiceContext::OnBindInterface() use the global map to
    intercept requests before dispatching them to target Services, that
    is to say, such interception may happen for any Services running in
    current process.
  - exposes {SetGlobalBinder,ClearGlobalBinders}ForTesting to set/clear
    binders in the global map.
  - adds a VibrationTest browser test to show the usage of the new infra
    described above.

BUG= 717377 ,  717378 
TEST=content_browsertests
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/d25e37df0f08a754fbc11041451a5ce7ca7d043d/content/browser/frame_host/render_frame_host_impl.cc
[add] https://crrev.com/d25e37df0f08a754fbc11041451a5ce7ca7d043d/content/browser/vibration_browsertest.cc
[modify] https://crrev.com/d25e37df0f08a754fbc11041451a5ce7ca7d043d/content/test/BUILD.gn
[modify] https://crrev.com/d25e37df0f08a754fbc11041451a5ce7ca7d043d/services/service_manager/public/cpp/binder_registry.cc
[modify] https://crrev.com/d25e37df0f08a754fbc11041451a5ce7ca7d043d/services/service_manager/public/cpp/binder_registry.h
[modify] https://crrev.com/d25e37df0f08a754fbc11041451a5ce7ca7d043d/services/service_manager/public/cpp/interface_binder.cc
[modify] https://crrev.com/d25e37df0f08a754fbc11041451a5ce7ca7d043d/services/service_manager/public/cpp/interface_binder.h
[modify] https://crrev.com/d25e37df0f08a754fbc11041451a5ce7ca7d043d/services/service_manager/public/cpp/service_context.cc
[modify] https://crrev.com/d25e37df0f08a754fbc11041451a5ce7ca7d043d/services/service_manager/public/cpp/service_context.h
Project Member

Comment 7 by bugdroid1@chromium.org, May 12 2017 

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

commit 7795bff0b81a0cd0a8f840357545f591d0cdf6b0
Author: henrika <henrika@chromium.org>
Date: Fri May 12 08:00:34 2017

Revert of Enable overriding interface binders for any services running in current process. (patchset #5 id:130001 of https://codereview.chromium.org/2859343003/ )

Reason for revert:
VibrationTest.Vibrate times out in browser_side_navigation_content_browsertests on Linux.

See https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.linux%2FLinux_Tests__dbg__1_%2F63093%2F%2B%2Frecipes%2Fsteps%2Fbrowser_side_navigation_content_browsertests%2F0%2Fstdout

Original issue's description:
> Enable overriding interface binders for any services running in current process.
>
> This CL
>   - holds a 'service name <--> binder registry' map inside
>     service manager client library, at run time it acts as a global
>     variable in any process linking with service manager client library.
>   - lets ServiceContext::OnBindInterface() use the global map to
>     intercept requests before dispatching them to target Services, that
>     is to say, such interception may happen for any Services running in
>     current process.
>   - exposes {SetGlobalBinder,ClearGlobalBinders}ForTesting to set/clear
>     binders in the global map.
>   - adds a VibrationTest browser test to show the usage of the new infra
>     described above.
>
> BUG= 717377 ,  717378 
> TEST=content_browsertests
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
>
> Review-Url: https://codereview.chromium.org/2859343003
> Cr-Commit-Position: refs/heads/master@{#471216}
> Committed: https://chromium.googlesource.com/chromium/src/+/d25e37df0f08a754fbc11041451a5ce7ca7d043d

TBR=rockot@chromium.org,blundell@chromium.org,jam@chromium.org,leon.han@intel.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 717377 ,  717378 

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

[modify] https://crrev.com/7795bff0b81a0cd0a8f840357545f591d0cdf6b0/content/browser/frame_host/render_frame_host_impl.cc
[delete] https://crrev.com/e1f4f5e2d1cc9c40e2ab79491606e46d2bc86a90/content/browser/vibration_browsertest.cc
[modify] https://crrev.com/7795bff0b81a0cd0a8f840357545f591d0cdf6b0/content/test/BUILD.gn
[modify] https://crrev.com/7795bff0b81a0cd0a8f840357545f591d0cdf6b0/services/service_manager/public/cpp/binder_registry.cc
[modify] https://crrev.com/7795bff0b81a0cd0a8f840357545f591d0cdf6b0/services/service_manager/public/cpp/binder_registry.h
[modify] https://crrev.com/7795bff0b81a0cd0a8f840357545f591d0cdf6b0/services/service_manager/public/cpp/interface_binder.cc
[modify] https://crrev.com/7795bff0b81a0cd0a8f840357545f591d0cdf6b0/services/service_manager/public/cpp/interface_binder.h
[modify] https://crrev.com/7795bff0b81a0cd0a8f840357545f591d0cdf6b0/services/service_manager/public/cpp/service_context.cc
[modify] https://crrev.com/7795bff0b81a0cd0a8f840357545f591d0cdf6b0/services/service_manager/public/cpp/service_context.h
Project Member

Comment 8 by bugdroid1@chromium.org, May 19 2017 

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

commit 22f596339a2bb40e8ffbae16f2dd87406e4d0ed7
Author: Ken Rockot <rockot@chromium.org>
Date: Fri May 19 03:03:01 2017

Service Manager: Public component targets

Makes public interfaces and cpp targets components instead of
source_sets. This is necessary for test support infrastructure inside
the client library.

In order to make the client library a component target, we have to make
the public mojom library a component target as well. This means we also
have to split apart the bits of the client library that are used in
typemaps (because those are actually part of the mojom component) to
avoid circular dependencies.

A few constants are moved into the public constants mojom as well since
one is needed by the Identity impl, and this in turn means that the
constants mojom target also has to be a component.

The dependency graph under service manager's public dir thus looks
something like (downward means depends-on):

  [cpp component]
       |        |
       |      [interfaces component]
       |                  |       |
       |                  |       |
       [cpp_types component]      |
                    |             |
                    |             |
                [interfaces/constants component]


BUG= 717377 
TBR=ben@chromium.org

Change-Id: I5fa8685929113728445b7395359500d76da9b677
Reviewed-on: https://chromium-review.googlesource.com/506598
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Colin Blundell (OOO until May 22) <blundell@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#473051}
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/BUILD.gn
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/bind_source_info.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/bind_source_info.typemap
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/bind_source_info_struct_traits.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/binder_registry.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/connector.h
[add] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/export.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/identity.cc
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/identity.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/identity.typemap
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/identity_struct_traits.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/interface_provider.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/interface_provider_spec.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/interface_provider_spec.typemap
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/interface_provider_spec_struct_traits.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/service.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/service_context.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/service_context_ref.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/service_runner.h
[add] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/cpp/types_export.h
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/interfaces/BUILD.gn
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/interfaces/connector.mojom
[modify] https://crrev.com/22f596339a2bb40e8ffbae16f2dd87406e4d0ed7/services/service_manager/public/interfaces/constants.mojom
Project Member

Comment 9 by bugdroid1@chromium.org, May 19 2017 

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

commit 31c48e20cf23dcc484e259d339cbc380d6f162b1
Author: leon.han <leon.han@intel.com>
Date: Fri May 19 07:52:14 2017

Reland 1: Enable overriding interface binders for any services running in current process.

The original CL was reverted because:
VibrationTest.Vibrate times out in
browser_side_navigation_content_browsertests on Linux.
And the root cause has been fixed by rockot@'s CL
https://chromium-review.googlesource.com/c/506598/,
so we can reland the original CL without any change now.

This CL
  - holds a 'service name <--> binder registry' map inside
    service manager client library, at run time it acts as a global
    variable in any process linking with service manager client library.
  - lets ServiceContext::OnBindInterface() use the global map to
    intercept requests before dispatching them to target Services, that
    is to say, such interception may happen for any Services running in
    current process.
  - exposes {SetGlobalBinder,ClearGlobalBinders}ForTesting to set/clear
    binders in the global map.
  - adds a VibrationTest browser test to show the usage of the new infra
    described above.

BUG= 717377 ,  717378 
TEST=content_browsertests
TBR=rockot@chromium.org,jam@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2859343003
Cr-Original-Commit-Position: refs/heads/master@{#471216}
Committed: https://chromium.googlesource.com/chromium/src/+/d25e37df0f08a754fbc11041451a5ce7ca7d043d
Review-Url: https://codereview.chromium.org/2879843002
Cr-Commit-Position: refs/heads/master@{#473120}

[modify] https://crrev.com/31c48e20cf23dcc484e259d339cbc380d6f162b1/content/browser/frame_host/render_frame_host_impl.cc
[add] https://crrev.com/31c48e20cf23dcc484e259d339cbc380d6f162b1/content/browser/vibration_browsertest.cc
[modify] https://crrev.com/31c48e20cf23dcc484e259d339cbc380d6f162b1/content/test/BUILD.gn
[modify] https://crrev.com/31c48e20cf23dcc484e259d339cbc380d6f162b1/services/service_manager/public/cpp/binder_registry.cc
[modify] https://crrev.com/31c48e20cf23dcc484e259d339cbc380d6f162b1/services/service_manager/public/cpp/binder_registry.h
[modify] https://crrev.com/31c48e20cf23dcc484e259d339cbc380d6f162b1/services/service_manager/public/cpp/interface_binder.cc
[modify] https://crrev.com/31c48e20cf23dcc484e259d339cbc380d6f162b1/services/service_manager/public/cpp/interface_binder.h
[modify] https://crrev.com/31c48e20cf23dcc484e259d339cbc380d6f162b1/services/service_manager/public/cpp/service_context.cc
[modify] https://crrev.com/31c48e20cf23dcc484e259d339cbc380d6f162b1/services/service_manager/public/cpp/service_context.h

Comment 10 by leon....@intel.com, May 20 2017

Status: Fixed (was: Started)

Comment 11 by blundell@chromium.org, Nov 7 2017

Components: Internals>Services>Device
►

Sign in to add a comment