Fallback to unsupported browser type instead of returning null

Fallback to unsupported browser type instead of returning null

Corresponds to:
https://chromiumcodereview.appspot.com/483263004/

BUG=dartbug.com/20592
R=leafp@google.com

Committed: https://src.chromium.org/viewvc/blink/branches/dart/dartium?view=rev&revision=180943

[vsm, 2014-08-26 14:52:23]

vsm@google.com changed reviewers:
+ asiva@google.com, leafp@google.com

[Leaf, 2014-08-26 17:49:20]

lgtm

[siva, 2014-08-26 19:31:14]

https://chromiumcodereview.appspot.com/490033005/diff/1/Source/bindings/dart/...
File Source/bindings/dart/DartDOMWrapper.cpp (right):

https://chromiumcodereview.appspot.com/490033005/diff/1/Source/bindings/dart/...
Source/bindings/dart/DartDOMWrapper.cpp:79: strncat(message,
DartWebkitClassInfo[cid].jsName, 255 - strlen(message));
Why not use
snprint(message, 256, "Unsupported native browser type: %s",
DartWebkitClassInfo[cid].jsName);


Also this new error is overriding the original error that was produced by
'getType' why not do this only under
if (!Dart_IsError(type)) {
  ....
  ....
}
type = Dart_GetType(...);

https://chromiumcodereview.appspot.com/490033005/diff/1/Source/bindings/dart/...
Source/bindings/dart/DartDOMWrapper.cpp:82: type = Dart_GetType(htmlLibrary,
Dart_NewStringFromCString("_UnsupportedBrowserObject"), 0, 0);
I hope this is just a temporary solution, when we start generating
code using the actual chrome IDL instead of the way things are being
done now this should never happen right. Any error here would be more
an InternalError.

[vsm, 2014-08-27 07:48:27]

Committed patchset #2 manually as 180943 (presubmit successful).

[vsm, 2014-08-27 07:50:29]

https://chromiumcodereview.appspot.com/490033005/diff/1/Source/bindings/dart/...
File Source/bindings/dart/DartDOMWrapper.cpp (right):

https://chromiumcodereview.appspot.com/490033005/diff/1/Source/bindings/dart/...
Source/bindings/dart/DartDOMWrapper.cpp:79: strncat(message,
DartWebkitClassInfo[cid].jsName, 255 - strlen(message));
On 2014/08/26 19:31:14, siva wrote:
> Why not use
> snprint(message, 256, "Unsupported native browser type: %s",
> DartWebkitClassInfo[cid].jsName);
> 
> 
> Also this new error is overriding the original error that was produced by
> 'getType' why not do this only under
> if (!Dart_IsError(type)) {
>   ....
>   ....
> }
> type = Dart_GetType(...);

Done.

[vsm, 2014-08-27 08:05:11]

https://chromiumcodereview.appspot.com/490033005/diff/1/Source/bindings/dart/...
File Source/bindings/dart/DartDOMWrapper.cpp (right):

https://chromiumcodereview.appspot.com/490033005/diff/1/Source/bindings/dart/...
Source/bindings/dart/DartDOMWrapper.cpp:82: type = Dart_GetType(htmlLibrary,
Dart_NewStringFromCString("_UnsupportedBrowserObject"), 0, 0);
As long as we continue to push the types to user code, we do need something
defensive here.  We cannot assume we'll get a type or a valid type.

Should we also verify that this type has (at least) two native fields?  What
happens if we call Dart_AllocateWithNativeFields on some arbitrary type?

On 2014/08/26 19:31:14, siva wrote:
> I hope this is just a temporary solution, when we start generating
> code using the actual chrome IDL instead of the way things are being
> done now this should never happen right. Any error here would be more
> an InternalError.