795132 - Cache DPI to avoid making JNI calls

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

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.

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

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

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch 2

put the cache in the bridge.

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

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.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

added the sDPI reset
http://hg.mozilla.org/integration/mozilla-inbound/rev/204ba80209a2

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/204ba80209a2