Update Window API for CSSOM smooth scrolling to match the spec

Source/core/frame/LocalDOMWindow.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 50 matching lines @@
 #include "core/frame/DOMWindowLifecycleNotifier.h"
 #include "core/frame/EventHandlerRegistry.h"
 #include "core/frame/FrameConsole.h"
 #include "core/frame/FrameHost.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/History.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Location.h"
 #include "core/frame/Navigator.h"
 #include "core/frame/Screen.h"
-#include "core/frame/ScrollOptions.h"
+#include "core/frame/ScrollToOptions.h"
 #include "core/frame/Settings.h"
 #include "core/html/HTMLFrameOwnerElement.h"
 #include "core/inspector/ConsoleMessage.h"
 #include "core/inspector/ConsoleMessageStorage.h"
 #include "core/inspector/InspectorInstrumentation.h"
 #include "core/inspector/InspectorTraceEvents.h"
 #include "core/inspector/ScriptCallStack.h"
 #include "core/loader/DocumentLoader.h"
 #include "core/loader/FrameLoadRequest.h"
 #include "core/loader/FrameLoader.h"
@@ skipping 1333 matching lines @@
 }
 
 double LocalDOMWindow::devicePixelRatio() const
 {
     if (!frame())
         return 0.0;
 
     return frame()->devicePixelRatio();
 }
 
-static bool scrollBehaviorFromScrollOptions(const ScrollOptions& scrollOptions, ScrollBehavior& scrollBehavior, ExceptionState& exceptionState)
-{
-    if (!scrollOptions.hasBehavior()) {
-        scrollBehavior = ScrollBehaviorAuto;
-        return true;
-    }
-
-    if (ScrollableArea::scrollBehaviorFromString(scrollOptions.behavior(), scrollBehavior))
-        return true;
-
-    exceptionState.throwTypeError("The ScrollBehavior provided is invalid.");

[ajuma, 2014/12/03 19:11:50]

We get this error checking for free now that we're using an enum rather than a
string for ScrollBehavior in ScrollOptions.idl.

-    return false;
-}
-
 // FIXME: This class shouldn't be explicitly moving the viewport around. crbug.com/371896
 static void scrollViewportTo(LocalFrame* frame, DoublePoint offset, ScrollBehavior scrollBehavior)
 {
     FrameView* view = frame->view();
     if (!view)
         return;
 
     FrameHost* host = frame->host();
     if (!host)
         return;
@@ skipping 26 matching lines @@
         return;
 
     DoublePoint currentOffset = host->settings().pinchVirtualViewportEnabled() && frame()->isMainFrame()
         ? DoublePoint(host->pinchViewport().visibleRectInDocument().location())
         : view->scrollPositionDouble();
 
     DoubleSize scaledOffset(x * frame()->pageZoomFactor(), y * frame()->pageZoomFactor());
     scrollViewportTo(frame(), currentOffset + scaledOffset, scrollBehavior);
 }
 
-void LocalDOMWindow::scrollBy(double x, double y, const ScrollOptions& scrollOptions, ExceptionState &exceptionState) const
+void LocalDOMWindow::scrollBy(const ScrollToOptions& scrollToOptions) const
 {
+    double x = 0.0;
+    double y = 0.0;
+    if (scrollToOptions.hasLeft())
+        x = scrollToOptions.left();
+    if (scrollToOptions.hasTop())
+        y = scrollToOptions.top();
     ScrollBehavior scrollBehavior = ScrollBehaviorAuto;
-    if (!scrollBehaviorFromScrollOptions(scrollOptions, scrollBehavior, exceptionState))
-        return;
+    ScrollableArea::scrollBehaviorFromString(scrollToOptions.behavior(), scrollBehavior);
     scrollBy(x, y, scrollBehavior);
 }
 
-void LocalDOMWindow::scrollTo(double x, double y, ScrollBehavior scrollBehavior) const
+void LocalDOMWindow::scrollTo(double x, double y) const
 {
     if (!isCurrentlyDisplayedInFrame())
         return;
 
     document()->updateLayoutIgnorePendingStylesheets();
 
     if (std::isnan(x) || std::isnan(y))
         return;
 
     DoublePoint layoutPos(x * frame()->pageZoomFactor(), y * frame()->pageZoomFactor());
-    scrollViewportTo(frame(), layoutPos, scrollBehavior);
+    scrollViewportTo(frame(), layoutPos, ScrollBehaviorAuto);
 }
 
-void LocalDOMWindow::scrollTo(double x, double y, const ScrollOptions& scrollOptions, ExceptionState& exceptionState) const
+void LocalDOMWindow::scrollTo(const ScrollToOptions& scrollToOptions) const
 {
+    if (!isCurrentlyDisplayedInFrame())
+        return;
+
+    document()->updateLayoutIgnorePendingStylesheets();
+
+    FrameView* view = frame()->view();
+    if (!view)
+        return;
+
+    FrameHost* host = frame()->host();
+    if (!host)
+        return;
+
+    double scaledX = 0.0;
+    double scaledY = 0.0;
+
+    DoublePoint currentOffset = host->settings().pinchVirtualViewportEnabled() && frame()->isMainFrame()
+        ? DoublePoint(host->pinchViewport().visibleRectInDocument().location())
+        : view->scrollPositionDouble();
+    scaledX = currentOffset.x();
+    scaledY = currentOffset.y();

[Ian Vollick, 2014/12/03 20:20:55]

There appears to be a real logic change here. Is there test coverage?

[ajuma, 2014/12/03 21:02:01]

Good point. Added test cases where the dictionary is missing left, top, or both.

+
+    if (scrollToOptions.hasLeft()) {
+        if (std::isnan(scrollToOptions.left()))
+            return;
+        scaledX = scrollToOptions.left() * frame()->pageZoomFactor();
+    }
+
+    if (scrollToOptions.hasTop()) {
+        if (std::isnan(scrollToOptions.top()))
+            return;
+        scaledY = scrollToOptions.top() * frame()->pageZoomFactor();
+    }
+
     ScrollBehavior scrollBehavior = ScrollBehaviorAuto;
-    if (!scrollBehaviorFromScrollOptions(scrollOptions, scrollBehavior, exceptionState))
-        return;
-    scrollTo(x, y, scrollBehavior);
+    ScrollableArea::scrollBehaviorFromString(scrollToOptions.behavior(), scrollBehavior);
+    scrollViewportTo(frame(), DoublePoint(scaledX, scaledY), scrollBehavior);
 }
 
 void LocalDOMWindow::moveBy(float x, float y) const
 {
     if (!frame() || !frame()->isMainFrame())
         return;
 
     FrameHost* host = frame()->host();
     if (!host)
         return;
@@ skipping 500 matching lines @@
     return m_frameObserver->frame();
 }
 
 v8::Handle<v8::Object> LocalDOMWindow::wrap(v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
 {
     ASSERT_NOT_REACHED(); // LocalDOMWindow has [Custom=ToV8].
     return v8::Handle<v8::Object>();
 }
 
 } // namespace blink