Closed Bug 795132 Opened 8 years ago Closed 8 years ago

Cache DPI to avoid making JNI calls

Categories

(Core :: Widget: Android, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: mfinkle, Assigned: mfinkle)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
nsWindow::GetDPI is called a lot, especially during a page load. This calls the AndroidBridge which does a JNI call to a Java method. Since DPI is fairly constant during runtime, we could cache the result in nsWindow. We did this in Maemo too:

http://mxr.mozilla.org/mozilla-central/source/widget/gtk2/nsWindow.cpp#718

The patch also uses 160 as the default in the bridge, since it's what we use in nsWindow.
Attachment #665666 - Flags: review?(blassey.bugs)
Comment on attachment 665666 [details] [diff] [review]
patch

Review of attachment 665666 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/android/AndroidBridge.cpp
@@ +778,5 @@
>  
>  int
>  AndroidBridge::GetDPI()
>  {
>      ALOG_BRIDGE("AndroidBridge::GetDPI");

static int sDPI = 0;
if (sDPI)
    return sDPI;

@@ +785,5 @@
>      if (!env)
>          return DEFAULT_DPI;
>      AutoLocalJNIFrame jniFrame(env);
>  
>      int dpi = (int)env->CallStaticIntMethod(mGeckoAppShellClass, jGetDpi);

s/int dpi/sDPI

::: widget/android/nsWindow.cpp
@@ +324,5 @@
>  nsWindow::GetDPI()
>  {
> +    static float sDPI = 0;
> +
> +    if (!sDPI && AndroidBridge::Bridge())

using a float as a boolean value scares me. Let's just contain this to AndroidBridge
Attachment #665666 - Flags: review?(blassey.bugs) → review-
Attached patch patch 2Splinter Review
put the cache in the bridge.
Assignee: nobody → mark.finkle
Attachment #665666 - Attachment is obsolete: true
Attachment #665891 - Flags: review?(blassey.bugs)
Comment on attachment 665891 [details] [diff] [review]
patch 2

Review of attachment 665891 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/android/AndroidBridge.cpp
@@ +794,2 @@
>      if (jniFrame.CheckForException())
>          return DEFAULT_DPI;

you probably want to set sDPI to 0 in the case of an exception so we don't return it on the next call.
Attachment #665891 - Flags: review?(blassey.bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/204ba80209a2
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.