Create Mojo service for locking/unlocking screen orientation.

Mojo-ify implementation of screen orientation locking/unlocking.

This CL creates a Mojo service to replace the //content-based IPC currently
used for locking/unlocking screen orientation. Major changes:
- Renderer and browser communicate via a Mojo interface defined in
  screen_orientation_service.mojom.
- The browser-side ScreenOrientationServiceImpl is per-RenderFrameHost, as
  opposed to ScreenOrientationDispatcherHostImpl's being per-WebContents. This
  change is due to the fact that the connection established from the renderer
  is a per-render frame connection. As a result, ScreenOrientationServiceImpl
  does not have to do the muxing of RenderFrameHosts that
  ScreenOrientationDispatcherHostImpl did.
- A new ScreenOrientationContext is introduced to provide the per-WebContents
  screen orientation context that ScreenOrientationServiceImpl requires (e.g.,
  notification of a change in screen orientation).
- Request IDs no longer have to be passed from the renderer to the browser,
  as the renderer binds the request ID associated with a request in the
  response callback that it creates for the request.

This CL does not attempt to decouple the screen orientation service from //
content. Decoupling ScreenOrientationServiceImpl, ScreenOrientationProvider,
and ScreenOrientationContext will be relatively easy: it will just require
some fiddling so that ScreenOrientationProvider::Create() no longer needs to
take a WebContents. Decoupling ScreenOrientationProviderAndroid will be an
interesting exercise, as it looks like there are some real dependencies on
//content that will need to be injected.

TEST=Test that websites using the screen orientation API can still correctly
lock/unlock the screen orientation on Android.

[blundell, 2014-09-18 09:24:23]

Patchset #7 (id:120001) has been deleted

[blundell, 2014-09-18 09:52:44]

blundell@chromium.org changed reviewers:
+ darin@chromium.org, mlamouri@chromium.org, qsr@chromium.org

[blundell, 2014-09-18 09:52:46]

mlamouri, qsr, darin: for review.
timvolodine: FYI.
jonross: FYI. I won't land this without coordinating with you in any case.

Mounir, I tested your test webpage on Android and everything seems to work as
expected.

I'm still in the process of porting the ScreenOrientationDispatcher unittest.

https://codereview.chromium.org/549603003/diff/100001/content/browser/screen_...
File content/browser/screen_orientation/screen_orientation_provider_android.cc
(right):

https://codereview.chromium.org/549603003/diff/100001/content/browser/screen_...
content/browser/screen_orientation/screen_orientation_provider_android.cc:43:
ScreenOrientationLockTypeToWebScreenOrientationLockType(lock_type);
I didn't bother changing this file to talk in the mojo lock type instead of the
blink lock type at this point to avoid adding noise.

https://codereview.chromium.org/549603003/diff/100001/content/public/browser/...
File content/public/browser/screen_orientation_dispatcher_host.h (left):

https://codereview.chromium.org/549603003/diff/100001/content/public/browser/...
content/public/browser/screen_orientation_dispatcher_host.h:21: // The renderer
process will be notified that the lock succeeded only if
Mounir, AFAICT, |request_id| will always match the current lock information in
the current implementation if the ScreenOrientationDispatcherHost has non-NULL
current lock information. My new implementation does not track request IDs on
the browser side. Is there a corner case that I'm not seeing where that will
cause ScreenOrientationServiceImpl to incorrectly call back the renderer on
receiving a NotifyLockResult() call from the ScreenOrientationProvider?

[qsr, 2014-09-18 13:23:01]

https://codereview.chromium.org/549603003/diff/140001/content/browser/screen_...
File content/browser/screen_orientation/screen_orientation_service_impl.h
(right):

https://codereview.chromium.org/549603003/diff/140001/content/browser/screen_...
content/browser/screen_orientation/screen_orientation_service_impl.h:31: //
ScreenOrientationContext:
ScreenOrientationContext::Observer ?

[blundell, 2014-09-22 11:31:29]

Patchset #10 (id:200001) has been deleted

[blundell, 2014-09-22 11:37:13]

I ported the ScreenOrientationDispatcher unittest to the new world.

mounir, darin: friendly ping.

https://codereview.chromium.org/549603003/diff/140001/content/browser/screen_...
File content/browser/screen_orientation/screen_orientation_service_impl.h
(right):

