Bug 39426

Summary:

Calls to CallJNIMethodIDA() in JavaInstance::invokeMethod() are required on Android

Product:

WebKit

Reporter:

Steve Block <steveblock>

Component:

WebKit Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

android-webkit-unforking, ap, mjs, steveblock

Priority:

Version:

528+ (Nightly build)

Hardware:

Android

OS:

Android

Attachments:

Description	Flags
Patch	none

Steve Block

Reported 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.

Attachments
Patch (1.13 KB, patch) 2010-05-20 08:03 PDT, Steve Block	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Steve Block

Comment 1 2010-05-20 08:03:11 PDT

Created attachment 56595 [details] Patch

Alexey Proskuryakov

Comment 2 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.

Steve Block

Comment 3 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.

Steve Block

Comment 4 2010-05-21 01:54:33 PDT

Comment on attachment 56595 [details] Patch Landed manually as http://trac.webkit.org/changeset/59918

Steve Block

Comment 5 2010-05-21 01:54:53 PDT

Closing bug as resolved fixed.

Alexey Proskuryakov

Comment 6 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?

Steve Block

Comment 7 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.

Note You need to log in before you can comment on or make changes to this bug.