Last Comment Bug 738049 - Use LayoutInflater.Factory to optimize rendering custom views
: Use LayoutInflater.Factory to optimize rendering custom views
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: Keyboards and IME (show other bugs)
: unspecified
: ARM Android
: -- normal (vote)
: Firefox 14
Assigned To: Sriram Ramasubramanian [:sriram]
:
: Jim Chen [:jchen] [:darchons]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-21 14:58 PDT by Sriram Ramasubramanian [:sriram]
Modified: 2012-04-04 10:26 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
Patch (6.57 KB, patch)
2012-03-21 15:27 PDT, Sriram Ramasubramanian [:sriram]
mark.finkle: review+
Details | Diff | Splinter Review

Description Sriram Ramasubramanian [:sriram] 2012-03-21 14:58:00 PDT
There is a LayoutInflater associated with every activity. This layout inflater is responsible for inflating views from XML. This is aware of "android.widget" namescape for creating Views. However, our custom views, like AboutHomeSection, take a round about way:
1. I tries to search if any factory can supply this.
2. It tried to find in default namespace.
3. It tries to resolve the view based on "package name" and "class name".

This can be avoided by asking it to search in the factory we supply to it. This will optimize the search done by android while inflating our custom views.
Comment 1 Sriram Ramasubramanian [:sriram] 2012-03-21 15:27:09 PDT
Created attachment 608116 [details] [diff] [review]
Patch

This patch uses a LayoutInflater.Factory. I felt it's better to use a singleton class that handles all the custom views, than every activity "implementing" LayoutInflater.Factory. 

This is the first attempt at it. Ideally, its better to hash all the view names in a HashMap for easier access.
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2012-03-21 16:36:32 PDT
Comment on attachment 608116 [details] [diff] [review]
Patch


>diff --git a/mobile/android/base/GeckoViewsFactory.java b/mobile/android/base/GeckoViewsFactory.java

>+    private static final String GECKO_VIEW_IDENTIFIER = "org.mozilla.gecko.";
>+
>+    private static final int GECKO_VIEW_IDENTIFIER_LENGTH = GECKO_VIEW_IDENTIFIER.length();

remove the extra blank line

>+    public View onCreateView(String name, Context context, AttributeSet attrs) {
>+
>+        if (!TextUtils.isEmpty(name) && name.startsWith(GECKO_VIEW_IDENTIFIER)) {

remove the extra blank line

>+            if (TextUtils.equals(viewName, "AboutHomeSection"))
>+                return new AboutHomeSection(context, attrs);
>+            else if (TextUtils.equals(viewName, "AwesomeBarTabs"))
>+                    return new AwesomeBarTabs(context, attrs);
>+            else if (TextUtils.equals(viewName, "FormAssistPopup"))
>+                    return new FormAssistPopup(context, attrs);
>+            else if (TextUtils.equals(viewName, "LinkTextView"))
>+                    return new LinkTextView(context, attrs);

indented too far in the "else if" blocks

r+ with nits fixed
Comment 3 Sriram Ramasubramanian [:sriram] 2012-03-23 12:02:31 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/b28c614c6fe3
Comment 4 Ed Morley [:emorley] 2012-03-24 13:58:01 PDT
Please can you set the target milestone & assignee when landing on inbound, along the lines of http://blog.bonardo.net/2012/03/23/how-you-can-help-mozilla-inbound-sheriffs-when-pushing :-)

https://hg.mozilla.org/mozilla-central/rev/b28c614c6fe3
Comment 5 Mark Finkle (:mfinkle) (use needinfo?) 2012-04-04 10:26:34 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/bd0d926a44f4

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