Introduce BorderImageLength and BorderImageLengthBox

Source/platform/LengthOrNumberBox.h

+/*
+    Copyright (c) 2013, Opera Software ASA. All rights reserved.
+
+    This library is free software; you can redistribute it and/or
+    modify it under the terms of the GNU Library General Public
+    License as published by the Free Software Foundation; either
+    version 2 of the License, or (at your option) any later version.
+
+    This library is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+    Library General Public License for more details.
+
+    You should have received a copy of the GNU Library General Public License
+    along with this library; see the file COPYING.LIB.  If not, write to
+    the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+    Boston, MA 02110-1301, USA.
+*/

[Julien - ping for review, 2013/11/01 18:27:11]

Same comment about the license.

[davve, 2013/11/04 12:42:54]

OK.

+
+#ifndef LengthOrNumberBox_h
+#define LengthOrNumberBox_h
+
+#include "platform/LengthBox.h"
+#include "platform/LengthOrNumber.h"
+
+namespace WebCore {
+
+class PLATFORM_EXPORT LengthOrNumberBox {

[Julien - ping for review, 2013/11/01 18:27:11]

While the name describe what it is, I think it could be improved.

There are only 2 users of this class and both mean "multiple of the
border-width" by number so I think we should just narrow the scope of this class
and make this explicit. Not so great name but I couldn't come up with better:
LengthOrMultipleOfBorderWidthBox.

[davve, 2013/11/04 09:55:37]

Hm. If we rename the box, I think we should also rename what the box contains.
Given your suggestion, we would also rename LengthOrNumber to
LengthOrMultipleOfBorderWidth.

But as you rightly point out, it isn't a great name. So perhaps we shouldn't
name it after what it contains, but after what it is: a length in border-image
land. So my new suggestion would be BorderImageLength and BorderImageLengthBox.

Side note: One of the reasons I went with LengthOrNumber was that GridLength and
other similar instances in the spec could share datatype, but I'm not sure it is
worth it. If the pattern gets more common in the future, perhaps we could make
it a shared type then (or a shared base class).

+public:
+    LengthOrNumberBox()
+    {
+    }
+
+    LengthOrNumberBox(Length t, Length r, Length b, Length l)
+        : m_left(LengthOrNumber(l))
+        , m_right(LengthOrNumber(r))
+        , m_top(LengthOrNumber(t))
+        , m_bottom(LengthOrNumber(b))
+    {
+    }
+
+    LengthOrNumberBox(double t, double r, double b, double l)
+        : m_left(LengthOrNumber(l))
+        , m_right(LengthOrNumber(r))
+        , m_top(LengthOrNumber(t))
+        , m_bottom(LengthOrNumber(b))
+    {
+    }
+
+    explicit LengthOrNumberBox(LengthBox lengthBox) :

[Julien - ping for review, 2013/11/01 18:27:11]

Should we have a FIXME about removing this constructor. Also style violation,
the colon should be on the next line before m_left and the commas should be
aligned with the colon (like you did above)

[davve, 2013/11/04 12:42:54]

OK.

+        m_left(lengthBox.m_left),
+        m_right(lengthBox.m_right),
+        m_top(lengthBox.m_top),
+        m_bottom(lengthBox.m_bottom)
+    {
+    }
+
+    LengthOrNumber left() const { return m_left; }

[Julien - ping for review, 2013/11/01 18:27:11]

const LengthOrNumber& to avoid a copy if we want?

[davve, 2013/11/04 12:42:54]

OK.

+    LengthOrNumber right() const { return m_right; }
+    LengthOrNumber top() const { return m_top; }
+    LengthOrNumber bottom() const { return m_bottom; }
+
+    void setLeft(const LengthOrNumber& o) { m_left = o; }

[Julien - ping for review, 2013/11/01 18:27:11]

|o| is never a good name, let's use |left|...

[davve, 2013/11/04 12:42:54]

OK.

+    void setRight(const LengthOrNumber& o) { m_right = o; }
+    void setTop(const LengthOrNumber& o) { m_top = o; }
+    void setBottom(const LengthOrNumber& o) { m_bottom = o; }
+
+    bool operator==(const LengthOrNumberBox& o) const

[Julien - ping for review, 2013/11/01 18:27:11]

OK, I lied, |o| is fine here (though |other| would be even more fine)

[davve, 2013/11/04 12:42:54]

:) OK. (I like |other|)

+    {
+        return m_left == o.m_left && m_right == o.m_right && m_top == o.m_top && m_bottom == o.m_bottom;
+    }
+
+    bool operator!=(const LengthOrNumberBox& o) const
+    {
+        return !(*this == o);
+    }
+
+    bool nonZero() const
+    {
+        return !(m_left.isZero() && m_right.isZero() && m_top.isZero() && m_bottom.isZero());
+    }
+
+    LengthBox lengthBox() const

[Julien - ping for review, 2013/11/01 18:27:11]

Should we have a FIXME to remove this explicit conversion operator? Also we
should name it deprecatedLengthBox if we intent to remove it.

[davve, 2013/11/04 12:42:54]

OK on both points.

+    {
+        return LengthBox(m_top.unifiedLength(), m_right.unifiedLength(),
+            m_bottom.unifiedLength(), m_left.unifiedLength());
+    }
+
+private:
+    LengthOrNumber m_left;
+    LengthOrNumber m_right;
+    LengthOrNumber m_top;
+    LengthOrNumber m_bottom;
+};
+
+} // namespace WebCore
+
+#endif // LengthOrNumberBox_h
+
+

[Julien - ping for review, 2013/11/01 18:27:11]

Trailing empty lines.

[davve, 2013/11/04 12:42:54]

Thanks.