New issue
Advanced search Search tips

Issue 793021 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Bindings does not generate enum value-checking code for unioned-sequence types

Project Member Reported by smcgruer@chromium.org, Dec 7 2017

Issue description

Using the following example:

dictionary TestDictionary {
  sequence<CompositeOperation> seq_member;
  (CompositeOperation or sequence<CompositeOperation>) union_member;
};

For seq_member, V8TestDictionary::ToImpl will call into NativeValueTraits<IDLSequence<IDLString>>::NativeValue for the conversion, then call IsValidEnum to check that the values are valid:

void V8TestDictionary::ToImpl(...) {
...
      Vector<String> seq_memberCppValue = NativeValueTraits<IDLSequence<IDLString>>::NativeValue(isolate, seq_memberValue, exceptionState);
      if (exceptionState.HadException())
        return;
      const char* validValues[] = {
          "replace",
          "add",
          "accumulate",
      };
      if (!IsValidEnum(seq_memberCppValue, validValues, WTF_ARRAY_LENGTH(validValues), "CompositeOperation", exceptionState))
        return;
      impl.setSeq_member(seq_memberCppValue);

...
}

For union_member, however, V8CompositeOperationOrCompositeOperationSequence::ToImpl does not do an enum value check in the case that it is a sequence:

void V8CompositeOperationOrCompositeOperationSequence::ToImpl(...) {
...
    if (HasCallableIteratorSymbol(isolate, v8Value, exceptionState)) {
      Vector<String> cppValue = NativeValueTraits<IDLSequence<IDLString>>::NativeValue(isolate, v8Value, exceptionState);
      if (exceptionState.HadException())
        return;
      impl.SetCompositeOperationSequence(cppValue);
      return;
    }
...
}

I think there needs to be a call to IsValidEnum before the impl.SetCompositeOperationSequence(cppValue);
 
Components: Blink>Bindings
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e350980d255a3bdd1ca67abe905af44e35676c71

commit e350980d255a3bdd1ca67abe905af44e35676c71
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Fri Dec 08 03:57:00 2017

bindings: Validate enum values in unioned-sequence

Before this CL the code generator didn't generate value-checking code
for unioned-sequence types. Make the code generator generate it.

Bug:  793021 
Change-Id: I6cca9282013a8f84ad7776d3df9a030b81ec6f0e
Reviewed-on: https://chromium-review.googlesource.com/816374
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522704}
[modify] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/LayoutTests/bindings/idl-dictionary-unittest-expected.txt
[modify] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/LayoutTests/bindings/idl-dictionary-unittest.html
[modify] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/Source/bindings/core/v8/BUILD.gn
[modify] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/Source/bindings/templates/union_container.cpp.tmpl
[modify] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/Source/bindings/tests/idls/core/TestDictionary.idl
[modify] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.cpp
[modify] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/Source/bindings/tests/results/core/TestDictionary.h
[modify] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/Source/bindings/tests/results/core/V8TestDictionary.cpp
[add] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/Source/bindings/tests/results/core/test_enum_or_test_enum_sequence.cc
[add] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/Source/bindings/tests/results/core/test_enum_or_test_enum_sequence.h
[modify] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/Source/core/testing/DictionaryTest.cpp
[modify] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/Source/core/testing/DictionaryTest.h
[modify] https://crrev.com/e350980d255a3bdd1ca67abe905af44e35676c71/third_party/WebKit/Source/core/testing/InternalDictionary.idl

Comment 3 by bashi@chromium.org, Dec 8 2017

Status: Fixed (was: Assigned)

Sign in to add a comment