Bug 1388230: Make nsColorControlFrame::UpdateColor() a no-op if value is empty (which implies our element is still being appended). r=jwatt

nsColorControlFrame::UpdateColor() looks up the color value from the
corresponding <input> element -- and it expects to receive a valid color
string, regardless of what the user/author has done (or whether they've done
anything), thanks to the HTMLInputElement sanitization code that gets run when
the value is set.

As a basic sanity-check, UpdateColor() has an assertion to verify that the
value it receives is non-empty. However, if it happens to be called while the
element is still being appended (e.g. due to greedy frame construction), then
it *can* legitimately get an empty value.  So, the assertion isn't entirely valid!

Hence, this patch relaxes the assertion to only take effect after the frame has
been reflowed, and it also makes UpdateColor() a no-op in that case.  This
is fine because we can expect that UpdateColor() will be called again (and will
see a non-empty color value at that point) before the frame gets
reflowed/painted.

MozReview-Commit-ID: LOymuwy6gIM

--HG--
extra : rebase_source : 57a2b58cc371ec82f1d2db0334b2236a33056e40

layout/forms/crashtests/1388230-1.html

@@ -0,0 +1,3 @@
+<cite contenteditable='true'>
+  <input type='color'/>
+</cite>

layout/forms/crashtests/1388230-2.html

@@ -0,0 +1 @@
+<input contenteditable='true' type='color'>

layout/forms/crashtests/crashtests.list

@@ -67,3 +67,5 @@ load 1182414.html
 load 1212688.html
 load 1228670.xhtml
 load 1279354.html
+load 1388230-1.html
+load 1388230-2.html

layout/forms/nsColorControlFrame.cpp

@@ -90,17 +90,31 @@ nsresult
 nsColorControlFrame::UpdateColor()
 {
   // Get the color from the "value" property of our content; it will return the
-  // default color (through the sanitization algorithm) if there is none.
+  // default color (through the sanitization algorithm) if the value is empty.
   nsAutoString color;
   HTMLInputElement* elt = HTMLInputElement::FromContent(mContent);
   elt->GetValue(color, CallerType::System);
-  MOZ_ASSERT(!color.IsEmpty(),
-             "Content node's GetValue() should return a valid color string "
-             "(the default color, in case no valid color is set)");
 
-  // Set the background-color style property of the swatch element to this color
+  if (color.IsEmpty()) {
+    // OK, there is one case the color string might be empty -- if our content
+    // is still being created, i.e. if it has mDoneCreating==false.  In that
+    // case, we simply do nothing, because we'll be called again with a complete
+    // content node before we ever reflow or paint. Specifically: we can expect
+    // that HTMLInputElement::DoneCreatingElement() will set mDoneCreating to
+    // true (which enables sanitization) and then it'll call SetValueInternal(),
+    // which produces a nonempty color (via sanitization), and then it'll call
+    // this function here, and we'll get the nonempty default color.
+    MOZ_ASSERT(HasAnyStateBits(NS_FRAME_FIRST_REFLOW),
+               "Content node's GetValue() should return a valid color string "
+               "by the time we've been reflowed (the default color, in case "
+               "no valid color is set)");
+    return NS_OK;
+  }
+
+  // Set the background-color CSS property of the swatch element to this color.
   return mColorContent->SetAttr(kNameSpaceID_None, nsGkAtoms::style,
-      NS_LITERAL_STRING("background-color:") + color, true);
+                                NS_LITERAL_STRING("background-color:") + color,
+                                /* aNotify */ true);
 }
 
 nsresult