Textarea resize-able only to larger; min-height and min-width properly set

Source/core/rendering/RenderLayerScrollableArea.cpp

Index: Source/core/rendering/RenderLayerScrollableArea.cpp
diff --git a/Source/core/rendering/RenderLayerScrollableArea.cpp b/Source/core/rendering/RenderLayerScrollableArea.cpp
index 4189864f02f153e1a78d1824c7d0e8c0a4c325ce..6609083fbc05adb987615f9b18ed5c4395dbdff5 100644
--- a/Source/core/rendering/RenderLayerScrollableArea.cpp
+++ b/Source/core/rendering/RenderLayerScrollableArea.cpp
@@ -74,6 +74,11 @@ namespace WebCore {
 
 const int ResizerControlExpandRatioForTouch = 2;
 
+// Default value is set to 15 as the default
+// minimum size used by firefox is 15x15.
+static const LayoutUnit defaultMinimumValueForResizing(15);
+static const LayoutSize defaultMinimumSizeForResizing(defaultMinimumValueForResizing, defaultMinimumValueForResizing);

[Julien - ping for review, 2014/04/28 17:07:29]

What I meant by 2 variables is minWidth and minHeight, instead of bundling the 2
together.

We can keep the internal logic to deal with LayoutSize though.

[harpreet.sk, 2014/04/29 08:57:43]

Added the variables defaultMinimumWidthForResizing and
defaultMinimumHeightForResizing.  

+
 RenderLayerScrollableArea::RenderLayerScrollableArea(RenderBox* box)
     : m_box(box)
     , m_inResizeMode(false)
@@ -82,6 +87,7 @@ RenderLayerScrollableArea::RenderLayerScrollableArea(RenderBox* box)
     , m_inOverflowRelayout(false)
     , m_needsCompositedScrolling(false)
     , m_forceNeedsCompositedScrolling(DoNotForceCompositedScrolling)
+    , m_minimumSizeForResizing(defaultMinimumSizeForResizing)
     , m_scrollCorner(0)
     , m_resizer(0)
 {
@@ -715,6 +721,21 @@ static bool overflowDefinesAutomaticScrollbar(EOverflow overflow)
     return overflow == OAUTO || overflow == OOVERLAY;
 }
 
+void RenderLayerScrollableArea::updateMinimumSizeForResizing()
+{
+    LayoutUnit minimumWidth;
+    LayoutUnit minimumHeight;
+
+    if (m_box->containingBlock()) {
+        minimumWidth = valueForLength(m_box->style()->logicalMinWidth(), m_box->containingBlock()->logicalWidth());
+        minimumHeight = valueForLength(m_box->style()->logicalMinHeight(), m_box->containingBlock()->logicalHeight());

[Julien - ping for review, 2014/04/28 17:07:29]

This should be in physical coordinates as m_minimumSizeForResizing is. FWIW the
current logic is confused as it uses LayoutSize (fractional pixel unit) and not
physical pixels. I think we should just make this logic consistent and use
integer / IntSize, not LayoutUnit / LayoutSize.

To ensure the mark, we should also add an orthogonal flow test.

[harpreet.sk, 2014/04/29 08:57:43]

Now using int for minimumWidth and minimumHeight and the api will return
IntSize. 

Also the orthogonal flow test has been added.

+    }
+
+    minimumWidth = std::max(minimumWidth, defaultMinimumSizeForResizing.width());
+    minimumHeight = std::max(minimumHeight, defaultMinimumSizeForResizing.height());

[Julien - ping for review, 2014/04/28 17:07:30]

I think this code would be better if you initialized |minimumWidth| and
|minimumHeight| to these minimum and then do the std::max if you have a
containing block (when do we not have a containing block btw?)

[harpreet.sk, 2014/04/29 08:57:43]

Actually i used containing block check earlier because browser was getting
crashed just on launch maybe because for some element containing block does not
exist. Now i had remove this containing block check as now the api
minimumSizeForResizing will be called in the resize api where we want to use
minimum size for resizing and not in the updateStyleChange api. It works fine
for all cases.  

+    m_minimumSizeForResizing = LayoutSize(minimumWidth, minimumHeight);
+}
+
 void RenderLayerScrollableArea::updateAfterStyleChange(const RenderStyle* oldStyle)
 {
     // List box parts handle the scrollbars by themselves so we have nothing to do.
@@ -731,6 +752,8 @@ void RenderLayerScrollableArea::updateAfterStyleChange(const RenderStyle* oldSty
     EOverflow overflowX = m_box->style()->overflowX();
     EOverflow overflowY = m_box->style()->overflowY();
 
+    updateMinimumSizeForResizing();
+
     // To avoid doing a relayout in updateScrollbarsAfterLayout, we try to keep any automatic scrollbar that was already present.
     bool needsHorizontalScrollbar = (hasHorizontalScrollbar() && overflowDefinesAutomaticScrollbar(overflowX)) || overflowRequiresScrollbar(overflowX);
     bool needsVerticalScrollbar = (hasVerticalScrollbar() && overflowDefinesAutomaticScrollbar(overflowY)) || overflowRequiresScrollbar(overflowY);
@@ -1353,8 +1376,6 @@ void RenderLayerScrollableArea::resize(const PlatformEvent& evt, const LayoutSiz
     newOffset.setHeight(newOffset.height() / zoomFactor);
 
     LayoutSize currentSize = LayoutSize(m_box->width() / zoomFactor, m_box->height() / zoomFactor);
-    LayoutSize minimumSize = element->minimumSizeForResizing().shrunkTo(currentSize);
-    element->setMinimumSizeForResizing(minimumSize);
 
     LayoutSize adjustedOldOffset = LayoutSize(oldOffset.width() / zoomFactor, oldOffset.height() / zoomFactor);
     if (m_box->style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft()) {
@@ -1362,7 +1383,7 @@ void RenderLayerScrollableArea::resize(const PlatformEvent& evt, const LayoutSiz
         adjustedOldOffset.setWidth(-adjustedOldOffset.width());
     }
 
-    LayoutSize difference = (currentSize + newOffset - adjustedOldOffset).expandedTo(minimumSize) - currentSize;
+    LayoutSize difference = (currentSize + newOffset - adjustedOldOffset).expandedTo(m_minimumSizeForResizing) - currentSize;

[Julien - ping for review, 2014/04/28 17:07:30]

We should probably not cache the value as resize() is a rare operation compared
to style changes.

That's why I would just change updateMinimumSizeForResizing() to
minimumSizeForResizing() and make it return the IntSize.

[harpreet.sk, 2014/04/29 08:57:43]

The api is changed to minimumSizeForResizing() and it return IntSize.

 
     bool isBoxSizingBorder = m_box->style()->boxSizing() == BORDER_BOX;