Bug 57533 - JavaClass should be an interface and free of JNI types
Summary: JavaClass should be an interface and free of JNI types
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Steve Block
URL:
Keywords:
Depends on:
Blocks: 55383
  Show dependency tree / graph
 
Reported: 2011-03-31 02:58 PDT by Steve Block
Modified: 2011-04-01 16:49 PDT (History)
7 users (show)

See Also:

Attachments
Patch 1 (14.92 KB, patch)
2011-03-31 04:54 PDT, Steve Block 		no flags Details | Formatted Diff | Diff
Patch 2 (14.86 KB, patch)
2011-04-01 03:06 PDT, Steve Block 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Block 2011-03-31 02:58:12 PDT 
This will allow implementations flexibility in how the binding to a Java object is achieved. See Bug 55383.

Note that V8 and JSC use different implementations of JavaClass. This bug tracks fixing V8's implementation only.
Comment 1 Steve Block 2011-03-31 04:54:21 PDT 
Created attachment 87703 [details]
Patch 1
Comment 2 Andrei Popescu 2011-04-01 02:55:19 PDT 
I know you didn't introduce this, but:

> Source/WebCore/bridge/jni/v8/JavaClassJobjectV8.cpp:41
> +        LOG_ERROR("unable to call getClass on instance %p", anInstance);

Why are we printing out the anInstance poninter? I can't see that being useful. Just return?

> Source/WebCore/bridge/jni/v8/JavaClassJobjectV8.cpp:45
> +    int i;

What's with this poor lonely 'i' here? He can live inside the for statements below.

> Source/WebCore/bridge/jni/v8/JavaClassJobjectV8.cpp:65
> +        {

Spurious { } ?
Comment 3 Steve Block 2011-04-01 03:04:23 PDT 
> Why are we printing out the anInstance poninter? I can't see that being useful. Just return?
I agree, but this has been touched recently - http://trac.webkit.org/changeset/82197 - so I don't want to modify it needlessly.

> What's with this poor lonely 'i' here? He can live inside the for statements below.
Done

> Spurious { } ?
Fixed, same problem with copy-paste from JSC which uses a scoped lock.
Comment 4 Steve Block 2011-04-01 03:06:32 PDT 
Created attachment 87835 [details]
Patch 2
Comment 5 Andrei Popescu 2011-04-01 03:08:29 PDT 
LGTM
Comment 6 Jeremy Orlow 2011-04-01 10:27:47 PDT 
Comment on attachment 87835 [details]
Patch 2

rubber stamp = me
Comment 7 WebKit Commit Bot 2011-04-01 12:00:00 PDT 
The commit-queue encountered the following flaky tests while processing attachment 87835 [details]:

fast/loader/recursive-before-unload-crash.html bug 50880 (authors: beidson@apple.com and eric@webkit.org)
The commit-queue is continuing to process your patch.
Comment 8 WebKit Commit Bot 2011-04-01 12:05:15 PDT 
Comment on attachment 87835 [details]
Patch 2

Clearing flags on attachment: 87835

Committed r82702: <http://trac.webkit.org/changeset/82702>
Comment 9 WebKit Commit Bot 2011-04-01 12:05:19 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 10 WebKit Review Bot 2011-04-01 16:49:31 PDT 
http://trac.webkit.org/changeset/82702 might have broken WinCE Release (Build)