Fixes a deadlock between shouldInterceptRequest() and getCookie()

Fixes a deadlock between shouldInterceptRequest() and getCookie()

In the WebView shouldInterceptRequest is called on the IO thread.
The CookieManager used to post cookie monster tasks to a thread
which was sometimes the IO thread (if the WebView was started 
before the CookieManager) but sometimes a special thread (if the
CookieManager was started before the WebView).

getCookie is synchronous and blocks waiting for the result after
posting its task so if getCookie was called from
shouldInterceptRequest and the cookie monster was on the IO thread
then it deadlocked.

We fix this by always starting the special thread for the cookie
monster.

Android Issue: https://code.google.com/p/android/issues/detail?id=65786

BUG=374203
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271715

[Torne, 2014-05-16 12:18:15]

https://codereview.chromium.org/284313005/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/CookieManagerStartupTest.java
(right):

https://codereview.chromium.org/284313005/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/CookieManagerStartupTest.java:93:
public void testShouldInterceptRequestDeadlock() throws Throwable {
Is there a reason why this is here and not in the regular cookie manager test?
It looks like you had to refactor the test to add it here..

[hjd, 2014-05-16 15:03:24]

ptal

https://codereview.chromium.org/284313005/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/CookieManagerStartupTest.java
(right):

https://codereview.chromium.org/284313005/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/CookieManagerStartupTest.java:93:
public void testShouldInterceptRequestDeadlock() throws Throwable {
On 2014/05/16 12:18:15, Torne wrote:
> Is there a reason why this is here and not in the regular cookie manager test?
> It looks like you had to refactor the test to add it here..

I verified that each test hits the correct code path when you run them together
(apparently we do kill/create the process for every test because it has caused
problems in the past). It seems better to me to put it here than in
CookieManagerTest since it has to do with weird startup conditions, I've edited
the class comment to make more sense given I've added this test.

[hjd, 2014-05-20 12:52:40]

On 2014/05/16 15:03:24, hjd wrote:
> ptal
> 
>
https://codereview.chromium.org/284313005/diff/1/android_webview/javatests/sr...
> File
>
android_webview/javatests/src/org/chromium/android_webview/test/CookieManagerStartupTest.java
> (right):
> 
>
https://codereview.chromium.org/284313005/diff/1/android_webview/javatests/sr...
>
android_webview/javatests/src/org/chromium/android_webview/test/CookieManagerStartupTest.java:93:
> public void testShouldInterceptRequestDeadlock() throws Throwable {
> On 2014/05/16 12:18:15, Torne wrote:
> > Is there a reason why this is here and not in the regular cookie manager
test?
> > It looks like you had to refactor the test to add it here..
> 
> I verified that each test hits the correct code path when you run them
together
> (apparently we do kill/create the process for every test because it has caused
> problems in the past). It seems better to me to put it here than in
> CookieManagerTest since it has to do with weird startup conditions, I've
edited
> the class comment to make more sense given I've added this test.

I filed a Chromium bug and updated the commit message :)
Were there any other issues to address I can't remember if we discussed anything
else?

[Torne, 2014-05-20 14:22:27]

lgtm

[hjd, 2014-05-20 14:24:02]

The CQ bit was checked by hjd@chromium.org

[commit-bot: I haz the power, 2014-05-20 14:24:29]

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

[mkosiba (inactive), 2014-05-20 14:42:28]

lgtm

https://codereview.chromium.org/284313005/diff/20001/android_webview/javatest...
File
android_webview/javatests/src/org/chromium/android_webview/test/CookieManagerStartupTest.java
(right):

https://codereview.chromium.org/284313005/diff/20001/android_webview/javatest...
android_webview/javatests/src/org/chromium/android_webview/test/CookieManagerStartupTest.java:90:
// https://code.google.com/p/android/issues/detail?id=65786
now that you've filed the crbug you might as well link to it :)

[commit-bot: I haz the power, 2014-05-20 14:57:19]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/18...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[hjd, 2014-05-20 14:58:18]

The CQ bit was unchecked by hjd@chromium.org

[hjd, 2014-05-20 15:09:39]

The CQ bit was checked by hjd@chromium.org

[commit-bot: I haz the power, 2014-05-20 15:10:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hjd@chromium.org/284313005/60001

[commit-bot: I haz the power, 2014-05-20 18:22:16]

Change committed as 271715