1261965 - [infer] Errors in FilePickerResultHandler

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:101: error: NULL_DEREFERENCE
  object cursor last assigned on line 99 could be null and is dereferenced at line 101

--
/var/lib/jenkins/workspace/fennec-infer/mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:238: error: RESOURCE_LEAK
   resource acquired to fos by call to FileOutputStream(...) at line 222 is not released after line 238

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1261965 - Correct leak FileOutputStream in FilePickerResultHandler. r=grisha

Review commit: https://reviewboard.mozilla.org/r/44205/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44205/

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1261965 - Correct potential NullPointerException in FilePickerResultHandler. r=grisha

Review commit: https://reviewboard.mozilla.org/r/44207/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44207/

Comment 3

[:Grisha Kruglov]

Comment on attachment 8737955 [details]
MozReview Request: Bug 1261965 - Correct leak FileOutputStream in FilePickerResultHandler. r=grisha

https://reviewboard.mozilla.org/r/44205/#review41001

Looks good

::: mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:198
(Diff revision 1)
>                                      null); // sortOrder
>          }
>  
>          @Override
>          public void onLoadFinished(Loader<Cursor> loader, Cursor cursor) {
>              if (cursor.moveToFirst()) {

Since you're touching this, maybe short-circuit out of the function here?

Comment 4

[:Grisha Kruglov]

Comment on attachment 8737956 [details]
MozReview Request: Bug 1261965 - Correct potential NullPointerException in FilePickerResultHandler. r=grisha

https://reviewboard.mozilla.org/r/44207/#review41005

looks good other than little typo

::: mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:97
(Diff revision 1)
>          final FragmentActivity fa = (FragmentActivity) GeckoAppShell.getGeckoInterface().getActivity();
>          final LoaderManager lm = fa.getSupportLoaderManager();
> +
>          // Finally, Video pickers and some file pickers may return a content provider.
> -        Cursor cursor = null;
> +        final ContentResolver cr = fa.getContentResolver();
> +        final Cursor cursor = cr.query(uri, new String[] { MediaStore.Video.Media.DATA }, null, null, null);;

two semicolons at the end of the line here

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/44205/#review41001

> Since you're touching this, maybe short-circuit out of the function here?

If I was making more drastic changes, I'd consider it but it'll change the indentations and thus the commit history so I don't think it's worth it.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://reviewboard.mozilla.org/r/44207/#review41005

> two semicolons at the end of the line here

Who would have thought? It compiles with two semi-colons! (...but I guess that makes sense)

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8737956 [details]
MozReview Request: Bug 1261965 - Correct potential NullPointerException in FilePickerResultHandler. r=grisha

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44207/diff/1-2/

Comment 8

[:Grisha Kruglov]

Comment on attachment 8737956 [details]
MozReview Request: Bug 1261965 - Correct potential NullPointerException in FilePickerResultHandler. r=grisha

https://reviewboard.mozilla.org/r/44207/#review41789

+1

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/18434342c5638551756e00bec2463c30a4a9abcc
Bug 1261965 - Correct leak FileOutputStream in FilePickerResultHandler. r=grisha

https://hg.mozilla.org/integration/fx-team/rev/81f58f4715ac5ec76ee7514ac172c2c66b1df6e1
Bug 1261965 - Correct potential NullPointerException in FilePickerResultHandler. r=grisha

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/18434342c563
https://hg.mozilla.org/mozilla-central/rev/81f58f4715ac