Model `android.view.View.findViewById` as an expensive method

Summary: public
The method `android.view.View.findViewById` and should not be run performance critical parts of the code like scrolling.

Reviewed By: sblackshear

Differential Revision: D2698196

fb-gh-sync-id: 2716ad7

infer/src/backend/interproc.ml

@@ -631,7 +631,8 @@ let report_context_leaks pname sigma tenv =
     IList.fold_left
       (fun exps hpred -> match hpred with
          | Sil.Hpointsto (_, Sil.Eexp (exp, _), Sil.Sizeof (Sil.Tptr (typ, _), _))
-           when AndroidFramework.is_context typ tenv ->
+           when AndroidFramework.is_context typ tenv
+                && not (AndroidFramework.is_application typ tenv) ->
              (exp, typ) :: exps
          | _ -> exps)
       []

infer/src/backend/mangled.ml

@@ -57,7 +57,8 @@ let get_mangled pn = match pn.mangled with
 
 (** Create a mangled type name from a package name and a class name *)
 let from_package_class package_name class_name =
-  from_string (package_name ^ "." ^ class_name)
+  if package_name = "" then from_string class_name
+  else from_string (package_name ^ "." ^ class_name)
 
 (** Pretty print a mangled name *)
 let pp f pn =

infer/src/backend/procname.ml

@@ -220,8 +220,14 @@ let java_get_class_components proc_name =
   Str.split (Str.regexp (Str.quote ".")) (java_get_class proc_name)
 
 (** Return the class name of a java procedure name. *)
-let java_get_simple_class proc_name =
+let java_get_simple_class = function
-  IList.hd (IList.rev (java_get_class_components proc_name))
+  | JAVA j -> snd j.classname
+  | _ -> assert false
+
+(** Return the package of a java procname. *)
+let java_get_package = function
+  | JAVA j -> fst j.classname
+  | _ -> assert false
 
 (** Return the method of a java procname. *)
 let java_get_method = function

infer/src/backend/procname.mli

@@ -84,6 +84,9 @@ val java_get_class : t -> string
 (** Return the simple class name of a java procedure name. *)
 val java_get_simple_class : t -> string
 
+(** Return the package name of a java procedure name. *)
+val java_get_package : t -> string option
+
 (** Return the method name of a java procedure name. *)
 val java_get_method : t -> string
 

infer/src/checkers/performanceCritical.ml

@@ -59,8 +59,26 @@ let method_overrides_performance_critical tenv pname =
   || overrides ()
 
 
-let method_is_expensive pname =
+let is_modeled_expensive tenv pname =
-  check_method Annotations.ia_is_expensive pname
+  if SymExec.function_is_builtin pname then false
+  else if Procname.java_get_method pname <> "findViewById" then false
+  else
+    let package =
+      match Procname.java_get_package pname with
+      | None -> ""
+      | Some p -> p in
+    let classname =
+      Mangled.from_package_class package (Procname.java_get_simple_class pname) in
+    match Sil.get_typ classname (Some Sil.Class) tenv with
+    | None -> false
+    | Some typ ->
+        AndroidFramework.is_view typ tenv
+        || AndroidFramework.is_activity typ tenv
+
+
+let method_is_expensive tenv pname =
+  is_modeled_expensive tenv pname
+  || check_method Annotations.ia_is_expensive pname
 
 
 let lookup_call_trees pname =
@@ -74,13 +92,13 @@ let lookup_call_trees pname =
       end
 
 
-let collect_expensive_call caller_pdesc checked_pnames call_trees (pname, _) =
+let collect_expensive_call tenv caller_pdesc checked_pnames call_trees (pname, _) =
   if Procname.Set.mem pname !checked_pnames then call_trees
   else
     begin
       Ondemand.do_analysis caller_pdesc pname;
       checked_pnames := Procname.Set.add pname !checked_pnames;
-      if method_is_expensive pname then
+      if method_is_expensive tenv pname then
         (CallTree.Direct pname) :: call_trees
       else
         match lookup_call_trees pname with
@@ -134,7 +152,7 @@ let check_one_procedure tenv pname pdesc =
   and performance_critical = is_performance_critical attributes in
 
   let check_expensive_subtyping_rules overridden_pname =
-    if not (method_is_expensive overridden_pname) then
+    if not (method_is_expensive tenv overridden_pname) then
       let description =
         Printf.sprintf
           "Method `%s` overrides unannotated method `%s` and cannot be annotated with `@%s`"
@@ -165,21 +183,21 @@ let check_one_procedure tenv pname pdesc =
   let expensive_call_trees =
     let checked_pnames = ref Procname.Set.empty in
     Cfg.Procdesc.fold_calls
-      (collect_expensive_call pdesc checked_pnames) [] pdesc in
+      (collect_expensive_call tenv pdesc checked_pnames) [] pdesc in
 
   update_summary expensive_call_trees pname;
 
   match expensive_call_trees with
   | [] -> ()
+  | call_trees when performance_critical ->
+      report_expensive_calls pname pdesc loc call_trees
   | call_trees when infer_performance_critical_methods ->
       if method_overrides_performance_critical tenv pname then
         report_expensive_calls pname pdesc loc call_trees