https://codereview.chromium.org/549603003/diff/140001/content/browser/screen_...
content/browser/screen_orientation/screen_orientation_service_impl.h:31: //
ScreenOrientationContext:
On 2014/09/18 13:23:01, qsr wrote:
> ScreenOrientationContext::Observer ?

Done.

[qsr, 2014-09-22 13:01:48]

https://codereview.chromium.org/549603003/diff/140001/content/browser/android...
File content/browser/android/content_view_core_impl.cc (right):

https://codereview.chromium.org/549603003/diff/140001/content/browser/android...
content/browser/android/content_view_core_impl.cc:1258:
->screen_orientation_context()
The -> should be on the previous line.

https://codereview.chromium.org/549603003/diff/140001/content/browser/frame_h...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/549603003/diff/140001/content/browser/frame_h...
content/browser/frame_host/render_frame_host_impl.cc:192:
GetServiceRegistry()->AddService<ScreenOrientationService>(
You might want to extract this into a registration method, as more AddService
will come.

https://codereview.chromium.org/549603003/diff/220001/content/browser/screen_...
File content/browser/screen_orientation/screen_orientation_service_impl.cc
(right):

https://codereview.chromium.org/549603003/diff/220001/content/browser/screen_...
content/browser/screen_orientation/screen_orientation_service_impl.cc:23:
->screen_orientation_context()),
The -> should be on the previous line.

https://codereview.chromium.org/549603003/diff/220001/content/browser/screen_...
content/browser/screen_orientation/screen_orientation_service_impl.cc:24:
provider_(ScreenOrientationProvider::Create(
As discussed live, maybe we want the provider to be per-webcontent given that a
single lock request can be live at any time.

https://codereview.chromium.org/549603003/diff/220001/content/public/common/s...
File content/public/common/screen_orientation.mojom (right):

https://codereview.chromium.org/549603003/diff/220001/content/public/common/s...
content/public/common/screen_orientation.mojom:24: ERROR_NOT_AVAILABLE,
Do we want to remove this, and instead not register a service for platform that
do not support his feature, and then rely on OnConnectionError when requesting
the service?

https://codereview.chromium.org/549603003/diff/220001/tools/vim/chromium.ycm_...
File tools/vim/chromium.ycm_extra_conf.py (right):

https://codereview.chromium.org/549603003/diff/220001/tools/vim/chromium.ycm_...
tools/vim/chromium.ycm_extra_conf.py:87: output_dir = 'out_linux'
I don't think you want to commit that.

[blundell, 2014-09-22 13:21:54]

https://codereview.chromium.org/549603003/diff/220001/content/browser/screen_...
File content/browser/screen_orientation/screen_orientation_service_impl.cc
(right):

https://codereview.chromium.org/549603003/diff/220001/content/browser/screen_...
content/browser/screen_orientation/screen_orientation_service_impl.cc:24:
provider_(ScreenOrientationProvider::Create(
On 2014/09/22 13:01:48, qsr wrote:
> As discussed live, maybe we want the provider to be per-webcontent given that
a
> single lock request can be live at any time.

Yes, I agree. The provider can be per-WebContents and can have a virtual
OnOrientationChange() method that is what ContentViewCoreImpl will call
directly. Rather than the provider taking in a callback in its constructor, it
can take in a callback in its LockOrientation() method. The
ScreenOrientationContext class that I introduced can then go back away. This
design seems like it will be cleaner.

Mounir, does this make sense to you?

[blundell, 2014-09-23 07:07:01]

I went ahead and implemented qsr's suggestion to have the
ScreenOrientationProvider be per-WebContents instead of per-RVH. It's
significatly cleaner I think.

[qsr, 2014-09-23 08:23:00]

https://chromiumcodereview.appspot.com/549603003/diff/280001/content/browser/...
File content/browser/screen_orientation/screen_orientation_provider_android.cc
(right):

https://chromiumcodereview.appspot.com/549603003/diff/280001/content/browser/...
content/browser/screen_orientation/screen_orientation_provider_android.cc:103:
if (LockMatchesCurrentOrientation(pending_lock_orientation_)) {
I don't understand exactly why we need this. If we can receive this message for
other reason than the request, then we can randomly get the right orientation,
so this might be true for another reason than the request. In the end, given the
way concurrent request can happen, I'm wondering why we don't just respond with
a success just after sending the request to the system. Given that a lock can be
overridden by a concurrent frame the ms after it succeeds, trying to return an
error for this case seems a lot of work for no benefit. What am I missing?

 Mounir, do you remember why we do it this way?