Early return when the value of test-justify is distribute, and the length of line is 0

Source/platform/fonts/Character.cpp

Index: Source/platform/fonts/Character.cpp
diff --git a/Source/platform/fonts/Character.cpp b/Source/platform/fonts/Character.cpp
index 9ae36fc83ca78d1ffee41b510d576eb556914cdd..9c9dc6db716ad030a39c9ea8c3eab26cb5164e00 100644
--- a/Source/platform/fonts/Character.cpp
+++ b/Source/platform/fonts/Character.cpp
@@ -343,22 +343,32 @@ unsigned Character::expansionOpportunityCount(const LChar* characters, size_t le
 {
     unsigned count = 0;
     if (textJustify == TextJustifyDistribute) {
-        count = length - 1;
-    } else {
-        if (direction == LTR) {
-            for (size_t i = 0; i < length; ++i) {
-                if (treatAsSpace(characters[i]))
-                    count++;
+        if (length == 0)
+            return 0;
+        int lastCharacter = (direction == LTR) ? length - 1 : 0;
+        isAfterExpansion = treatAsSpace(characters[lastCharacter]);
+        return length - 1;

[kojii, 2014/12/11 08:47:36]

I jumped in from the mid of review discussions without reading past comments, so
ignore me if other reviewers said differently.

It looks like you reverted non-distribute case back to before the patch, I
personally like that.

This "if" is an optimization of distribute case, which is the assumption of
every call to |treatAsSpace()| returns true in the loop below. By thinking that
way, I think this is not a correct optimization. It should rather be:

isAfterExpansion = true;
return length;

Right?

There's also another edge case where |isAfterExpansion| is |false| on the entry.
In this case, you may want to return |length + 1| so that there's an expansion
opportunity before the first character, but the spec doesn't clearly state
whether there should be an expansion opportunity between non-distribute and
distribute. Since the 99% of use cases for distribute is applying to a whole
block, not to part of a block, and this suggestion applies only to such edge
cases, I suggest you ask this to www-style and then make another bug or CL if it
resolved to differently from what we have with this patch.

+    }
+
+    if (direction == LTR) {
+        for (size_t i = 0; i < length; ++i) {
+            if (treatAsSpace(characters[i])) {
+                count++;
+                isAfterExpansion = true;
+            } else {
+                isAfterExpansion = false;
             }
-        } else {
-            for (size_t i = length; i > 0; --i) {
-                if (treatAsSpace(characters[i - 1]))
-                    count++;
+        }
+    } else {
+        for (size_t i = length; i > 0; --i) {
+            if (treatAsSpace(characters[i - 1])) {
+                count++;
+                isAfterExpansion = true;
+            } else {
+                isAfterExpansion = false;
             }
         }
     }
-    int lastCharacter = (direction == LTR) ? length - 1 : 0;
-    isAfterExpansion = treatAsSpace(characters[lastCharacter]);
 
     return count;
 }