btw above is for the content/browser code. The code in content/renderer should move to blink per onion soup project.

Hi, I am interested in this and  trying to fix this issue.

+rjkroege The mus window server may need to know about screen orientation changes for display management.

+rockot@

Note that a while ago I had a patch to get this started:

https://codereview.chromium.org/549603003/

and a design doc for the effort:

https://docs.google.com/document/d/1wiarE1qW7k1fFm1pe6LXkjEqx1bfHOiTnMGeWxOw_2s/edit#

At that time landing the patch was blocked by the fact that the Mojo messages required ordering with other Chrome IPC (see the design doc). However, that is now supported. Ken, do you have a pointer for supporting ordering between Chrome IPC and a given Mojo pipe? Or does it Just Work, in which case is there a recommended convention for noting that a given Mojo impl *relies* on such ordering for revisiting when the time comes?

If you need FIFO with Chrome IPC, you need to use Channel-associated interfaces.

For associated interfaces in general see https://www.chromium.org/developers/design-documents/mojo/associated-interfaces

In content there are several specific ways you might want to use this feature depending on the nature of the IPC you're porting.

Here are some example CLs which use content::BrowserAssociatedInterface<T> to simplify porting BrowserMessageFilter messages:
  1. https://codereview.chromium.org/2167513003 (SetCookie)
  2. https://codereview.chromium.org/2202723005 (follow-up CL for GetCookies)

I recently also added support for routed associated interfaces (that is, having a 1:1 pairing between instances of some associated interface and corresponding routing IDs), with some to support per-frame interfaces bound to a specific WebContents (browser-side) or RenderFrame (renderer-side).

For that, see https://codereview.chromium.org/2310583002. This hasn't landed yet, but the facilities it depends on are on ToT now. This ports two routed messages to associated mojom interfaces, one browser->renderer and one renderer->browser.

Finally, you can also add Channel-associated interface factories directly to an IPC::ChannelProxy. See IPC::ChannelProxy::AddAssociatedInterface and IPC::ChannelProxy::AddAssociatedInterfaceForIOThread. Though if you find yourself wanting to use these in content directly, it's probably a sign that we're missing a helper class in content. For example, we'll most likely add a WebContentsViewBindingSet<T> which is similar to the WebContentsFrameBindingSet<T> used above, but for per-view interfaces.

I drew this to help me understand the project. Hope this will help :)

Deleted: Screenshot from 2016-09-16 14:47:51.png 73.9 KB

	Screenshot from 2016-09-16 14:47:51.png 73.9 KB View Download

Do we need to support screen orientation changes on CrOS? My expectation is yes? And there's a step (3) missing to eliminate the content portion right? Certainly we'll need a way to tell the mus window server about orientation changes.

re: comment #12: rjkroege@, I'd like to understand the needs of CrOS/Mus+Ash here so that we can take them into account in doing this work. Who's the best person to sync up with there?

lunalu@'s diagram indeed doesn't go through decoupling from //content/browser. IIUC, lunalu@ is approaching this work from the Blink Onion Soup perspective rather than from the servicification perspective.

kuscher@: does CrOS need to support rotation (aka screen orientation changes in response to device rotation)?

I am working on a CL that will do the mojofization part. 

Thanks!

Could you a post a link to the CL on this bug when it's ready for review?

Will do.

Hi Ken,

Luna has put together https://codereview.chromium.org/2391883006/. This is a great start, but doesn't yet address the ordering issue described in the design doc I linked to above. Specifically, ordering is required between a send of ViewMsg_Resize and the message that screen orientation changed here: https://cs.chromium.org/chromium/src/content/browser/android/content_view_core_impl.cc?q=screen_orientation_dispatcher_host&sq=package:chromium&dr=C&l=1369. (The call to UpdateScreenInfo() results in the send of ViewMsg_Resize).

ViewMsg_Resize is sent from RenderWidgetHostImpl to RenderWidget. Is there a pattern in place for association at that level?

Thanks,

Colin

Just dumping some thoughts, since we've looked at screen orientation in terms of out-of-process iframes.

Screen orientation is more or less global state. I realize moving from IPC to Mojo is convenient step, but why not take the opportunity to design it properly? Why should we be sending messages for each frame on a page when they all share the state? Furthermore, why send it to all tabs using the process, when we can send it once to the process and it can inform blink?
This will allow us to decrease the number of IPCs, not increase them and should still be able to keep the correct behavior.

In addition, one reason we haven't yet refactored it for OOPIF is that there was a performance issue with splitting the screen orientation information out of ViewMsg_Resize. I think mlamouri@ tried it in the past and might have more context/details.

I've commented on the CL (https://codereview.chromium.org/2391883006) about
how to preserve ordering, but I do agree 100% with this sentiment. If it's
simple enough to reconsider the ordering dependency and just eliminate it
(and in this case, sounds like it might be) that would be preferable to
leaning on associated interfaces as a crutch.

