Bug 163611 - [WebIDL] Allow sequences of dictionaries as members of other dictionaries and as parameters
Summary: [WebIDL] Allow sequences of dictionaries as members of other dictionaries and...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on: 163624
Blocks:
  Show dependency treegraph
 
Reported: 2016-10-18 12:32 PDT by Zan Dobersek
Modified: 2016-10-18 23:20 PDT (History)
6 users (show)

See Also:


Attachments
Patch (9.43 KB, patch)
2016-10-18 12:44 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2016-10-18 12:32:46 PDT
[WebIDL] Allow sequences of dictionaries as members of other dictionaries and as parameters
Comment 1 Zan Dobersek 2016-10-18 12:44:53 PDT
Created attachment 291970 [details]
Patch
Comment 2 Sam Weinig 2016-10-18 12:48:43 PDT
This makes me think that Converter<IDLDictionary<T>>::convert() should maybe return a T not Optional<T>.  I need to think through what the consequences of this would be.
Comment 3 Chris Dumez 2016-10-18 12:56:17 PDT
(In reply to comment #2)
> This makes me think that Converter<IDLDictionary<T>>::convert() should maybe
> return a T not Optional<T>.  I need to think through what the consequences
> of this would be.

I think I made the switch to returning Optional a while back so we could return Nullopt in error cases. However, since then, we updated our dictionary code so that all our native dictionary structs must have a default constructor. Therefore, I think it is no longer necessary to return an Optional<>, we can just return { } instead.

So I agree with Sam.
Comment 4 Chris Dumez 2016-10-18 12:57:00 PDT
@Zan: I am happy to make the code change if you want. Just let me know.
Comment 5 Zan Dobersek 2016-10-18 23:19:37 PDT
(In reply to comment #4)
> @Zan: I am happy to make the code change if you want. Just let me know.

Saw it in bug #163624. Thanks!

With that this patch isn't needed anymore.