Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
138483004:
Improve test coverage of TreatNullAs=NullString and TreatUndefinedAs=NullString (Closed)
|
Created:
6 years, 11 months ago by philipj_slow Modified:
6 years, 11 months ago CC:
blink-reviews, kojih, arv+blink, jsbell+bindings_chromium.org, sof, abarth-chromium, marja+watch_chromium.org, adamk+blink_chromium.org, Nate Chapin, Inactive Base URL:
svn://svn.chromium.org/blink/trunk Visibility:
Public. |
DescriptionImprove test coverage of TreatNullAs=NullString and TreatUndefinedAs=NullString
BUG=334519
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=165178
Patch Set 1 #Patch Set 2 : rebase #
Messages
Total messages: 13 (0 generated)
PTAL. I couldn't figure out whether or not TestObjectPython.idl should be kept in sync or not, so I left it alone for now.
LGTM
On 2014/01/15 04:19:06, philipj wrote: > PTAL. LGTM. > I couldn't figure out whether or not TestObjectPython.idl should be kept > in sync or not, so I left it alone for now. No need to keep in sync -- I do this periodically, and anyway Python should support most of these tests already. I've intentionally only added tests to TestObjectPython.idl that are actually used in real IDL files, so some cases that the Perl CG supports but aren't actually used may result in test failures. If you'd like to add them and run Tools/Scripts/run-bindings-tests --test-python ...and comment out (with FIXME) those that don't work, that's a nice touch, but it's not necessary. Thanks!
On 2014/01/15 04:54:02, Nils Barth wrote: > On 2014/01/15 04:19:06, philipj wrote: > > PTAL. > > LGTM. > > > I couldn't figure out whether or not TestObjectPython.idl should be kept > > in sync or not, so I left it alone for now. > > No need to keep in sync -- I do this periodically, and anyway Python should > support most of these tests already. > > I've intentionally only added tests to TestObjectPython.idl that are actually > used in real IDL files, so some cases that the Perl CG supports but aren't > actually used may result in test failures. > > If you'd like to add them and run > Tools/Scripts/run-bindings-tests --test-python > ...and comment out (with FIXME) those that don't work, that's a nice touch, but > it's not necessary. > > Thanks! I see that the grouping and order of TestObjectPython.idl is a little bit different, and several of the tests I added aren't used in real IDL files, so I'll leave the Python bits alone this time.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/philipj@opera.com/138483004/1
Failed to apply patch for Source/bindings/tests/results/V8TestObject.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file Source/bindings/tests/results/V8TestObject.cpp
Hunk #1 succeeded at 446 with fuzz 1.
Hunk #3 succeeded at 668 with fuzz 1.
Hunk #5 FAILED at 844.
Hunk #6 FAILED at 865.
Hunk #8 succeeded at 961 with fuzz 1.
2 out of 9 hunks FAILED -- saving rejects to file
Source/bindings/tests/results/V8TestObject.cpp.rej
Patch: Source/bindings/tests/results/V8TestObject.cpp
Index: Source/bindings/tests/results/V8TestObject.cpp
diff --git a/Source/bindings/tests/results/V8TestObject.cpp
b/Source/bindings/tests/results/V8TestObject.cpp
index
aefc8f6b507a14f93903b6bda4bb5592ffaeb8c5..585dfaac2afbe8a6c0d0ee5f7a030518e8cc86b9
100644
--- a/Source/bindings/tests/results/V8TestObject.cpp
+++ b/Source/bindings/tests/results/V8TestObject.cpp
@@ -446,6 +446,60 @@ static void
stringAttrAttributeSetterCallback(v8::Local<v8::String>, v8::Local<v
TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
}
+static void treatNullAsNullStringStringAttrAttributeGetter(const
v8::PropertyCallbackInfo<v8::Value>& info)
+{
+ TestObj* imp = V8TestObject::toNative(info.Holder());
+ v8SetReturnValueString(info, imp->treatNullAsNullStringStringAttr(),
info.GetIsolate());
+}
+
+static void
treatNullAsNullStringStringAttrAttributeGetterCallback(v8::Local<v8::String>,
const v8::PropertyCallbackInfo<v8::Value>& info)
+{
+ TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMGetter");
+ TestObjV8Internal::treatNullAsNullStringStringAttrAttributeGetter(info);
+ TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
+}
+
+static void treatNullAsNullStringStringAttrAttributeSetter(v8::Local<v8::Value>
jsValue, const v8::PropertyCallbackInfo<void>& info)
+{
+ TestObj* imp = V8TestObject::toNative(info.Holder());
+ V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<WithNullCheck>,
cppValue, jsValue);
+ imp->setTreatNullAsNullStringStringAttr(cppValue);
+}
+
+static void
treatNullAsNullStringStringAttrAttributeSetterCallback(v8::Local<v8::String>,
v8::Local<v8::Value> jsValue, const v8::PropertyCallbackInfo<void>& info)
+{
+ TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMSetter");
+ TestObjV8Internal::treatNullAsNullStringStringAttrAttributeSetter(jsValue,
info);
+ TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
+}
+
+static void
treatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeGetter(const
v8::PropertyCallbackInfo<v8::Value>& info)
+{
+ TestObj* imp = V8TestObject::toNative(info.Holder());
+ v8SetReturnValueString(info,
imp->treatNullAsNullStringTreatUndefinedAsNullStringStringAttr(),
info.GetIsolate());
+}
+
+static void
treatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeGetterCallback(v8::Local<v8::String>,
const v8::PropertyCallbackInfo<v8::Value>& info)
+{
+ TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMGetter");
+
TestObjV8Internal::treatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeGetter(info);
+ TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
+}
+
+static void
treatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeSetter(v8::Local<v8::Value>
jsValue, const v8::PropertyCallbackInfo<void>& info)
+{
+ TestObj* imp = V8TestObject::toNative(info.Holder());
+
V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<WithUndefinedOrNullCheck>,
cppValue, jsValue);
+
imp->setTreatNullAsNullStringTreatUndefinedAsNullStringStringAttr(cppValue);
+}
+
+static void
treatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeSetterCallback(v8::Local<v8::String>,
v8::Local<v8::Value> jsValue, const v8::PropertyCallbackInfo<void>& info)
+{
+ TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMSetter");
+
TestObjV8Internal::treatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeSetter(jsValue,
info);
+ TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
+}
+
static void eventHandlerAttrAttributeGetter(const
v8::PropertyCallbackInfo<v8::Value>& info)
{
TestObj* imp = V8TestObject::toNative(info.Holder());
@@ -601,7 +655,7 @@ static void
reflectedStringAttrAttributeGetterCallback(v8::Local<v8::String>, co
static void reflectedStringAttrAttributeSetter(v8::Local<v8::Value> jsValue,
const v8::PropertyCallbackInfo<void>& info)
{
TestObj* imp = V8TestObject::toNative(info.Holder());
- V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<WithNullCheck>,
cppValue, jsValue);
+ V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<>, cppValue,
jsValue);
CustomElementCallbackDispatcher::CallbackDeliveryScope deliveryScope;
imp->setAttribute(HTMLNames::reflectedstringattrAttr, cppValue);
}
@@ -614,6 +668,64 @@ static void
reflectedStringAttrAttributeSetterCallback(v8::Local<v8::String>, v8
TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
}
+static void reflectedTreatNullAsNullStringStringAttrAttributeGetter(const
v8::PropertyCallbackInfo<v8::Value>& info)
+{
+ TestObj* imp = V8TestObject::toNative(info.Holder());
+ v8SetReturnValueString(info,
imp->fastGetAttribute(HTMLNames::reflectedtreatnullasnullstringstringattrAttr),
info.GetIsolate());
+}
+
+static void
reflectedTreatNullAsNullStringStringAttrAttributeGetterCallback(v8::Local<v8::String>,
const v8::PropertyCallbackInfo<v8::Value>& info)
+{
+ TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMGetter");
+
TestObjV8Internal::reflectedTreatNullAsNullStringStringAttrAttributeGetter(info);
+ TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
+}
+
+static void
reflectedTreatNullAsNullStringStringAttrAttributeSetter(v8::Local<v8::Value>
jsValue, const v8::PropertyCallbackInfo<void>& info)
+{
+ TestObj* imp = V8TestObject::toNative(info.Holder());
+ V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<WithNullCheck>,
cppValue, jsValue);
+ CustomElementCallbackDispatcher::CallbackDeliveryScope deliveryScope;
+ imp->setAttribute(HTMLNames::reflectedtreatnullasnullstringstringattrAttr,
cppValue);
+}
+
+static void
reflectedTreatNullAsNullStringStringAttrAttributeSetterCallback(v8::Local<v8::String>,
v8::Local<v8::Value> jsValue, const v8::PropertyCallbackInfo<void>& info)
+{
+ TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMSetter");
+ CustomElementCallbackDispatcher::CallbackDeliveryScope deliveryScope;
+
TestObjV8Internal::reflectedTreatNullAsNullStringStringAttrAttributeSetter(jsValue,
info);
+ TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
+}
+
+static void
reflectedTreatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeGetter(const
v8::PropertyCallbackInfo<v8::Value>& info)
+{
+ TestObj* imp = V8TestObject::toNative(info.Holder());
+ v8SetReturnValueString(info,
imp->fastGetAttribute(HTMLNames::reflectedtreatnullasnullstringtreatundefinedasnullstringstringattrAttr),
info.GetIsolate());
+}
+
+static void
reflectedTreatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeGetterCallback(v8::Local<v8::String>,
const v8::PropertyCallbackInfo<v8::Value>& info)
+{
+ TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMGetter");
+
TestObjV8Internal::reflectedTreatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeGetter(info);
+ TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
+}
+
+static void
reflectedTreatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeSetter(v8::Local<v8::Value>
jsValue, const v8::PropertyCallbackInfo<void>& info)
+{
+ TestObj* imp = V8TestObject::toNative(info.Holder());
+
V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<WithUndefinedOrNullCheck>,
cppValue, jsValue);
+ CustomElementCallbackDispatcher::CallbackDeliveryScope deliveryScope;
+
imp->setAttribute(HTMLNames::reflectedtreatnullasnullstringtreatundefinedasnullstringstringattrAttr,
cppValue);
+}
+
+static void
reflectedTreatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeSetterCallback(v8::Local<v8::String>,
v8::Local<v8::Value> jsValue, const v8::PropertyCallbackInfo<void>& info)
+{
+ TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMSetter");
+ CustomElementCallbackDispatcher::CallbackDeliveryScope deliveryScope;
+
TestObjV8Internal::reflectedTreatNullAsNullStringTreatUndefinedAsNullStringStringAttrAttributeSetter(jsValue,
info);
+ TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
+}
+
static void reflectedIntegralAttrAttributeGetter(const
v8::PropertyCallbackInfo<v8::Value>& info)
{
TestObj* imp = V8TestObject::toNative(info.Holder());
@@ -719,7 +831,7 @@ static void
reflectedURLAttrAttributeGetterCallback(v8::Local<v8::String>, const
static void reflectedURLAttrAttributeSetter(v8::Local<v8::Value> jsValue, const
v8::PropertyCallbackInfo<void>& info)
{
TestObj* imp = V8TestObject::toNative(info.Holder());
- V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<WithNullCheck>,
cppValue, jsValue);
+ V8TRYCATCH_FOR_V8STRINGRESOURCE_VOID(V8StringResource<>, cppValue,
jsValue);
CustomElementCallbackDispatcher::CallbackDeliveryScope deliveryScope;
imp->setAttribute(HTMLNames::reflectedurlattrAttr, cppValue);
}
@@ -732,20 +844,107 @@ static void
reflectedURLAttrAttributeSetterCallback(v8::Local<v8::String>, v8::L
TRACE_EVENT_SET_SAMPLING_STATE("V8", "Execution");
}
-static void reflectedStringAttrAttributeGetter(const
v8::PropertyCallbackInfo<v8::Value>& info)
+static void reflectedTreatNullAsNullStringURLAttrAttributeGetter(const
v8::PropertyCallbackInfo<v8::Value>& info)
+{
+ TestObj* imp = V8TestObject::toNative(info.Holder());
+ v8SetReturnValueString(info,
imp->getURLAttribute(HTMLNames::reflectedtreatnullasnullstringurlattrAttr),
info.GetIsolate());
+}
+
+static void
reflectedTreatNullAsNullStringURLAttrAttributeGetterCallback(v8::Local<v8::String>,
const v8::PropertyCallbackInfo<v8::Value>& info)
+{
+ TRACE_EVENT_SET_SAMPLING_STATE("Blink", "DOMGetter");
+
TestObjV8Internal::reflectedTreatNullAsNullStringURLAttrAttributeGetter(info);
+ …
(message too large)
On 2014/01/15 08:03:24, philipj wrote: > I see that the grouping and order of TestObjectPython.idl is a little bit > different, and several of the tests I added aren't used in real IDL files, so > I'll leave the Python bits alone this time. Sounds fine! I'll check everything when implementing [TreatNullAs=EmptyString] in Python (after you've done it in Perl).
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/philipj@opera.com/138483004/110001
LGTM
I've abandoned TreatNullAs=EmptyString and updated the commit message, but will still commit this unless someone says boo.
Commit queue rejected this change because the description was changed between the time the change entered the commit queue and the time it was ready to commit. You can safely check the commit box again.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/philipj@opera.com/138483004/110001
Message was sent while issue was closed.
Change committed as 165178 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
