Implement canvas2d direction attribute

Source/core/html/canvas/CanvasRenderingContext2D.cpp

Index: Source/core/html/canvas/CanvasRenderingContext2D.cpp
diff --git a/Source/core/html/canvas/CanvasRenderingContext2D.cpp b/Source/core/html/canvas/CanvasRenderingContext2D.cpp
index 4eea12475e641bce52e6482f6efa3b73d6060b7e..ec957298f75b2a7151016ecd635fa41173db9087 100644
--- a/Source/core/html/canvas/CanvasRenderingContext2D.cpp
+++ b/Source/core/html/canvas/CanvasRenderingContext2D.cpp
@@ -257,6 +257,7 @@ CanvasRenderingContext2D::State::State()
     , m_imageSmoothingEnabled(true)
     , m_textAlign(StartTextAlign)
     , m_textBaseline(AlphabeticTextBaseline)
+    , m_direction(Inherit)
     , m_unparsedFont(defaultFont)
     , m_realizedFont(false)
     , m_hasClip(false)
@@ -286,6 +287,7 @@ CanvasRenderingContext2D::State::State(const State& other)
     , m_imageSmoothingEnabled(other.m_imageSmoothingEnabled)
     , m_textAlign(other.m_textAlign)
     , m_textBaseline(other.m_textBaseline)
+    , m_direction(other.m_direction)
     , m_unparsedFont(other.m_unparsedFont)
     , m_font(other.m_font)
     , m_realizedFont(other.m_realizedFont)
@@ -325,6 +327,7 @@ CanvasRenderingContext2D::State& CanvasRenderingContext2D::State::operator=(cons
     m_imageSmoothingEnabled = other.m_imageSmoothingEnabled;
     m_textAlign = other.m_textAlign;
     m_textBaseline = other.m_textBaseline;
+    m_direction = other.m_direction;
     m_unparsedFont = other.m_unparsedFont;
     m_font = other.m_font;
     m_realizedFont = other.m_realizedFont;
@@ -2016,6 +2019,45 @@ void CanvasRenderingContext2D::setTextBaseline(const String& s)
     modifiableState().m_textBaseline = baseline;
 }
 
+TextDirection CanvasRenderingContext2D::toTextDirection(Direction direction, RenderStyle*& computedStyle, bool updateRenderTree) const

[Stephen White, 2014/08/22 17:45:35]

I'm not fond of non-const references, although they are not banned by the blink
style guide. Would you mind using a RenderStyle** instead, for clarity at the
call site?

[Justin Novosad, 2014/08/22 18:15:02]

Does Blink really need to have an additional enum (Direction)? RTL vs. Rtl is a
bit unclear. I mean blink already has enough enums for this: WritingDirection,
TextDirection, WebTextDirection.

[vivekg, 2014/08/26 11:31:52]

We wanted to be explicit with "inherit" hence ended up adding the Direction enum
which is only visible to CanvasRenderingContext2D being private.

[vivekg, 2014/08/26 11:31:52]

Done.

+{
+    if (updateRenderTree)
+        canvas()->document().updateRenderTreeIfNeeded();
+    computedStyle = canvas()->computedStyle();
+    switch (direction) {
+    case Inherit:
+        return computedStyle ? computedStyle->direction() : LTR;
+    case Rtl:
+        return RTL;
+    case Ltr:
+        return LTR;
+    }
+    ASSERT_NOT_REACHED();
+    return LTR;
+}
+
+String CanvasRenderingContext2D::direction() const
+{
+    RenderStyle* computedStyle = nullptr;
+    return toTextDirection(state().m_direction, computedStyle) == RTL ? "rtl" : "ltr";
+}
+
+void CanvasRenderingContext2D::setDirection(const String& directionString)
+{
+    Direction direction;
+    if (directionString == "inherit")
+        direction = Inherit;
+    else if (directionString == "rtl")

[Justin Novosad, 2014/08/22 18:15:02]

This literal is in more than one place. A constant would be appropriate.

[vivekg, 2014/08/26 11:31:52]

Done.

+        direction = Rtl;
+    else if (directionString == "ltr")
+        direction = Ltr;
+    else
+        return;
+
+    realizeSaves();
+    modifiableState().m_direction = direction;
+}
+
 void CanvasRenderingContext2D::fillText(const String& text, float x, float y)
 {
     drawTextInternal(text, x, y, true);
@@ -2142,8 +2184,8 @@ void CanvasRenderingContext2D::drawTextInternal(const String& text, float x, flo
 
     // FIXME: Need to turn off font smoothing.
 
-    RenderStyle* computedStyle = canvas()->computedStyle();
-    TextDirection direction = computedStyle ? computedStyle->direction() : LTR;
+    RenderStyle* computedStyle = nullptr;
+    TextDirection direction = toTextDirection(state().m_direction, computedStyle, false);

[Stephen White, 2014/08/22 17:45:35]

Passing a boolean constant is not self-documenting: "Prefer enums to bools on
function parameters if callers are likely to be passing constants, since named
constants are easier to read at the call site."

Could you just get rid of this argument altogether, and put the code at the call
site where it's needed?

[vivekg, 2014/08/26 11:31:52]

Done.

     bool isRTL = direction == RTL;
     bool override = computedStyle ? isOverride(computedStyle->unicodeBidi()) : false;