LayoutBox::scrollRectToVisible doesn't respect overflow:hidden property.

third_party/WebKit/Source/core/layout/LayoutBox.cpp

Index: third_party/WebKit/Source/core/layout/LayoutBox.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBox.cpp b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
index 0272e0ac8552e1f0090d5ad84d5e849afad17c0b..c310ac2346f8c2fdf7649234c7ec754ee27e64d9 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
@@ -503,8 +503,9 @@ static bool isDisallowedAutoscroll(HTMLFrameOwnerElement* ownerElement, FrameVie
     return false;
 }
 
-void LayoutBox::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignment& alignX, const ScrollAlignment& alignY)
+void LayoutBox::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignment& alignX, const ScrollAlignment& alignY, ScrollType scrollType)
 {
+    ASSERT(scrollType == ProgrammaticScroll || scrollType == UserScroll);
     // Presumably the same issue as in setScrollTop. See crbug.com/343132.
     DisableCompositingQueryAsserts disabler;
 
@@ -520,12 +521,12 @@ void LayoutBox::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignmen
     if (hasOverflowClip() && !restrictedByLineClamp) {
         // Don't scroll to reveal an overflow layer that is restricted by the -webkit-line-clamp property.
         // This will prevent us from revealing text hidden by the slider in Safari RSS.
-        newRect = layer()->scrollableArea()->scrollIntoView(rect, alignX, alignY);
+        newRect = layer()->scrollableArea()->scrollIntoView(rect, alignX, alignY, scrollType);
     } else if (!parentBox && canBeProgramaticallyScrolled()) {

[bokan, 2015/09/29 19:42:49]

I suspect we can remove the canBeProgrammaticallyScrolled condition. If we have
no parentBox then I think the node will be the documentNode so it will always
return true anyway. 

[ymalik, 2015/09/30 00:41:09]

I would like to keep this CL just to propagate the ScrollType change along with
fixing the overflow bug. Removing the call to canBeProgramaticallyScrolled in
this CL may make it hard to point to the change in case there are bugs. Will
remove it in follow-up CL.

[bokan, 2015/09/30 12:52:39]

sgtm

[ymalik, 2015/09/30 14:16:58]

Acknowledged.

         if (FrameView* frameView = this->frameView()) {
             HTMLFrameOwnerElement* ownerElement = document().ownerElement();
             if (!isDisallowedAutoscroll(ownerElement, frameView)) {

[bokan, 2015/09/29 19:42:49]

So many special cases...

[ymalik, 2015/09/30 00:41:09]

Indeed. I will see if I can clean it up in a followup CL.

-                frameView->scrollableArea()->scrollIntoView(rect, alignX, alignY);
+                frameView->scrollableArea()->scrollIntoView(rect, alignX, alignY, scrollType);
 
                 if (ownerElement && ownerElement->layoutObject()) {
                     if (frameView->safeToPropagateScrollToParent()) {
@@ -550,7 +551,7 @@ void LayoutBox::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignmen
         parentBox = enclosingScrollableBox();
 
     if (parentBox)
-        parentBox->scrollRectToVisible(newRect, alignX, alignY);
+        parentBox->scrollRectToVisible(newRect, alignX, alignY, scrollType);
 }
 
 void LayoutBox::absoluteRects(Vector<IntRect>& rects, const LayoutPoint& accumulatedOffset) const
@@ -773,7 +774,7 @@ void LayoutBox::autoscroll(const IntPoint& positionInRootFrame)
         return;
 
     IntPoint positionInContent = frameView->rootFrameToContents(positionInRootFrame);
-    scrollRectToVisible(LayoutRect(positionInContent, LayoutSize(1, 1)), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded);
+    scrollRectToVisible(LayoutRect(positionInContent, LayoutSize(1, 1)), ScrollAlignment::alignToEdgeIfNeeded, ScrollAlignment::alignToEdgeIfNeeded, UserScroll);
 }
 
 // There are two kinds of layoutObject that can autoscroll.