To make sure that we're on the same page here:

- The current task is just changing the existing screen orientation-specific IPC from Chrome IPC to Mojo. I don't see how it increases the number of IPCs, but maybe there's something I'm missing.

- The ordering requirement is between the Resize IPC and the screen orientation IPC.

- I believe that eliminating the ordering requirement would in fact require "splitting the screen orientation information out of ViewMsg_Resize." It's orthogonal to the granularity at which the screen orientation IPC is sent, but is due simply to the fact that the View_Resize message conveys information that the renderer uses as part of processing the subsequently-sent screen orientation-specific IPC. [Note: The fact that it was unclear how to remove this ordering requirement is why I wasn't able to land Mojo-ification of screen orientation IPC ~2 years ago].

- The reason that the screen orientation IPC is itself per-frame is because it originates from webpages' requests to lock the screen, which themselves come in per-frame (and thus need per-frame callbacks). It seems plausible that we could send fewer IPCs and fan out entirely in the renderer. I'd prefer to separate out that change from moving to Mojo. IMO, it also seems like it would make the code more fiddly, since we'd be doing explicit muxing and demuxing where there otherwise wouldn't be any. That's to be weighed against the advantage of reducing IPCs.

Does that make sense, or did I misunderstand something?

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

commit d4a2cc818e19f277ea030e98cd86c477740ed1a6
Author: lunalu <lunalu@chromium.org>
Date: Tue Nov 22 00:27:23 2016

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 ScreenOrientationImpl implements a Mojo interface that sends
  request to ScreenOrientationProvider in replacement of
  ScreenOrientationDispatcherHostImpl.
- Request ID 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.
- To ensure the ordering between ViewMsg_Resize and the screen orientation
  messages, WebContentsFrameBindingSet<T> is used on the browser side and
  GetRemoteAssociatedInterface (and mojom::ScreenOrientationAssociatedPtr) is
  is used on the renderer side to make the mojo interface share the same
  underlying pipe as the legacy IPC Channel. As a result, the unit tests for
  ScreenOrientationDispatcher is moved under content_browsertests as it requires
  a dummy remote associated interface for the tests to be run.

BUG=612339
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/browser/BUILD.gn
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/browser/android/content_view_core_impl.cc
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/browser/frame_host/render_frame_host_delegate.cc
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/browser/frame_host/render_frame_host_delegate.h
[add] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/browser/screen_orientation/screen_orientation.cc
[add] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/browser/screen_orientation/screen_orientation.h
[delete] https://crrev.com/20c2723180d4969f8a4b7541d92b3ddc98e96f1c/content/browser/screen_orientation/screen_orientation_dispatcher_host_impl.cc
[delete] https://crrev.com/20c2723180d4969f8a4b7541d92b3ddc98e96f1c/content/browser/screen_orientation/screen_orientation_dispatcher_host_impl.h
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/common/BUILD.gn
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/common/DEPS
[add] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/common/screen_orientation.mojom
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/common/screen_orientation_messages.h
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/public/browser/BUILD.gn
[delete] https://crrev.com/20c2723180d4969f8a4b7541d92b3ddc98e96f1c/content/public/browser/screen_orientation_dispatcher_host.h
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/public/browser/screen_orientation_provider.cc
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/public/browser/screen_orientation_provider.h
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/renderer/screen_orientation/screen_orientation_dispatcher.cc
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/renderer/screen_orientation/screen_orientation_dispatcher.h
[add] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/renderer/screen_orientation/screen_orientation_dispatcher_browsertest.cc
[delete] https://crrev.com/20c2723180d4969f8a4b7541d92b3ddc98e96f1c/content/renderer/screen_orientation/screen_orientation_dispatcher_unittest.cc
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/content/test/BUILD.gn
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/third_party/WebKit/public/BUILD.gn
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/third_party/WebKit/public/platform/modules/screen_orientation/OWNERS
[add] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/third_party/WebKit/public/platform/modules/screen_orientation/WebScreenOrientationEnumTraits.h
[add] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/third_party/WebKit/public/platform/modules/screen_orientation/screen_orientation_lock_types.mojom
[add] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/third_party/WebKit/public/platform/modules/screen_orientation/screen_orientation_lock_types.typemap
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/third_party/WebKit/public/public_typemaps.gni
[modify] https://crrev.com/d4a2cc818e19f277ea030e98cd86c477740ed1a6/third_party/WebKit/public/web/WebFrameOwnerProperties.h

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

commit f2ae32c8f18e80cb12a37b359c11fe54eba38f94
Author: mlamouri <mlamouri@chromium.org>
Date: Mon Dec 12 15:11:45 2016

Fix reversed DCHECK() in ScreenOrientationProvider.

Also fixes a typo.

This is a follow-up from https://codereview.chromium.org/2391883006

BUG=612339

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

[modify] https://crrev.com/f2ae32c8f18e80cb12a37b359c11fe54eba38f94/content/public/browser/screen_orientation_provider.cc

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

