Make screen.orientation implementation use WebScreenInfo::orientationType

Source/modules/screen_orientation/ScreenOrientationController.cpp

Index: Source/modules/screen_orientation/ScreenOrientationController.cpp
diff --git a/Source/modules/screen_orientation/ScreenOrientationController.cpp b/Source/modules/screen_orientation/ScreenOrientationController.cpp
index 86066d5bf105db2b270a47f0552161984baeb544..60141480e94b50f92262bc0bedf9bfb30444a67b 100644
--- a/Source/modules/screen_orientation/ScreenOrientationController.cpp
+++ b/Source/modules/screen_orientation/ScreenOrientationController.cpp
@@ -8,18 +8,17 @@
 #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)
 {
@@ -33,18 +32,7 @@ ScreenOrientationController& ScreenOrientationController::from(Document& documen
 
 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()
@@ -52,13 +40,43 @@ 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?

+
+    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;
+    }
+}
 
-    m_orientation = orientation;
-    dispatchOrientationChangeEvent();
+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