Attachment 361462 Details for Bug 190997 – Patch

[patch] Patch

bug-190997-20190207160455.patch (text/plain), 19.86 KB, created by Andy Estes on 2019-02-07 16:04:56 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Andy Estes

Created: 2019-02-07 16:04:56 PST

Size: 19.86 KB

patch

obsolete

>Subversion Revision: 241049
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 20840c60dafa706d51455081fc61894f90fcfc79..8f7fafe6f4ef2ff389a9ca3e31720780e347f5f9 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,36 @@
>+2019-02-07  Andy Estes  <aestes@apple.com>
>+
>+        [WebIDL] Support serializing sequences and FrozenArrays of non-interfaces
>+        https://bugs.webkit.org/show_bug.cgi?id=190997
>+        <rdar://problem/35983035>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Support serializing sequences and FrozenArrays of types that aren't interfaces. This is
>+        needed to properly serialize PaymentAddress, which has a FrozenArray of DOMStrings.
>+
>+        We should support serializing sequences of interfaces too, but that's slightly more
>+        complicated since it involves iterating the sequence and serializing each of its items. I
>+        left that as a follow-up task, since I don't see any IDLs that currently need this.
>+
>+        We also don't support serializing sequences with the CachedAttribute or CustomGetter
>+        extended attributes, because WebIDL specifies that a new array should be created when
>+        converting an IDL sequence into an ECMAScript value.
>+
>+        Added bindings test cases to TestSerialization.idl and PaymentAddress test cases to
>+        http/tests/paymentrequest/payment-address-attributes-and-toJSON-method.https.html.
>+
>+        * bindings/scripts/CodeGenerator.pm:
>+        (GetInterfaceForType): Renamed from GetInterfaceForAttribute.
>+        (IsSerializableType): Modified to allow sequences and FrozenArrays of non-interface types.
>+        (hasCachedAttributeOrCustomGetterExtendedAttribute): Added a helper to determine if an
>+        attribute has the CachedAttribute or CustomGetter extended attributes.
>+        (IsSerializableAttribute): Checked for sequences with the CachedAttribute or CustomGetter
>+        extended attributes before calling IsSerializableType.
>+        (GetInterfaceForAttribute): Renamed to GetInterfaceForType.
>+        * bindings/scripts/test/JS/JSTestSerialization.cpp:
>+        * bindings/scripts/test/TestSerialization.idl:
>+
> 2019-02-06  John Wilander  <wilander@apple.com>
> 
>         Forward Ad Click Attribution data from HTMLAnchorElement::handleClick() to WebKit::NavigationActionData
>diff --git a/Source/WebCore/bindings/scripts/CodeGenerator.pm b/Source/WebCore/bindings/scripts/CodeGenerator.pm
>index 75c19ecdba868f52d2981df750fd593d76238943..2164fb81a3cb8f46e0e4a43a9aff07eb8649287c 100644
>--- a/Source/WebCore/bindings/scripts/CodeGenerator.pm
>+++ b/Source/WebCore/bindings/scripts/CodeGenerator.pm
>@@ -317,13 +317,13 @@ sub IDLFileForInterface
>     return $idlFiles->{$interfaceName};
> }
> 
>-sub GetInterfaceForAttribute
>+sub GetInterfaceForType
> {
>-    my ($object, $currentInterface, $attribute) = @_;
>+    my ($object, $currentInterface, $type) = @_;
> 
>-    return undef unless $object->IsInterfaceType($attribute->type);
>+    return undef unless $object->IsInterfaceType($type);
> 
>-    return $object->ParseInterface($currentInterface, $attribute->type->name);
>+    return $object->ParseInterface($currentInterface, $type->name);
> }
> 
> sub GetAttributeFromInterface
>@@ -935,34 +935,59 @@ sub InheritsSerializable
>     return $anyParentIsSerializable;
> }
> 
>-sub IsSerializableAttribute
>+sub IsSerializableType
> {
>-    my ($object, $interface, $attribute) = @_;
>+    my ($object, $interface, $type) = @_;
> 
>     # https://heycam.github.io/webidl/#dfn-serializable-type
> 
>-    my $type = $attribute->type;
>     return 1 if $type->name eq "boolean";
>     return 1 if $object->IsNumericType($type);
>     return 1 if $object->IsEnumType($type);
>     return 1 if $object->IsStringType($type);
>     return 0 if $type->name eq "EventHandler";
> 
>-    if ($type->isUnion || $object->IsSequenceType($type) || $object->IsDictionaryType($type)) {
>-        die "Serializer for non-primitive types is not currently supported\n";
>+    if ($type->isUnion || $object->IsDictionaryType($type)) {
>+        die "Serializers for union and dictionary types are not currently supported.\n";
>+    }
>+
>+    if ($object->IsSequenceOrFrozenArrayType($type)) {
>+        my $subtype = @{$type->subtypes}[0];
>+
>+        # FIXME: Teach GenerateSerializerDefinition to serialize sequences of interfaces, then remove this line.
>+        return 0 if $object->IsInterfaceType($subtype);
>+
>+        return $object->IsSerializableType($interface, $subtype);
>     }
> 
>     return 0 if !$object->IsInterfaceType($type);
> 
>-    my $interfaceForAttribute = $object->GetInterfaceForAttribute($interface, $attribute);
>-    if ($interfaceForAttribute) {
>-        return 1 if $interfaceForAttribute->serializable;
>-        return $object->InheritsSerializable($interfaceForAttribute);
>+    my $interfaceForType = $object->GetInterfaceForType($interface, $type);
>+    if ($interfaceForType) {
>+        return 1 if $interfaceForType->serializable;
>+        return $object->InheritsSerializable($interfaceForType);
>     }
> 
>     return 0;
> }
> 
>+sub hasCachedAttributeOrCustomGetterExtendedAttribute
>+{
>+    my ($attribute) = @_;
>+    return $attribute->extendedAttributes->{CachedAttribute} || $attribute->extendedAttributes->{CustomGetter};
>+}
>+
>+sub IsSerializableAttribute
>+{
>+    my ($object, $interface, $attribute) = @_;
>+
>+    if ($object->IsSequenceType($attribute->type) && hasCachedAttributeOrCustomGetterExtendedAttribute($attribute)) {
>+        die "Serializers for sequence types with CachedAttribute or CustomGetter extended attributes are not currently supported.\n";
>+    }
>+
>+    return $object->IsSerializableType($interface, $attribute->type);
>+}
>+
> sub GetInterfaceExtendedAttributesFromName
> {
>     # FIXME: It's bad to have a function like this that opens another IDL file to answer a question.
>diff --git a/Source/WebCore/bindings/scripts/test/JS/JSTestSerialization.cpp b/Source/WebCore/bindings/scripts/test/JS/JSTestSerialization.cpp
>index eaf57f8a81372e01c3ff195abd524b7294d1709d..965634adf1a35e2047fd2db4af8758db354f7517 100644
>--- a/Source/WebCore/bindings/scripts/test/JS/JSTestSerialization.cpp
>+++ b/Source/WebCore/bindings/scripts/test/JS/JSTestSerialization.cpp
>@@ -24,9 +24,11 @@
> #include "JSDOMAttribute.h"
> #include "JSDOMBinding.h"
> #include "JSDOMConstructorNotConstructable.h"
>+#include "JSDOMConvertBoolean.h"
> #include "JSDOMConvertInterface.h"
> #include "JSDOMConvertNullable.h"
> #include "JSDOMConvertNumbers.h"
>+#include "JSDOMConvertSequences.h"
> #include "JSDOMConvertStrings.h"
> #include "JSDOMExceptionHandling.h"
> #include "JSDOMGlobalObject.h"
>@@ -38,6 +40,7 @@
> #include "ScriptExecutionContext.h"
> #include <JavaScriptCore/FunctionPrototype.h>
> #include <JavaScriptCore/HeapSnapshotBuilder.h>
>+#include <JavaScriptCore/JSArray.h>
> #include <JavaScriptCore/JSCInlines.h>
> #include <JavaScriptCore/ObjectConstructor.h>
> #include <wtf/GetPtr.h>
>@@ -74,6 +77,12 @@ JSC::EncodedJSValue jsTestSerializationEighthIndirectlyAttribute(JSC::ExecState*
> bool setJSTestSerializationEighthIndirectlyAttribute(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
> JSC::EncodedJSValue jsTestSerializationNinthOptionalDirectlySerializableAttribute(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
> bool setJSTestSerializationNinthOptionalDirectlySerializableAttribute(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
>+JSC::EncodedJSValue jsTestSerializationTenthFrozenArrayAttribute(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
>+bool setJSTestSerializationTenthFrozenArrayAttribute(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
>+JSC::EncodedJSValue jsTestSerializationEleventhSequenceAttribute(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
>+bool setJSTestSerializationEleventhSequenceAttribute(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
>+JSC::EncodedJSValue jsTestSerializationTwelfthInterfaceSequenceAttribute(JSC::ExecState*, JSC::EncodedJSValue, JSC::PropertyName);
>+bool setJSTestSerializationTwelfthInterfaceSequenceAttribute(JSC::ExecState*, JSC::EncodedJSValue, JSC::EncodedJSValue);
> 
> class JSTestSerializationPrototype : public JSC::JSNonFinalObject {
> public:
>@@ -131,6 +140,9 @@ static const HashTableValue JSTestSerializationPrototypeTableValues[] =
>     { "seventhDirectlySerializableAttribute", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestSerializationSeventhDirectlySerializableAttribute), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestSerializationSeventhDirectlySerializableAttribute) } },
>     { "eighthIndirectlyAttribute", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestSerializationEighthIndirectlyAttribute), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestSerializationEighthIndirectlyAttribute) } },
>     { "ninthOptionalDirectlySerializableAttribute", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestSerializationNinthOptionalDirectlySerializableAttribute), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestSerializationNinthOptionalDirectlySerializableAttribute) } },
>+    { "tenthFrozenArrayAttribute", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestSerializationTenthFrozenArrayAttribute), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestSerializationTenthFrozenArrayAttribute) } },
>+    { "eleventhSequenceAttribute", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestSerializationEleventhSequenceAttribute), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestSerializationEleventhSequenceAttribute) } },
>+    { "twelfthInterfaceSequenceAttribute", static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::DOMAttribute), NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestSerializationTwelfthInterfaceSequenceAttribute), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestSerializationTwelfthInterfaceSequenceAttribute) } },
>     { "toJSON", static_cast<unsigned>(JSC::PropertyAttribute::Function), NoIntrinsic, { (intptr_t)static_cast<RawNativeFunction>(jsTestSerializationPrototypeFunctionToJSON), (intptr_t) (0) } },
> };
> 
>@@ -489,6 +501,99 @@ bool setJSTestSerializationNinthOptionalDirectlySerializableAttribute(ExecState*
>     return IDLAttribute<JSTestSerialization>::set<setJSTestSerializationNinthOptionalDirectlySerializableAttributeSetter>(*state, thisValue, encodedValue, "ninthOptionalDirectlySerializableAttribute");
> }
> 
>+static inline JSValue jsTestSerializationTenthFrozenArrayAttributeGetter(ExecState& state, JSTestSerialization& thisObject, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    UNUSED_PARAM(state);
>+    auto& impl = thisObject.wrapped();
>+    JSValue result = toJS<IDLFrozenArray<IDLBoolean>>(state, *thisObject.globalObject(), throwScope, impl.tenthFrozenArrayAttribute());
>+    return result;
>+}
>+
>+EncodedJSValue jsTestSerializationTenthFrozenArrayAttribute(ExecState* state, EncodedJSValue thisValue, PropertyName)
>+{
>+    return IDLAttribute<JSTestSerialization>::get<jsTestSerializationTenthFrozenArrayAttributeGetter, CastedThisErrorBehavior::Assert>(*state, thisValue, "tenthFrozenArrayAttribute");
>+}
>+
>+static inline bool setJSTestSerializationTenthFrozenArrayAttributeSetter(ExecState& state, JSTestSerialization& thisObject, JSValue value, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    auto& impl = thisObject.wrapped();
>+    auto nativeValue = convert<IDLFrozenArray<IDLBoolean>>(state, value);
>+    RETURN_IF_EXCEPTION(throwScope, false);
>+    AttributeSetter::call(state, throwScope, [&] {
>+        return impl.setTenthFrozenArrayAttribute(WTFMove(nativeValue));
>+    });
>+    return true;
>+}
>+
>+bool setJSTestSerializationTenthFrozenArrayAttribute(ExecState* state, EncodedJSValue thisValue, EncodedJSValue encodedValue)
>+{
>+    return IDLAttribute<JSTestSerialization>::set<setJSTestSerializationTenthFrozenArrayAttributeSetter>(*state, thisValue, encodedValue, "tenthFrozenArrayAttribute");
>+}
>+
>+static inline JSValue jsTestSerializationEleventhSequenceAttributeGetter(ExecState& state, JSTestSerialization& thisObject, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    UNUSED_PARAM(state);
>+    auto& impl = thisObject.wrapped();
>+    JSValue result = toJS<IDLSequence<IDLDOMString>>(state, *thisObject.globalObject(), throwScope, impl.eleventhSequenceAttribute());
>+    return result;
>+}
>+
>+EncodedJSValue jsTestSerializationEleventhSequenceAttribute(ExecState* state, EncodedJSValue thisValue, PropertyName)
>+{
>+    return IDLAttribute<JSTestSerialization>::get<jsTestSerializationEleventhSequenceAttributeGetter, CastedThisErrorBehavior::Assert>(*state, thisValue, "eleventhSequenceAttribute");
>+}
>+
>+static inline bool setJSTestSerializationEleventhSequenceAttributeSetter(ExecState& state, JSTestSerialization& thisObject, JSValue value, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    auto& impl = thisObject.wrapped();
>+    auto nativeValue = convert<IDLSequence<IDLDOMString>>(state, value);
>+    RETURN_IF_EXCEPTION(throwScope, false);
>+    AttributeSetter::call(state, throwScope, [&] {
>+        return impl.setEleventhSequenceAttribute(WTFMove(nativeValue));
>+    });
>+    return true;
>+}
>+
>+bool setJSTestSerializationEleventhSequenceAttribute(ExecState* state, EncodedJSValue thisValue, EncodedJSValue encodedValue)
>+{
>+    return IDLAttribute<JSTestSerialization>::set<setJSTestSerializationEleventhSequenceAttributeSetter>(*state, thisValue, encodedValue, "eleventhSequenceAttribute");
>+}
>+
>+static inline JSValue jsTestSerializationTwelfthInterfaceSequenceAttributeGetter(ExecState& state, JSTestSerialization& thisObject, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    UNUSED_PARAM(state);
>+    auto& impl = thisObject.wrapped();
>+    JSValue result = toJS<IDLSequence<IDLInterface<TestSerializationInheritFinal>>>(state, *thisObject.globalObject(), throwScope, impl.twelfthInterfaceSequenceAttribute());
>+    return result;
>+}
>+
>+EncodedJSValue jsTestSerializationTwelfthInterfaceSequenceAttribute(ExecState* state, EncodedJSValue thisValue, PropertyName)
>+{
>+    return IDLAttribute<JSTestSerialization>::get<jsTestSerializationTwelfthInterfaceSequenceAttributeGetter, CastedThisErrorBehavior::Assert>(*state, thisValue, "twelfthInterfaceSequenceAttribute");
>+}
>+
>+static inline bool setJSTestSerializationTwelfthInterfaceSequenceAttributeSetter(ExecState& state, JSTestSerialization& thisObject, JSValue value, ThrowScope& throwScope)
>+{
>+    UNUSED_PARAM(throwScope);
>+    auto& impl = thisObject.wrapped();
>+    auto nativeValue = convert<IDLSequence<IDLInterface<TestSerializationInheritFinal>>>(state, value);
>+    RETURN_IF_EXCEPTION(throwScope, false);
>+    AttributeSetter::call(state, throwScope, [&] {
>+        return impl.setTwelfthInterfaceSequenceAttribute(WTFMove(nativeValue));
>+    });
>+    return true;
>+}
>+
>+bool setJSTestSerializationTwelfthInterfaceSequenceAttribute(ExecState* state, EncodedJSValue thisValue, EncodedJSValue encodedValue)
>+{
>+    return IDLAttribute<JSTestSerialization>::set<setJSTestSerializationTwelfthInterfaceSequenceAttributeSetter>(*state, thisValue, encodedValue, "twelfthInterfaceSequenceAttribute");
>+}
>+
> JSC::JSObject* JSTestSerialization::serialize(ExecState& state, JSTestSerialization& thisObject, JSDOMGlobalObject& globalObject, ThrowScope& throwScope)
> {
>     auto& vm = state.vm();
>@@ -532,6 +637,14 @@ JSC::JSObject* JSTestSerialization::serialize(ExecState& state, JSTestSerializat
>     } else
>         result->putDirect(vm, Identifier::fromString(&vm, "ninthOptionalDirectlySerializableAttribute"), ninthOptionalDirectlySerializableAttributeValue);
> 
>+    auto tenthFrozenArrayAttributeValue = jsTestSerializationTenthFrozenArrayAttributeGetter(state, thisObject, throwScope);
>+    throwScope.assertNoException();
>+    result->putDirect(vm, Identifier::fromString(&vm, "tenthFrozenArrayAttribute"), tenthFrozenArrayAttributeValue);
>+
>+    auto eleventhSequenceAttributeValue = jsTestSerializationEleventhSequenceAttributeGetter(state, thisObject, throwScope);
>+    throwScope.assertNoException();
>+    result->putDirect(vm, Identifier::fromString(&vm, "eleventhSequenceAttribute"), eleventhSequenceAttributeValue);
>+
>     return result;
> }
> 
>diff --git a/Source/WebCore/bindings/scripts/test/TestSerialization.idl b/Source/WebCore/bindings/scripts/test/TestSerialization.idl
>index 0c72e919623e95b9215a2e72ef463a565537d68f..41eb7b93aa9dd23d7f73d0ca0fa174a3f6c34e1d 100644
>--- a/Source/WebCore/bindings/scripts/test/TestSerialization.idl
>+++ b/Source/WebCore/bindings/scripts/test/TestSerialization.idl
>@@ -37,5 +37,9 @@ interface TestSerialization {
>     attribute TestSerializationIndirectInheritance eighthIndirectlyAttribute;
>     attribute TestSerializationInheritFinal? ninthOptionalDirectlySerializableAttribute;
> 
>+    attribute FrozenArray<boolean> tenthFrozenArrayAttribute;
>+    attribute sequence<DOMString> eleventhSequenceAttribute;
>+    attribute sequence<TestSerializationInheritFinal> twelfthInterfaceSequenceAttribute;
>+
>     serializer = { attribute };
> };
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index d8a3974fa6e6fe7036db860646699e9480f28fb3..3c597ec045f341897992ae326a01a94a49fdb487 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-02-07  Andy Estes  <aestes@apple.com>
>+
>+        [WebIDL] Support serializing sequences and FrozenArrays of non-interfaces
>+        https://bugs.webkit.org/show_bug.cgi?id=190997
>+        <rdar://problem/35983035>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/tests/paymentrequest/payment-address-attributes-and-toJSON-method.https.html:
>+
> 2019-02-06  Justin Fan  <justin_fan@apple.com>
> 
>         [Web GPU] Implement supporting dictionaries for GPUTexture
>diff --git a/LayoutTests/http/tests/paymentrequest/payment-address-attributes-and-toJSON-method.https.html b/LayoutTests/http/tests/paymentrequest/payment-address-attributes-and-toJSON-method.https.html
>index 70a433f6bd856d67662f7e84fba5ef94a5cd59c6..c6039b530cef72fe8b1ecad95ea3e2a74fb335bd 100644
>--- a/LayoutTests/http/tests/paymentrequest/payment-address-attributes-and-toJSON-method.https.html
>+++ b/LayoutTests/http/tests/paymentrequest/payment-address-attributes-and-toJSON-method.https.html
>@@ -38,7 +38,8 @@ function runTest(button, expected = {}) {
>       },
>       "Array must be frozen"
>     );
>-    for (let [attr, expectedValue] of Object.entries(expected)) {
>+    const expectedEntries = Object.entries(expected);
>+    for (let [attr, expectedValue] of expectedEntries) {
>       assert_idl_attribute(addr, attr);
>       const msg = `Expected paymentAddress.${attr} to equal ${expectedValue}.`;
>       //.toString() flattens array addressLine,
>@@ -48,12 +49,22 @@ function runTest(button, expected = {}) {
>       assert_equals(actualValue, expectedValue, msg);
>     }
>     // Check toJSON result
>-    for (let [prop, jsonValue] of Object.entries(addr.toJSON())) {
>+    const jsonAddress = addr.toJSON();
>+    const jsonAddressEntries = Object.entries(jsonAddress);
>+    for (let [prop, jsonValue] of jsonAddressEntries) {
>       const actualValue = jsonValue.toString().toLowerCase();
>       const expectedValue = expected[prop].toString().toLowerCase();
>       const msg = `Expected JSON ${prop} to be ${expectedValue}`;
>       assert_equals(actualValue, expectedValue, msg);
>     }
>+    assert_equals(jsonAddressEntries.length, expectedEntries.length);
>+    // Check that .toJSON() created a frozen array
>+    assert_throws(
>+      new TypeError(),
>+      () => {
>+        jsonAddress.addressLine.push("this must throw");
>+      },
>+    );
>   }, button.textContent.trim());
>   done();
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 190997: 353247 | 353254 | 361462 | 361511