[cronet] Fix race to call InitApplicationContext

[cronet] Fix race to call InitApplicationContext

Avoid the race by:
1. Calling InitApplicationContext() from CronetLibraryLoader.ensureInitialized()
2. Remove other calls to InitApplicationContext()
3. Make ChromiumUrlRequestContext() call CronetLibraryLoader.ensureInitialized()
Also:
4. Remove duplicate NetworkChangeNotifier initialization from ChromiumUrlRequestContext()
5. Remove now duplicate CronetLibraryLoader.ensureInitialized() call from ChromiumUrlRequestFactory()

BUG=465392
Committed: https://crrev.com/496c0b55f055b8b421169196a3597246b119bac1
Cr-Commit-Position: refs/heads/master@{#319729}

[pauljensen, 2015-03-09 17:23:47]

pauljensen@chromium.org changed reviewers:
+ mef@chromium.org, mmenke@chromium.org, xunjieli@chromium.org

[pauljensen, 2015-03-09 17:23:47]

PTAL, thanks!

[mef, 2015-03-09 17:28:26]

lgtm

[mmenke, 2015-03-09 18:05:39]

On 2015/03/09 17:28:26, mef wrote:
> lgtm

I'll defer to Misha.  Hrm...Do we really need 3 reviewers on all cronet CLs?

[xunjieli, 2015-03-09 18:13:21]

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/cr...
File components/cronet/android/cronet_library_loader.cc (right):

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/cr...
components/cronet/android/cronet_library_loader.cc:101:
base::i18n::InitializeICU();
Should we move this to CronetOnLoad and maybe after url::Initialize() ? I am not
sure if this has to be on the main thread.

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/ja...
File
components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java
(right):

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/ja...
components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java:133:
String userAgent, int loggingLevel, String config);
Can we also remove the not used appContext argument in
nativeCreateRequestContextAdapter of CronetUrlRequestContext.java ? Thanks!

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/ja...
File
components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java
(right):

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/ja...
components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java:71:
private static native void nativeCronetInitApplicationContext(Context
appContext);
nit: I feel that there are too many "Cronet" in the method names. A little
redundant. How about just using nativeInitOnMainThread and
nativeInitApplicationContext ?

[pauljensen, 2015-03-09 19:04:36]

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/cr...
File components/cronet/android/cronet_library_loader.cc (right):

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/cr...
components/cronet/android/cronet_library_loader.cc:101:
base::i18n::InitializeICU();
On 2015/03/09 18:13:21, xunjieli wrote:
> Should we move this to CronetOnLoad and maybe after url::Initialize() ? I am
not
> sure if this has to be on the main thread.

How about in another CL?  This seems orthogonal to the race condition goal this
CL is trying to address.

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/ja...
File
components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java
(right):

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/ja...
components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java:133:
String userAgent, int loggingLevel, String config);
On 2015/03/09 18:13:21, xunjieli wrote:
> Can we also remove the not used appContext argument in
> nativeCreateRequestContextAdapter of CronetUrlRequestContext.java ? Thanks!

Done.

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/ja...
File
components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java
(right):

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/ja...
components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java:71:
private static native void nativeCronetInitApplicationContext(Context
appContext);
On 2015/03/09 18:13:21, xunjieli wrote:
> nit: I feel that there are too many "Cronet" in the method names. A little
> redundant. How about just using nativeInitOnMainThread and
> nativeInitApplicationContext ?

I didn't want to use InitApplicationContext as that might overlap with the real
base::android::InitApplicationContext.

[xunjieli, 2015-03-09 19:37:51]

lgtm

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/cr...
File components/cronet/android/cronet_library_loader.cc (right):

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/cr...
components/cronet/android/cronet_library_loader.cc:101:
base::i18n::InitializeICU();
On 2015/03/09 19:04:36, pauljensen wrote:
> On 2015/03/09 18:13:21, xunjieli wrote:
> > Should we move this to CronetOnLoad and maybe after url::Initialize() ? I am
> not
> > sure if this has to be on the main thread.
> 
> How about in another CL?  This seems orthogonal to the race condition goal
this
> CL is trying to address.

SGTM

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/ja...
File
components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java
(right):

https://codereview.chromium.org/991923002/diff/1/components/cronet/android/ja...
components/cronet/android/java/src/org/chromium/net/CronetLibraryLoader.java:71:
private static native void nativeCronetInitApplicationContext(Context
appContext);
On 2015/03/09 19:04:36, pauljensen wrote:
> On 2015/03/09 18:13:21, xunjieli wrote:
> > nit: I feel that there are too many "Cronet" in the method names. A little
> > redundant. How about just using nativeInitOnMainThread and
> > nativeInitApplicationContext ?
> 
> I didn't want to use InitApplicationContext as that might overlap with the
real
> base::android::InitApplicationContext.

But this two are in cronet namespace, so there won't be an overlap right?
Anyway, I am fine with not changing it.

[mef, 2015-03-09 20:11:57]

On 2015/03/09 18:05:39, mmenke wrote:
> On 2015/03/09 17:28:26, mef wrote:
> > lgtm
> 
> I'll defer to Misha.  Hrm...Do we really need 3 reviewers on all cronet CLs?

Still lgtm. 
I don't think we need 3 reviewers on every Cronet CL. 
All 4 of us are proud OWNERS anyway. :)

[pauljensen, 2015-03-09 20:17:45]

The CQ bit was checked by pauljensen@chromium.org

[commit-bot: I haz the power, 2015-03-09 20:18:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/991923002/20001

[commit-bot: I haz the power, 2015-03-09 20:48:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-09 20:50:04]

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