Bug 39426 - Calls to CallJNIMethodIDA() in JavaInstance::invokeMethod() are required on Android
Summary: Calls to CallJNIMethodIDA() in JavaInstance::invokeMethod() are required on A...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Android Android
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-20 08:00 PDT by Steve Block
Modified: 2010-05-21 08:49 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.13 KB, patch)
2010-05-20 08:03 PDT, Steve Block
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Block 2010-05-20 08:00:20 PDT
These calls were guarded with BUILDING_ON_TIGER in http://trac.webkit.org/changeset/55054 but they are required on Android too.
Comment 1 Steve Block 2010-05-20 08:03:11 PDT
Created attachment 56595 [details]
Patch
Comment 2 Alexey Proskuryakov 2010-05-20 14:11:16 PDT
Comment on attachment 56595 [details]
Patch

I would appreciate a more detailed explanation of the change. How is this a build fix? Why are these calls required?

I'm going to say r=me since this helps Android, but it's a deprecated code path, so an explanation of why it's needed (and when it can be finally removed) would help a lot.
Comment 3 Steve Block 2010-05-21 01:51:10 PDT
Thanks for the review Alexey.

> I would appreciate a more detailed explanation of the change. How is this a
> build fix?
Android has always used this code path, so adding the BUILDING_ON_TIGER guards broke the Android build. The android port is not yet fully upstreamed and we don't sync WebKit too frequently, so it took us a while to notice the break.

> Why are these calls required?
I'm not sure. I'm not too familiar with this code, but have filed Bug 39476 to track this.

I'll land the patch now so as to fix the build, but with an additional comment about the need to avoid the use of this deprecated code path.
Comment 4 Steve Block 2010-05-21 01:54:33 PDT
Comment on attachment 56595 [details]
Patch

Landed manually as http://trac.webkit.org/changeset/59918
Comment 5 Steve Block 2010-05-21 01:54:53 PDT
Closing bug as resolved fixed.
Comment 6 Alexey Proskuryakov 2010-05-21 08:40:42 PDT
But how does this missing block of code break the build? I would understand if it broke behavior, but I don't understand breaking the build. How did the compiler complain?
Comment 7 Steve Block 2010-05-21 08:49:02 PDT
(In reply to comment #6)
> But how does this missing block of code break the build? I would understand if
> it broke behavior, but I don't understand breaking the build. How did the
> compiler complain?
Sorry, 'build' was misleading. Removing this code block caused LayoutTestController to stop working, so lots of the LayoutTests started failing.