[Android] Fix findbugs errors.

[Android] Fix findbugs errors.

These were failing on Android Builder (dbg) FYI bot.

BUG=409152
TBR=qinmin@chromium.org

Committed: https://crrev.com/a29dd6ccf30bda316f32e17952dc2258103b2801
Cr-Commit-Position: refs/heads/master@{#293206}

[newt (away), 2014-08-29 20:48:26]

newt@chromium.org changed reviewers:
+ qinmin@chromium.org, torne@chromium.org, yfriedman@chromium.org

[newt (away), 2014-08-29 20:48:27]

+torne: android_webview/.../HttpAuthDatabase.java
+yfriedman: net/* and content/* 
+qinmin: media/.../MediaDrmBridge.java

[hush (inactive), 2014-08-29 23:17:22]

hush@chromium.org changed reviewers:
+ hush@chromium.org

[hush (inactive), 2014-08-29 23:17:23]

https://codereview.chromium.org/522943003/diff/1/android_webview/java/src/org...
File android_webview/java/src/org/chromium/android_webview/HttpAuthDatabase.java
(right):

https://codereview.chromium.org/522943003/diff/1/android_webview/java/src/org...
android_webview/java/src/org/chromium/android_webview/HttpAuthDatabase.java:66:
AsyncTask.THREAD_POOL_EXECUTOR.execute(new Runnable() {
sorry for the drive-by comment...
The issue that findbugs is not happy about is that if you have a child class
extending "HttpAuthDatabase", the child class constructor will call
HttpAuthDatabase constructor first. So the thread may see the incompletely
initialized child class.
Switching this to an AsyncTask does not help I guess, because when the AsyncTask
gets run, it could still see the incompletely initialized child class.


http://findbugs.sourceforge.net/bugDescriptions.html#SC_START_IN_CTOR

is making HttpAuthDatabase a final class a bad thing to do?

[Torne, 2014-09-01 11:18:06]

https://codereview.chromium.org/522943003/diff/1/android_webview/java/src/org...
File android_webview/java/src/org/chromium/android_webview/HttpAuthDatabase.java
(right):

https://codereview.chromium.org/522943003/diff/1/android_webview/java/src/org...
android_webview/java/src/org/chromium/android_webview/HttpAuthDatabase.java:66:
AsyncTask.THREAD_POOL_EXECUTOR.execute(new Runnable() {
On 2014/08/29 23:17:22, hush wrote:
> sorry for the drive-by comment...
> The issue that findbugs is not happy about is that if you have a child class
> extending "HttpAuthDatabase", the child class constructor will call
> HttpAuthDatabase constructor first. So the thread may see the incompletely
> initialized child class.
> Switching this to an AsyncTask does not help I guess, because when the
AsyncTask
> gets run, it could still see the incompletely initialized child class.
> http://findbugs.sourceforge.net/bugDescriptions.html#SC_START_IN_CTOR

Yeah, this doesn't actually solve the real problem, it just makes it no longer
match a pattern that findbugs can recognise.

> is making HttpAuthDatabase a final class a bad thing to do?

That's not really any better either since if the constructor were to do anything
else after the start() it will still be potentially accessing this with improper
synchronisation :)

The right fix for this is to stop spawning the background task from the
constructor at all. Add a public method on this object which will trigger
initialisation, and have AwContents call that method after it constructs the
object. This is only constructed in one place and is not an external interface,
so that's simple and guaranteed to be safe.

[newt (away), 2014-09-02 21:15:23]

https://codereview.chromium.org/522943003/diff/1/android_webview/java/src/org...
File android_webview/java/src/org/chromium/android_webview/HttpAuthDatabase.java
(right):

https://codereview.chromium.org/522943003/diff/1/android_webview/java/src/org...
android_webview/java/src/org/chromium/android_webview/HttpAuthDatabase.java:66:
AsyncTask.THREAD_POOL_EXECUTOR.execute(new Runnable() {
On 2014/09/01 11:18:06, Torne wrote:
> On 2014/08/29 23:17:22, hush wrote:
> > sorry for the drive-by comment...
> > The issue that findbugs is not happy about is that if you have a child class
> > extending "HttpAuthDatabase", the child class constructor will call
> > HttpAuthDatabase constructor first. So the thread may see the incompletely
> > initialized child class.
> > Switching this to an AsyncTask does not help I guess, because when the
> AsyncTask
> > gets run, it could still see the incompletely initialized child class.
> > http://findbugs.sourceforge.net/bugDescriptions.html#SC_START_IN_CTOR
> 
> Yeah, this doesn't actually solve the real problem, it just makes it no longer
> match a pattern that findbugs can recognise.
> 
> > is making HttpAuthDatabase a final class a bad thing to do?
> 
> That's not really any better either since if the constructor were to do
anything
> else after the start() it will still be potentially accessing this with
improper
> synchronisation :)
> 
> The right fix for this is to stop spawning the background task from the
> constructor at all. Add a public method on this object which will trigger
> initialisation, and have AwContents call that method after it constructs the
> object. This is only constructed in one place and is not an external
interface,
> so that's simple and guaranteed to be safe.

Thanks for the comments. I clearly wasn't paying enough attention to why
findbugs was complaining. Should be fixed now.

[Yaron, 2014-09-03 00:42:36]

net and content lgtm

[Torne, 2014-09-03 09:30:46]

android_webview LGTM

[newt (away), 2014-09-03 18:27:57]

The CQ bit was checked by newt@chromium.org

[commit-bot: I haz the power, 2014-09-03 18:38:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/newt@chromium.org/522943003/20001

[commit-bot: I haz the power, 2014-09-03 21:50:24]

Committed patchset #2 (id:20001) as b2bd9643c839d526482f5ee3ce6acaf92a610f39

[commit-bot: I haz the power, 2014-09-10 03:28:05]

Patchset 2 (id:??) landed as
https://crrev.com/a29dd6ccf30bda316f32e17952dc2258103b2801
Cr-Commit-Position: refs/heads/master@{#293206}