Issue 10636021: Blob constructor

Issue 10636021: Blob constructor (Closed)

Created:
8 years, 6 months ago by sra1

Modified:
8 years, 5 months ago

Reviewers:
vsm, Anton Muhin

CC:
reviews_dartlang.org, Anton Muhin, podivilov1

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Blob constructor Fix for http://code.google.com/p/dart/issues/detail?id=2181. Needs custom constructor to be implemented to work on Dartium. Committed: https://code.google.com/p/dart/source/detail?r=9267

Patch Set 3 : #

Created: 8 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+152 lines, -4 lines)			Patch
M	lib/dom/dom.dart	View		2 chunks	+10 lines, -1 line	0 comments	Download
M	lib/dom/frog/dom_frog.dart	View		2 chunks	+11 lines, -1 line	0 comments	Download
M	lib/dom/idl/dart/dart.idl	View		1 chunk	+11 lines, -0 lines	0 comments	Download
A	lib/dom/templates/html/frog/factoryprovider_Blob.darttemplate	View		1 chunk	+24 lines, -0 lines	0 comments	Download
M	lib/html/dartium/html_dartium.dart	View		2 chunks	+11 lines, -1 line	0 comments	Download
M	lib/html/frog/html_frog.dart	View		2 chunks	+27 lines, -1 line	0 comments	Download
A	tests/html/blob_constructor_test.dart	View	1	1 chunk	+57 lines, -0 lines	0 comments	Download
M	tests/html/html.status	View	1 2	1 chunk	+1 line, -0 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Anton Muhin

https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/dom/dom.dart File lib/dom/dom.dart (right): https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/dom/dom.dart#newcode17 lib/dom/dom.dart:17: factory Blob(List blobParts, [String type = null, String endings ...

8 years, 5 months ago (2012-06-29 12:19:43 UTC) #3

sra1

Anton - should I really pass _null to a native constructor? https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/dom/dom.dart File lib/dom/dom.dart (right): ...

8 years, 5 months ago (2012-06-29 19:13:30 UTC) #4

sra1

On 2012/06/29 19:13:30, sra1 wrote: > Anton - should I really pass _null to a ...

8 years, 5 months ago (2012-06-29 20:54:58 UTC) #5

On 2012/06/29 19:13:30, sra1 wrote:
> Anton - should I really pass _null to a native constructor?
> 
> https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/dom/dom.dart
> File lib/dom/dom.dart (right):
> 
>
https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/dom/dom.dart#ne...
> lib/dom/dom.dart:17: factory Blob(List blobParts, [String type = null, String
> endings = null]) => _dummy();
> On 2012/06/29 12:19:43, antonmuhin wrote:
> > nit: maybe omit explicit null default?  and you looks somewhat inconsistent:
> > sometimes you provide them, sometimes not.
> 
> The generated JS bindings usually have '= null' in the implementation and no
> value in the interface (since that used to be illegal).
> See DataView below - it is consistent with that.
> 
> Hand-written functions often omit '= null'.
> 
> I would want to 'fix' this as a separate CL since it would affect all
generated
> functions with optional arguments.
> 
>
https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/dom/templates/h...
> File lib/dom/templates/html/frog/factoryprovider_Blob.darttemplate (right):
> 
>
https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/dom/templates/h...
> lib/dom/templates/html/frog/factoryprovider_Blob.darttemplate:10: if (type ==
> null && endings == null) {
> On 2012/06/29 12:19:43, antonmuhin wrote:
> > nit: === for null comparison.
> 
> The languages is changing to remove ===.
> See the 'Operator Cleanups' document.
> 
>
https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/html/dartium/ht...
> File lib/html/dartium/html_dartium.dart (right):
> 
>
https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/html/dartium/ht...
> lib/html/dartium/html_dartium.dart:650: +  factory Blob(List blobParts,
[String
> type = null, String endings = null]) => _createBlob(blobParts, type, endings);
> On 2012/06/29 12:19:43, antonmuhin wrote:
> > that should be _null in Dartium.
> 
> Even when passed directly to a native function?

I'm going to submit since it is the only way I have of finding the suppressions
I need for various browsers.  I will fix the '= null' under another CL.  I think
the '_null' comment it incorrect but if it is correct, we need to fix several
other constructors with the same generator change.

Anton Muhin

8 years, 5 months ago (2012-07-02 13:02:39 UTC) #6

https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/dom/templates/h...
File lib/dom/templates/html/frog/factoryprovider_Blob.darttemplate (right):

https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/dom/templates/h...
lib/dom/templates/html/frog/factoryprovider_Blob.darttemplate:10: if (type ==
null && endings == null) {
I am only striving for uniformity.  I believe we use === a lot in dart:html code
and it kind of easier to grep for === instead of (null ==||== null), esp., w/
line breaks.  But that's minor issue.

On 2012/06/29 19:13:30, sra1 wrote:
> On 2012/06/29 12:19:43, antonmuhin wrote:
> > nit: === for null comparison.
> 
> The languages is changing to remove ===.
> See the 'Operator Cleanups' document.

https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/html/dartium/ht...
File lib/html/dartium/html_dartium.dart (right):

https://chromiumcodereview.appspot.com/10636021/diff/6001/lib/html/dartium/ht...
lib/html/dartium/html_dartium.dart:650: +  factory Blob(List blobParts, [String
type = null, String endings = null]) => _createBlob(blobParts, type, endings);
That's a good point.  I think ideally you should have a dispatcher in pure Dart
here (and ideally we should generate those).

As it's not the case right now anyway, please, disregard, we need to fix it
later though.

On 2012/06/29 19:13:30, sra1 wrote:
> On 2012/06/29 12:19:43, antonmuhin wrote:
> > that should be _null in Dartium.
> 
> Even when passed directly to a native function?

Expand Messages | Collapse Messages