commit dfbaf46a8f088f494d65ca9621a912510f3fd10b
Author: leon.han <leon.han@intel.com>
Date: Wed Dec 14 01:05:10 2016

[DeviceService] Move screen_orientation.mojom from //content to //device.

Also moves screen_orientation_lock_types.mojom out from blink into //device.

This is the first CL to decouple screen orientation codes from //content.
Follow-up CLs will
  * migrate ScreenOrientationHostMsg_StartListening and
    ScreenOrientationHostMsg_StopListening IPC messages into this mojom file.
  * decouple content/renderer/screen_orientation/* into blink.
  * decouple content/browser/screen_orientation/* into //device.

BUG=612339

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

[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/browser/BUILD.gn
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/browser/DEPS
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/browser/screen_orientation/screen_orientation.cc
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/browser/screen_orientation/screen_orientation.h
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/common/BUILD.gn
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/common/DEPS
[delete] https://crrev.com/b26677ab730d5d529467611b35232f61bead2ba3/content/common/screen_orientation.mojom
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/public/browser/BUILD.gn
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/public/browser/DEPS
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/public/browser/screen_orientation_provider.cc
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/public/browser/screen_orientation_provider.h
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/renderer/BUILD.gn
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/renderer/DEPS
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/renderer/screen_orientation/screen_orientation_dispatcher.cc
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/renderer/screen_orientation/screen_orientation_dispatcher.h
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/renderer/screen_orientation/screen_orientation_dispatcher_browsertest.cc
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/content/test/BUILD.gn
[add] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/device/screen_orientation/OWNERS
[add] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/device/screen_orientation/public/interfaces/BUILD.gn
[add] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/device/screen_orientation/public/interfaces/OWNERS
[add] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/device/screen_orientation/public/interfaces/screen_orientation.mojom
[rename] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/device/screen_orientation/public/interfaces/screen_orientation_lock_types.mojom
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/third_party/WebKit/public/BUILD.gn
[add] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/third_party/WebKit/public/platform/modules/screen_orientation/DEPS
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/third_party/WebKit/public/platform/modules/screen_orientation/WebScreenOrientationEnumTraits.h
[modify] https://crrev.com/dfbaf46a8f088f494d65ca9621a912510f3fd10b/third_party/WebKit/public/platform/modules/screen_orientation/screen_orientation_lock_types.typemap

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

commit bd4620dd0a74a6af494b51983903cb09e9aa75cb
Author: leon.han <leon.han@intel.com>
Date: Wed Dec 21 03:45:51 2016

[DeviceService] Mojofy left screen orientation IPC messages.

This CL converts following IPCs into mojo interface:
  ScreenOrientationHostMsg_StartListening
  ScreenOrientationHostMsg_StopListening

BUG=612339
TBR=kinuko@chromium.org

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

[modify] https://crrev.com/bd4620dd0a74a6af494b51983903cb09e9aa75cb/content/browser/BUILD.gn
[modify] https://crrev.com/bd4620dd0a74a6af494b51983903cb09e9aa75cb/content/browser/renderer_host/render_process_host_impl.cc
[add] https://crrev.com/bd4620dd0a74a6af494b51983903cb09e9aa75cb/content/browser/screen_orientation/screen_orientation_listener_android.cc
[add] https://crrev.com/bd4620dd0a74a6af494b51983903cb09e9aa75cb/content/browser/screen_orientation/screen_orientation_listener_android.h
[delete] https://crrev.com/6efd374daca7c10f6a20e5395689303430ba0324/content/browser/screen_orientation/screen_orientation_message_filter_android.cc
[delete] https://crrev.com/6efd374daca7c10f6a20e5395689303430ba0324/content/browser/screen_orientation/screen_orientation_message_filter_android.h
[modify] https://crrev.com/bd4620dd0a74a6af494b51983903cb09e9aa75cb/content/common/screen_orientation_messages.h
[modify] https://crrev.com/bd4620dd0a74a6af494b51983903cb09e9aa75cb/content/renderer/screen_orientation/screen_orientation_observer.cc
[modify] https://crrev.com/bd4620dd0a74a6af494b51983903cb09e9aa75cb/content/renderer/screen_orientation/screen_orientation_observer.h
[modify] https://crrev.com/bd4620dd0a74a6af494b51983903cb09e9aa75cb/device/screen_orientation/public/interfaces/screen_orientation.mojom

Migrating S13N meta bugs to Type=Task, so that they can be distinguished from technical work.

Deprecating Proj-Mustash-Mus-WS label in favor of Components.

The assigned owner "ke.he@intel.com" is not able to receive e-mails, please re-triage.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

***UI Mass Triage ***
Adding labels for expert review.

This seems like something most useful for CrOS? oshima@ Please tirage.

OS sepcific orientation logic is already in ash, so I don't think CrOS is special. What's left now?