Setup a peer for read only strings that are not externalized in

- Setup a peer for read only strings that are not externalized in
Dart_MakeExternalString so that it is not necessary to create
a new reference for it everytime it is accessed in native code.

- Removed unused function Dart_ExternalStringGetPeer

R=iposva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=26538

[Ivan Posva, 2013-08-22 15:44:51]

LGTM with comments.

-Ivan

https://codereview.chromium.org/22980022/diff/27001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/22980022/diff/27001/runtime/include/dart_api....
runtime/include/dart_api.h:1560: * \param peer Returns the peer pointer if the
String is an external String.
Please update this comment.

https://codereview.chromium.org/22980022/diff/27001/runtime/include/dart_api....
runtime/include/dart_api.h:1999: * \param peer Returns the peer pointer if the
String is an external String.
Please update this comment.

https://codereview.chromium.org/22980022/diff/27001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/22980022/diff/27001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:243: #define EXTERNAL_PEER_HELPER(type, ctype,
raw_obj, peer)                       \
Is this #define really worth the added confusion? To me it feels like the code
would be more readable if we just wrote the code twice with the appropriate
types inline.

https://codereview.chromium.org/22980022/diff/27001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:260: *peer =
Isolate::Current()->heap()->GetPeer(raw_obj);
args carries the isolate or pass it in from the caller?

https://codereview.chromium.org/22980022/diff/27001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:3676: return Api::Success();
This function does not do what it is documented to do: Return the string object
handle if no error occurs.

[siva, 2013-08-22 16:57:07]

https://codereview.chromium.org/22980022/diff/27001/runtime/include/dart_api.h
File runtime/include/dart_api.h (right):

https://codereview.chromium.org/22980022/diff/27001/runtime/include/dart_api....
runtime/include/dart_api.h:1560: * \param peer Returns the peer pointer if the
String is an external String.
On 2013/08/22 15:44:51, Ivan Posva wrote:
> Please update this comment.

Done.

https://codereview.chromium.org/22980022/diff/27001/runtime/include/dart_api....
runtime/include/dart_api.h:1999: * \param peer Returns the peer pointer if the
String is an external String.
On 2013/08/22 15:44:51, Ivan Posva wrote:
> Please update this comment.

Done.

https://codereview.chromium.org/22980022/diff/27001/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/22980022/diff/27001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:243: #define EXTERNAL_PEER_HELPER(type, ctype,
raw_obj, peer)                       \
Removed the macro and inline the code below.

On 2013/08/22 15:44:51, Ivan Posva wrote:
> Is this #define really worth the added confusion? To me it feels like the code
> would be more readable if we just wrote the code twice with the appropriate
> types inline.

https://codereview.chromium.org/22980022/diff/27001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:260: *peer =
Isolate::Current()->heap()->GetPeer(raw_obj);
Good point.

On 2013/08/22 15:44:51, Ivan Posva wrote:
> args carries the isolate or pass it in from the caller?

https://codereview.chromium.org/22980022/diff/27001/runtime/vm/dart_api_impl....
runtime/vm/dart_api_impl.cc:3676: return Api::Success();
Updated comment to reflect this. If a peer is present then the
caller is only interested in the peer.

On 2013/08/22 15:44:51, Ivan Posva wrote:
> This function does not do what it is documented to do: Return the string
object
> handle if no error occurs.

[siva, 2013-08-22 17:10:43]

Committed patchset #6 manually as r26538 (presubmit successful).