VM: Evaluate-in-frame should happen in the scope of the method class, not the receiver class.

VM: Evaluate-in-frame should happen in the scope of the method class, not the receiver class.

BUG=http://dartbug.com/26161
R=hausner@google.com

Committed: https://github.com/dart-lang/sdk/commit/87e8bc2c96d2a90ae701d10fa986fcb63a1f102e

[rmacnak, 2016-04-04 19:32:59]

rmacnak@google.com changed reviewers:
+ hausner@google.com

[hausner, 2016-04-04 19:52:32]

LGTM w/comment

https://codereview.chromium.org/1854243002/diff/1/runtime/observatory/tests/s...
File
runtime/observatory/tests/service/evaluate_activation_in_method_class_test.dart
(right):

https://codereview.chromium.org/1854243002/diff/1/runtime/observatory/tests/s...
runtime/observatory/tests/service/evaluate_activation_in_method_class_test.dart:12:

Would be nice to have a comment in this tests that explains what is being
tested.

https://codereview.chromium.org/1854243002/diff/1/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/1854243002/diff/1/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1074: const Class& method_cls =
Class::Handle(function().Owner());
I think this should be the method's origin(), not Owner(). For a function that
is from a mixin class, you want the class where the function is defined, not the
class to which the function is "mixed in". The difference is noticeable when you
refer to static members or the library of the mixin class.

Your test should cover that aspect as well, ideally.

[rmacnak, 2016-04-04 22:07:26]

https://codereview.chromium.org/1854243002/diff/1/runtime/observatory/tests/s...
File
runtime/observatory/tests/service/evaluate_activation_in_method_class_test.dart
(right):

https://codereview.chromium.org/1854243002/diff/1/runtime/observatory/tests/s...
runtime/observatory/tests/service/evaluate_activation_in_method_class_test.dart:12:

On 2016/04/04 19:52:32, hausner wrote:
> Would be nice to have a comment in this tests that explains what is being
> tested.

Done.

https://codereview.chromium.org/1854243002/diff/1/runtime/vm/debugger.cc
File runtime/vm/debugger.cc (right):

https://codereview.chromium.org/1854243002/diff/1/runtime/vm/debugger.cc#newc...
runtime/vm/debugger.cc:1074: const Class& method_cls =
Class::Handle(function().Owner());
On 2016/04/04 19:52:32, hausner wrote:
> I think this should be the method's origin(), not Owner(). For a function that
> is from a mixin class, you want the class where the function is defined, not
the
> class to which the function is "mixed in". The difference is noticeable when
you
> refer to static members or the library of the mixin class.

Good catch.

> Your test should cover that aspect as well, ideally.

Added.

[rmacnak, 2016-04-04 22:23:02]

Description was changed from

==========
VM: Evaluate-in-frame should happen in the scope of the method class, not the
receiver class.

BUG=http://dartbug.com/26161
==========

to

==========
VM: Evaluate-in-frame should happen in the scope of the method class, not the
receiver class.

BUG=http://dartbug.com/26161
R=hausner@google.com

Committed:
https://github.com/dart-lang/sdk/commit/87e8bc2c96d2a90ae701d10fa986fcb63a1f102e
==========

[rmacnak, 2016-04-04 22:23:03]

Committed patchset #2 (id:20001) manually as
87e8bc2c96d2a90ae701d10fa986fcb63a1f102e (presubmit successful).