-  | call_trees when performance_critical ->
-      report_expensive_calls pname pdesc loc call_trees
   | _ -> ()
 
 
-let callback_performance_checker _ get_proc_desc _ tenv pname proc_desc =
+let callback_performance_checker _ get_proc_desc _ tenv pname pdesc =
   let callbacks =
     let analyze_ondemand pn =
       match get_proc_desc pn with
@@ -187,10 +205,10 @@ let callback_performance_checker _ get_proc_desc _ tenv pname proc_desc =
       | Some pd -> check_one_procedure tenv pn pd in
     { Ondemand.analyze_ondemand; get_proc_desc; } in
   if !Config.ondemand_enabled
-  || Ondemand.procedure_should_be_analyzed proc_desc pname
+     || Ondemand.procedure_should_be_analyzed pdesc pname
   then
     begin
       Ondemand.set_callbacks callbacks;
-      check_one_procedure tenv pname proc_desc;
+      check_one_procedure tenv pname pdesc;
       Ondemand.unset_callbacks ()
     end

infer/src/harness/androidFramework.ml

@@ -284,15 +284,23 @@ let get_all_supertypes typ tenv =
 let is_subtype (typ0 : Sil.typ) (typ1 : Sil.typ) tenv =
   TypSet.mem typ1 (get_all_supertypes typ0 tenv)
 
-(** return true if [typ] <: android.content.Context *)
+let is_subtype_package_class typ package classname tenv =
-let is_context typ tenv =
+  let classname = Mangled.from_package_class package classname in
-  let context_classname = Mangled.from_package_class "android.content" "Context"
-  and application_classname = Mangled.from_package_class "android.app" "Application" in
-  let subclass_of classname =
   match Sil.get_typ classname (Some Sil.Class) tenv with
-    | Some class_typ -> is_subtype typ class_typ tenv
+  | Some found_typ -> is_subtype typ found_typ tenv
-    | None -> false in
+  | _ -> false
-  subclass_of context_classname && not (subclass_of application_classname)
+
+let is_context typ tenv =
+  is_subtype_package_class typ "android.content" "Context" tenv
+
+let is_application typ tenv =
+  is_subtype_package_class typ "android.app" "Application" tenv
+
+let is_activity typ tenv =
+  is_subtype_package_class typ "android.app" "Activity" tenv
+
+let is_view typ tenv =
+  is_subtype_package_class typ "android.view" "View" tenv
 
 (** return true if [class_name] is a known callback class name *)
 let is_callback_class_name class_name = Mangled.MangledSet.mem class_name android_callbacks

infer/src/harness/androidFramework.mli

@@ -33,6 +33,15 @@ val is_callback_class : Sil.typ -> Sil.tenv -> bool
 (** return true if [typ] <: android.content.Context *)
 val is_context : Sil.typ -> Sil.tenv -> bool
 
+(** return true if [typ] <: android.app.Application *)
+val is_application : Sil.typ -> Sil.tenv -> bool
+
+(** return true if [typ] <: android.app.Activity *)
+val is_activity : Sil.typ -> Sil.tenv -> bool
+
+(** return true if [typ] <: android.view.View *)
+val is_view : Sil.typ -> Sil.tenv -> bool
+
 (** return true if [procname] is a special lifecycle cleanup method *)
 val is_destroy_method : Procname.t -> bool
 

infer/tests/codetoanalyze/java/checkers/BUCK

@@ -1,6 +1,7 @@
 sources = glob(['**/*.java'])
 
 dependencies = [
+    '//dependencies/java/android/support/v4:android-support-v4',
     '//infer/annotations:annotations',
     '//infer/lib/java/android:android',
 ]

infer/tests/codetoanalyze/java/checkers/ExpensiveCallExample.java

@@ -9,6 +9,10 @@
 
 package codetoanalyze.java.checkers;
 
+import android.support.v4.app.FragmentActivity;
+import android.widget.ImageView;
+import android.view.View;
+
 import com.facebook.infer.annotation.Expensive;
 import com.facebook.infer.annotation.PerformanceCritical;
 
@@ -70,4 +74,14 @@ public class ExpensiveCallExample {
     callsExpensive2();
   }
 
+  @PerformanceCritical
+  View callsFindViewByIdFromView(ImageView view, int id) {
+    return view.findViewById(id);
+  }
+
+  @PerformanceCritical
+  View callsFindViewByIdFromActivity(FragmentActivity activity, int id) {
+    return activity.findViewById(id);
+  }
+
 }

infer/tests/endtoend/java/checkers/ExpensiveCallTest.java

@@ -44,6 +44,8 @@ public class ExpensiveCallTest {
         "indirectlyCallingExpensiveMethod",
         "callingExpensiveMethodFromInterface",
         "longerCallStackToExpensive",
+        "callsFindViewByIdFromView",
+        "callsFindViewByIdFromActivity",
     };
     assertThat(
         "Results should contain " + CALLS_EXPENSIVE_METHOD,