Make screen.orientation implementation use WebScreenInfo::orientationType

Source/modules/screen_orientation/ScreenOrientationController.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "modules/screen_orientation/ScreenOrientationController.h"
 
 #include "core/dom/Document.h"
 #include "core/events/Event.h"
 #include "core/frame/DOMWindow.h"
+#include "core/frame/FrameView.h"
+#include "core/frame/LocalFrame.h"
 #include "core/frame/Screen.h"
-#include "modules/screen_orientation/ScreenOrientationDispatcher.h"
+#include "platform/LayoutTestSupport.h"
+#include "platform/PlatformScreen.h"
 
 namespace WebCore {
 
-#if !ENABLE(OILPAN)
 ScreenOrientationController::~ScreenOrientationController()
 {
-    // With oilpan, weak processing removes the controller once it is dead.
-    ScreenOrientationDispatcher::instance().removeController(this);
 }
-#endif
 
 ScreenOrientationController& ScreenOrientationController::from(Document& document)
 {
     ScreenOrientationController* controller = static_cast<ScreenOrientationController*>(DocumentSupplement::from(document, supplementName()));
     if (!controller) {
         controller = new ScreenOrientationController(document);
         DocumentSupplement::provideTo(document, supplementName(), adoptPtrWillBeNoop(controller));
     }
     return *controller;
 }
 
 ScreenOrientationController::ScreenOrientationController(Document& document)
     : m_document(document)
-    , m_orientation(blink::WebScreenOrientationPortraitPrimary)
 {
-    // FIXME: We should listen for screen orientation change events only when the page is visible.
-    ScreenOrientationDispatcher::instance().addController(this);
-}
-
-void ScreenOrientationController::dispatchOrientationChangeEvent()
-{
-    if (m_document.domWindow()
-        && !m_document.activeDOMObjectsAreSuspended()
-        && !m_document.activeDOMObjectsAreStopped())
-        m_document.domWindow()->dispatchEvent(Event::create(EventTypeNames::orientationchange));
 }
 
 const char* ScreenOrientationController::supplementName()
 {
     return "ScreenOrientationController";
 }
 
-void ScreenOrientationController::didChangeScreenOrientation(blink::WebScreenOrientationType orientation)
+// Compute the screen orientation using the orientation angle and the screen width / height.
+blink::WebScreenOrientationType ScreenOrientationController::computeOrientation(FrameView* view)
 {
-    if (orientation == m_orientation)
-        return;
+    // Bypass orientation detection in layout tests to get consistent results.
+    if (isRunningLayoutTest())
+        return blink::WebScreenOrientationPortraitPrimary;

[mlamouri (slow - plz ping), 2014/05/19 14:06:15]

That means we will not get test coverage for this in LayoutTests. Could you
write unit tests then?

[Inactive, 2014/05/19 15:14:30]

The issue is that depending on the machine where the tests are ran, the
orientation would be different and thus the tests could fail.

We cannot easily override the screen dimensions from a layout tests. I will
check if it is easier from a unit test (could this be done in a follow-up?).

[mlamouri (slow - plz ping), 2014/05/20 20:09:25]

Sure, this could be done in a follow-up. We could definitely add something to
override the WebScreenInfo via the test runner. WDYT?

 
-    m_orientation = orientation;
-    dispatchOrientationChangeEvent();
+    FloatRect rect = screenRect(view);
+    uint16_t rotation = screenOrientationAngle(view);
+    bool isTallDisplay = rotation % 180 ? rect.height() < rect.width() : rect.height() > rect.width();

[mlamouri (slow - plz ping), 2014/05/19 14:06:15]

Hmm, I feel like I'm missing something here. Shouldn't that be the opposite?

[Inactive, 2014/05/19 15:14:30]

Well, I tested on an Android phone and it worked so it should be correct.

if (rotation % 180) or (rotation % 180 != 0), it means the rotation is either 90
or 270. Since the device is rotated 90 degrees in either direction, the device
has a tall display if the screen height is less than the width (reversed).

[mlamouri (slow - plz ping), 2014/05/20 20:09:25]

Sorry, my bad. I got confused with (rotation % 180) meaning (rotation != 0 &&
rotation != 180) instead of (rotation == 0 || rotation == 180).

+    switch (rotation) {
+    case 0:
+        return isTallDisplay ? blink::WebScreenOrientationPortraitPrimary : blink::WebScreenOrientationLandscapePrimary;
+    case 90:
+        return isTallDisplay ? blink::WebScreenOrientationLandscapePrimary : blink::WebScreenOrientationPortraitSecondary;

[mlamouri (slow - plz ping), 2014/05/19 14:06:15]

Why did you pick those values here?

[Inactive, 2014/05/19 15:14:30]

Not sure I understand the question. Those are the same values as from
https://www.captechconsulting.com/blog/eric-miles/programmatically-locking-an...
and they seem to be the common ones.

Could you clarify what you think is wrong or ambiguous?

[mlamouri (slow - plz ping), 2014/05/20 20:09:25]

Nothing was wrong, I was wondering what you used to decide which values should
be set for 90 and 270.

+    case 180:
+        return isTallDisplay ? blink::WebScreenOrientationPortraitSecondary : blink::WebScreenOrientationLandscapeSecondary;
+    case 270:
+        return isTallDisplay ? blink::WebScreenOrientationLandscapeSecondary : blink::WebScreenOrientationPortraitPrimary;
+    default:
+        ASSERT_NOT_REACHED();
+        return blink::WebScreenOrientationPortraitPrimary;
+    }
+}
+
+blink::WebScreenOrientationType ScreenOrientationController::orientation() const
+{
+    LocalFrame* mainFrame = m_document.frame();
+    if (!mainFrame)
+        return blink::WebScreenOrientationPortraitPrimary;
+    blink::WebScreenOrientationType orientationType = screenOrientationType(mainFrame->view());
+    if (orientationType == blink::WebScreenOrientationUndefined) {
+        // The embedder could not provide us with an orientation, deduce it ourselves.
+        orientationType = computeOrientation(mainFrame->view());
+    }
+    ASSERT(orientationType != blink::WebScreenOrientationUndefined);
+    return orientationType;
 }
 
 } // namespace WebCore