Properly throw Dart error from PluginUtilities.getCallbackHandle FFI

[moffatman]

Restore dart error propagation which was missing since refactoring for FFI.

Not sure how to test this as the engine dart tests seem to not perform the level of optimizations required to reproduce my failure case (handle to optimized-out function).

Fixes #111243

Pre-launch Checklist

• I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
• I read the [Tree Hygiene] wiki page, which explains my responsibilities.
• I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the [CLA].
• All existing and new tests are passing.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[dcharkes]

handle to optimized-out function

A test needs to run in AOT to optimize out functions, in JIT mode all functions will be there.

[moffatman]

I found that even in host_release test, it wasn't getting optimized out. Might it be required to do full APK / Xcode build to get all the optimizing?

[jonahwilliams]

We only ever run unit tests in debug mode, so it is not possible to unit test a change that fixes a release mode issue. That said, is it possible to reproduce the same error by using a bogus ID?

[dnfield]

On the engine we run tests in release mode

[dnfield]

@moffatman can you add the test case to //testing/dart? I'd expect it to work in at least some runtime mode.

[moffatman]

@dnfield Added the broken test. I believe all the tests are compiled as "kernel" which is not the same as aot, even in release mode?

[moffatman]

@jonahwilliams Bogus ID will just return a null callback, it won't hit the same error case.

[dnfield]

I think we should skip this test like this:

if (!const bool.fromEnvironment('dart.vm.product')) {
  return;
}

I'd expec tthis test to pass for the host_release tests...

[moffatman]

That value is false even in host_release

[dnfield]

That's unfortunate.

I don't think we should try to add more dart dfines for this.

Perhaps the right option here would be to move this file to a different directory and update run_tests.py so that it only picks this file if doing the host_release build.

First, I would recommend verifying that this test does in fact pass on host_release.

Alternatively, you could write this as a shell_unittest and there you could check whether FLUTTER_RUNTIME_MODE is release or not in the C++ part of the test.

[dnfield]

(also, if it's easier to discuss this happy to talk more on discord or over a vc)

[chinmaygarde]

Ping @dnfield

[dnfield]

LGTM