Update dart bindings to correctly patch print.

Update dart bindings to correctly patch print.

This needs to land simultaneously with: https://chromiumcodereview.appspot.com/10928145

Committed: https://src.chromium.org/viewvc/multivm?view=rev&revision=865

[Anton Muhin, 2012-09-12 14:50:54]

https://chromiumcodereview.appspot.com/10928146/diff/5002/WebCore/bindings/da...
File WebCore/bindings/dart/DartController.cpp (right):

https://chromiumcodereview.appspot.com/10928146/diff/5002/WebCore/bindings/da...
WebCore/bindings/dart/DartController.cpp:77: if (targetClass) {
Please, to keep the cases as similar as they can be, reword it like:

Dart_Handle target = targetClass ? Dart_GetClass(library, Dart_NewString(...) :
library;
ASSERT(!Dart_IsError(target));
Dart_SetField(target, Dart_NewString(...)))

https://chromiumcodereview.appspot.com/10928146/diff/5002/WebCore/bindings/da...
WebCore/bindings/dart/DartController.cpp:80: Dart_SetField(cls,
Dart_NewString(targetField), value);
nit: 4 spaces for indent.

https://chromiumcodereview.appspot.com/10928146/diff/5002/WebCore/bindings/da...
WebCore/bindings/dart/DartController.cpp:99: copyValue(html, "_printClosure",
"dart:core", 0, "_printClosure");
cannot we unify this stuff and use class for print configuration too?  or
migrate both to top-level fields?

[Mads Ager (google), 2012-09-13 07:17:41]

https://chromiumcodereview.appspot.com/10928146/diff/5002/WebCore/bindings/da...
File WebCore/bindings/dart/DartController.cpp (right):

https://chromiumcodereview.appspot.com/10928146/diff/5002/WebCore/bindings/da...
WebCore/bindings/dart/DartController.cpp:77: if (targetClass) {
On 2012/09/12 14:50:54, Anton Muhin wrote:
> Please, to keep the cases as similar as they can be, reword it like:
> 
> Dart_Handle target = targetClass ? Dart_GetClass(library, Dart_NewString(...)
:
> library;
> ASSERT(!Dart_IsError(target));
> Dart_SetField(target, Dart_NewString(...)))

Done.

https://chromiumcodereview.appspot.com/10928146/diff/5002/WebCore/bindings/da...
WebCore/bindings/dart/DartController.cpp:80: Dart_SetField(cls,
Dart_NewString(targetField), value);
On 2012/09/12 14:50:54, Anton Muhin wrote:
> nit: 4 spaces for indent.

Thanks. Code is now gone.

https://chromiumcodereview.appspot.com/10928146/diff/5002/WebCore/bindings/da...
WebCore/bindings/dart/DartController.cpp:99: copyValue(html, "_printClosure",
"dart:core", 0, "_printClosure");
On 2012/09/12 14:50:54, Anton Muhin wrote:
> cannot we unify this stuff and use class for print configuration too?  or
> migrate both to top-level fields?

We could but I don't think we want to put that requirement on stuff that can be
patched from the embedder. I agree with you that we could probably make this
easier to handle for the embedder. Maybe by having some shared dart utility code
that contains something like your 'copyValue' method above and is used by all
embedders?

[Anton Muhin, 2012-09-13 07:25:33]

LGTM.

I can land it for you when in the office.

https://chromiumcodereview.appspot.com/10928146/diff/5002/WebCore/bindings/da...
File WebCore/bindings/dart/DartController.cpp (right):

https://chromiumcodereview.appspot.com/10928146/diff/5002/WebCore/bindings/da...
WebCore/bindings/dart/DartController.cpp:99: copyValue(html, "_printClosure",
"dart:core", 0, "_printClosure");
That'd be cool.

On 2012/09/13 07:17:42, Mads Ager wrote:
> On 2012/09/12 14:50:54, Anton Muhin wrote:
> > cannot we unify this stuff and use class for print configuration too?  or
> > migrate both to top-level fields?
> 
> We could but I don't think we want to put that requirement on stuff that can
be
> patched from the embedder. I agree with you that we could probably make this
> easier to handle for the embedder. Maybe by having some shared dart utility
code
> that contains something like your 'copyValue' method above and is used by all
> embedders?

[Mads Ager (google), 2012-09-13 07:30:36]

On 2012/09/13 07:25:33, Anton Muhin wrote:
> LGTM.
> 
> I can land it for you when in the office.

Thanks Anton. Ping me when you are in. We should land the two changes
simultaneously to avoid too much buildbot redness.