Make it possible to set the display mode from the WebView API

Make it possible to set the display mode from the WebView API

Added a unit test to make sure it works as advertised.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188960

[kenneth.r.christiansen, 2015-01-23 10:29:55]

kenneth.r.christiansen@intel.com changed reviewers:
+ mkwst@chromium.org, yoav@yoav.ws

[Mike West, 2015-01-23 11:15:38]

At first glance, this seems like a reasonable approach to do the thing you want
to do. It's not clear to me, however, why you want to do it. Can you point me to
a bug or design doc or something that explains the problem so I can understand
why this is the right solution?

[kenneth.r.christiansen, 2015-01-23 11:27:59]

On 2015/01/23 at 11:15:38, mkwst wrote:
> At first glance, this seems like a reasonable approach to do the thing you
want to do. It's not clear to me, however, why you want to do it. Can you point
me to a bug or design doc or something that explains the problem so I can
understand why this is the right solution?

The intent is here
https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/intent$20...

That should explain the use-case. Ie. people need to be able to set this in
certain cases, like when launching apps in separate windows.

This might not be the right approach :-) like maybe it should be a
WebPreferences instead when being exposed in content/

[kenneth.r.christiansen, 2015-01-23 13:22:40]

So I researched this a bit more, like how the default orientation lock is set on
Android - which really wasn´t easy to find :-)

It transfers the default orientation as part of the launch intent:

 11 public class ScreenOrientationConstants {
 12     // Intent's bundle's key for the default orientation field.
 13     public static final String EXTRA_ORIENTATION =
 14             "org.chromium.content_public.common.orientation";
 15
 16     private ScreenOrientationConstants() {
 17     }
 18 }

which is then gets when calling unlock (as you always unlock to the default set
of allowed orientations)

 56 void ScreenOrientationDelegateAndroid::Unlock(WebContents* web_contents) {
 57   Java_ScreenOrientationProvider_unlockOrientation(
 58       base::android::AttachCurrentThread());
 59 }

 82     @CalledByNative
 83     static void unlockOrientation() {
 84         Activity activity =
ApplicationStatus.getLastTrackedFocusedActivity();
 85         if (activity == null) {
 86             return;
 87         }
 88 
 89         int defaultOrientation =
ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED;
 90 
 91         // Activities opened from a shortcut may have EXTRA_ORIENTATION set.
In
 92         // which case, we want to use that as the default orientation.
 93         int orientation = activity.getIntent().getIntExtra(
 94                 ScreenOrientationConstants.EXTRA_ORIENTATION,
 95                 ScreenOrientationValues.DEFAULT);
 96         defaultOrientation = getOrientationFromWebScreenOrientations(
 97                 (byte) orientation, activity);

Transfering the display mode in a similar way is fine, but the question is more
how that should be propagates to Blink.

I cannot really see how the android shortcut works (Android UI is closed source)
but I assume that it sets the orientation as it launches the app, thus no need
to propagate the first lock to Blink. It is only needed when unlock is called
from JS.

In my case I need a way for Android (Chrome OS etc) to set the desktop mode when
creating the window. I think a setDisplayMode method should do fine, but where
should I add that to the content/ WebContents? RenderView? and how should I call
into Blink from there..

[kenneth.r.christiansen, 2015-01-23 14:45:06]

So the way I see this, after digging some more, I can send the display mode
along with ViewMsg_Resize_Params which already has fullscreen (which can be
integrated in the future).

That is received by the RenderWidget::OnResize (from which RenderViewImpl
inherits). RenderViewImpl have access to blink via the blink::WebView* webview()
const call, which ties into the patch in this CL.

As far as I see that is the proper way to change the value in blink, as it is
really view related and presumable can change in the future (ie, when ChromeOS
might turns whole apps into fullscreen without using the fullscreen API - which
is for fullscreening elements) then the value could also change (it is a resize)

[kenneth.r.christiansen, 2015-01-23 14:58:21]

Another question Mike,

It seems that Chromium uses blink enums in the API, so I could use the enum in
the blink::WebView API as well, but then I would like to know where I should
place the enum definition in Blink (right now it is in core/css)

I guess public/platform/WebDisplayMode.h or similar. That way I should also be
able to use it in Chromium like blink::WebScreenOrientationType is used.

[Yoav Weiss, 2015-01-26 09:24:30]

LGTM for the core parts. Regarding the web parts, as you said, passing the enum
all the way up to the Chrome side would be better, but I'm not sure what the
ideal location for it would be.

On 2015/01/23 11:15:38, Mike West wrote:
> At first glance, this seems like a reasonable approach to do the thing you
want
> to do. It's not clear to me, however, why you want to do it. Can you point me
to
> a bug or design doc or something that explains the problem so I can understand
> why this is the right solution?

Mike, did the "intent to ship" Kenneth linked to explain the problem?

[Mike West, 2015-01-26 10:00:15]

Yes Kenneth's links explained the rationale, thanks. LGTM % the enum work.

On 2015/01/23 14:58:21, kenneth.r.christiansen wrote:
> I guess public/platform/WebDisplayMode.h or similar. That way I should also be
> able to use it in Chromium like blink::WebScreenOrientationType is used.

That'd be a fine place for an enum that you use on both sides of the platform
divide.

[kenneth.christiansen, 2015-01-26 14:40:23]

OK, using enum now

[Yoav Weiss, 2015-01-26 14:44:44]

On 2015/01/26 14:40:23, kenneth.christiansen wrote:
> OK, using enum now

re-LGTM for the core parts

[kenneth.r.christiansen, 2015-01-26 14:56:03]

The CQ bit was checked by kenneth.r.christiansen@intel.com

[commit-bot: I haz the power, 2015-01-26 14:57:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/870933002/20001

[commit-bot: I haz the power, 2015-01-26 15:49:35]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=188960