Add WebView implementation for CookieManager.

Add WebView implementation for CookieManager.

Additional details of the requirements can be found from the Android SDK.
http://developer.android.com/reference/android/webkit/CookieManager.html

BUG=
TEST=make forwarder android_webview_apk android_webview_test_apk


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=155331

[Ted C, 2012-09-04 20:49:39]

@joth - android_webview
@jam - changes to base/threading/thread_restrictions.h (see
scoped_allow_wait_for_legacy_web_view_api.h for further descriptions of its
need).

[Ted C, 2012-09-04 20:50:15]

Blast...silly publish button having focus!

@yfriedman for content/public/android owners

[Yaron, 2012-09-04 21:10:26]

https://chromiumcodereview.appspot.com/10913074/diff/1/content/public/android...
File
content/public/android/java/src/org/chromium/content/third_party/ParseException.java
(right):

https://chromiumcodereview.appspot.com/10913074/diff/1/content/public/android...
content/public/android/java/src/org/chromium/content/third_party/ParseException.java:1:
/*
Since your'e adding a new third_party repo, should there be a licence file?
http://dev.chromium.org/developers/adding-3rd-party-libraries

[jam, 2012-09-05 01:38:22]

lgtm with nit

https://codereview.chromium.org/10913074/diff/1/android_webview/browser/scope...
File android_webview/browser/scoped_allow_wait_for_legacy_web_view_api.h
(right):

https://codereview.chromium.org/10913074/diff/1/android_webview/browser/scope...
android_webview/browser/scoped_allow_wait_for_legacy_web_view_api.h:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
nit: it doesn't seem necessary to add a header just for this. just put this
class in cookie_manager.cc

[Yaron, 2012-09-05 06:10:43]

looked at base/android and content/public/android

http://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/org...
File base/android/javatests/src/org/chromium/base/test/TestHttpServerClient.java
(right):

http://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/org...
base/android/javatests/src/org/chromium/base/test/TestHttpServerClient.java:35:
checkServerIsUp(getUrl("chrome/test/data/android/ok.txt"), "OK Computer");
You need to add this file.

http://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/org...
base/android/javatests/src/org/chromium/base/test/TestHttpServerClient.java:35:
checkServerIsUp(getUrl("chrome/test/data/android/ok.txt"), "OK Computer");
Use UrlUtils.getTestFileUrl

http://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/org...
File base/android/javatests/src/org/chromium/base/test/UrlUtils.java (right):

http://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/org...
base/android/javatests/src/org/chromium/base/test/UrlUtils.java:16: * @param
path Pathname relative to external/chrome/testing/data
Path could be <chrome_src>/testing/data. Or actually, should it be
.../test/data?

http://codereview.chromium.org/10913074/diff/1/content/public/android/java/sr...
File
content/public/android/java/src/org/chromium/content/third_party/ParseException.java
(right):

http://codereview.chromium.org/10913074/diff/1/content/public/android/java/sr...
content/public/android/java/src/org/chromium/content/third_party/ParseException.java:18:
* NOTE: This class is taken from the AOSP
frameworks/base/core/java/android/net/ParseException.java.
I think you can drop this  comment with the license change.

http://codereview.chromium.org/10913074/diff/1/content/public/android/java/sr...
File
content/public/android/java/src/org/chromium/content/third_party/WebAddress.java
(right):

http://codereview.chromium.org/10913074/diff/1/content/public/android/java/sr...
content/public/android/java/src/org/chromium/content/third_party/WebAddress.java:18:
* NOTE: This class is taken from the AOSP
frameworks/base/core/java/android/net/WebAddress.java.
Remove here too.

http://codereview.chromium.org/10913074/diff/1/content/public/android/javates...
File
content/public/android/javatests/src/org/chromium/content/browser/Criteria.java
(right):

http://codereview.chromium.org/10913074/diff/1/content/public/android/javates...
content/public/android/javatests/src/org/chromium/content/browser/Criteria.java:7:
public interface Criteria {
Can you concoct a minimal, non-lame comment?

http://codereview.chromium.org/10913074/diff/1/content/public/android/javates...
File
content/public/android/javatests/src/org/chromium/content/browser/CriteriaHelper.java
(right):

http://codereview.chromium.org/10913074/diff/1/content/public/android/javates...
content/public/android/javatests/src/org/chromium/content/browser/CriteriaHelper.java:22:
/**
indentation looks off

http://codereview.chromium.org/10913074/diff/1/content/public/android/javates...
content/public/android/javatests/src/org/chromium/content/browser/CriteriaHelper.java:72:
public static class NotZoomingCriteria implements Criteria {
How re-used is this? Seems like it doesn't really belong here (especially given
the content view abuse below)

[Ted C, 2012-09-05 18:45:35]

PTAL

https://codereview.chromium.org/10913074/diff/1/android_webview/browser/scope...
File android_webview/browser/scoped_allow_wait_for_legacy_web_view_api.h
(right):

https://codereview.chromium.org/10913074/diff/1/android_webview/browser/scope...
android_webview/browser/scoped_allow_wait_for_legacy_web_view_api.h:1: //
Copyright (c) 2012 The Chromium Authors. All rights reserved.
On 2012/09/05 01:38:22, John Abd-El-Malek wrote:
> nit: it doesn't seem necessary to add a header just for this. just put this
> class in cookie_manager.cc

Done.

https://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/or...
File base/android/javatests/src/org/chromium/base/test/TestHttpServerClient.java
(right):

https://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/or...
base/android/javatests/src/org/chromium/base/test/TestHttpServerClient.java:35:
checkServerIsUp(getUrl("chrome/test/data/android/ok.txt"), "OK Computer");
On 2012/09/05 06:10:43, Yaron wrote:
> You need to add this file.

Done.

https://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/or...
base/android/javatests/src/org/chromium/base/test/TestHttpServerClient.java:35:
checkServerIsUp(getUrl("chrome/test/data/android/ok.txt"), "OK Computer");
On 2012/09/05 06:10:43, Yaron wrote:
> Use UrlUtils.getTestFileUrl

UrlUtils appears to me to be doing something different.  This is creating a URL
that hits http://localhost:8000/chrome/test/data/android/ok.txt

While UrlUtils would be:
file:///data/local/tmp/chrome/test/data/android/ok.txt

So that wouldn't test that the localhost server is actually up and running.

https://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/or...
File base/android/javatests/src/org/chromium/base/test/UrlUtils.java (right):

https://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/or...
base/android/javatests/src/org/chromium/base/test/UrlUtils.java:16: * @param
path Pathname relative to external/chrome/testing/data
On 2012/09/05 06:10:43, Yaron wrote:
> Path could be <chrome_src>/testing/data. Or actually, should it be
> .../test/data?

Done.  I prefer the <chrome_src> as that is easier to parse for me.

https://codereview.chromium.org/10913074/diff/1/content/public/android/java/s...
File
content/public/android/java/src/org/chromium/content/third_party/ParseException.java
(right):

https://codereview.chromium.org/10913074/diff/1/content/public/android/java/s...
content/public/android/java/src/org/chromium/content/third_party/ParseException.java:1:
/*
On 2012/09/04 21:10:26, Yaron wrote:
> Since your'e adding a new third_party repo, should there be a licence file?
> http://dev.chromium.org/developers/adding-3rd-party-libraries

Talked with joth and we shouldn't be using these at all in cookie manager.  We
should rely on GURL's parsing like the rest of chrome.

https://codereview.chromium.org/10913074/diff/1/content/public/android/javate...
File
content/public/android/javatests/src/org/chromium/content/browser/Criteria.java
(right):

https://codereview.chromium.org/10913074/diff/1/content/public/android/javate...
content/public/android/javatests/src/org/chromium/content/browser/Criteria.java:7:
public interface Criteria {
On 2012/09/05 06:10:43, Yaron wrote:
> Can you concoct a minimal, non-lame comment?

Done.

https://codereview.chromium.org/10913074/diff/1/content/public/android/javate...
File
content/public/android/javatests/src/org/chromium/content/browser/CriteriaHelper.java
(right):

https://codereview.chromium.org/10913074/diff/1/content/public/android/javate...
content/public/android/javatests/src/org/chromium/content/browser/CriteriaHelper.java:22:
/**
On 2012/09/05 06:10:43, Yaron wrote:
> indentation looks off

Done.

https://codereview.chromium.org/10913074/diff/1/content/public/android/javate...
content/public/android/javatests/src/org/chromium/content/browser/CriteriaHelper.java:72:
public static class NotZoomingCriteria implements Criteria {
On 2012/09/05 06:10:43, Yaron wrote:
> How re-used is this? Seems like it doesn't really belong here (especially
given
> the content view abuse below)

Removed as it isn't used yet.

[Yaron, 2012-09-05 18:55:07]

lgtm for base/android and content/public/android

https://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/or...
File base/android/javatests/src/org/chromium/base/test/UrlUtils.java (right):

https://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/or...
base/android/javatests/src/org/chromium/base/test/UrlUtils.java:16: * @param
path Pathname relative to external/chrome/testing/data
On 2012/09/05 18:45:35, Ted C wrote:
> On 2012/09/05 06:10:43, Yaron wrote:
> > Path could be <chrome_src>/testing/data. Or actually, should it be
> > .../test/data?
> 
> Done.  I prefer the <chrome_src> as that is easier to parse for me.

Ok. I think the path should be <chrome_src>/test/data though.

[Ted C, 2012-09-05 19:02:31]

On 2012/09/05 18:55:07, Yaron wrote:
> lgtm for base/android and content/public/android
> 
>
https://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/or...
> File base/android/javatests/src/org/chromium/base/test/UrlUtils.java (right):
> 
>
https://codereview.chromium.org/10913074/diff/1/base/android/javatests/src/or...
> base/android/javatests/src/org/chromium/base/test/UrlUtils.java:16: * @param
> path Pathname relative to external/chrome/testing/data
> On 2012/09/05 18:45:35, Ted C wrote:
> > On 2012/09/05 06:10:43, Yaron wrote:
> > > Path could be <chrome_src>/testing/data. Or actually, should it be
> > > .../test/data?
> > 
> > Done.  I prefer the <chrome_src> as that is easier to parse for me.
> 
> Ok. I think the path should be <chrome_src>/test/data though.

Ah gotcha, actually it needs to be:
<chrome_src>/chrome/test/data/android/device_files/

[joth, 2012-09-05 20:13:05]

+mnaganov, as FYI - you have overlapping patches in the UrlUtils area.

https://codereview.chromium.org/10913074/diff/11003/base/android/javatests/sr...
File base/android/javatests/src/org/chromium/base/test/TestHttpServerClient.java
(right):

https://codereview.chromium.org/10913074/diff/11003/base/android/javatests/sr...
base/android/javatests/src/org/chromium/base/test/TestHttpServerClient.java:14:
* A utility class for interacting with HTTP server containing test files.
this should be in net/javatests, not base.

https://codereview.chromium.org/10913074/diff/11003/base/android/javatests/sr...
File base/android/javatests/src/org/chromium/base/test/UrlUtils.java (right):

https://codereview.chromium.org/10913074/diff/11003/base/android/javatests/sr...
base/android/javatests/src/org/chromium/base/test/UrlUtils.java:16: * @param
path Pathname relative to <chrome_src>/chrome/test/data/android/device_files/
See https://chromiumcodereview.appspot.com/10911097/#msg7 -- base/ shouldn't be
referring to paths in chrome/....

https://codereview.chromium.org/10913074/diff/11003/base/android/javatests/sr...
base/android/javatests/src/org/chromium/base/test/UrlUtils.java:23: * Construct
a suitable URL for loading a test data file from the hosts's
nit: remove one s, and make less ambiguous: the local host's

[Ted C, 2012-09-05 23:04:19]

On 2012/09/05 20:13:05, joth wrote:
> +mnaganov, as FYI - you have overlapping patches in the UrlUtils area.
> 
>
https://codereview.chromium.org/10913074/diff/11003/base/android/javatests/sr...
> File
base/android/javatests/src/org/chromium/base/test/TestHttpServerClient.java
> (right):
> 
>
https://codereview.chromium.org/10913074/diff/11003/base/android/javatests/sr...
>
base/android/javatests/src/org/chromium/base/test/TestHttpServerClient.java:14:
> * A utility class for interacting with HTTP server containing test files.
> this should be in net/javatests, not base.
> 
>
https://codereview.chromium.org/10913074/diff/11003/base/android/javatests/sr...
> File base/android/javatests/src/org/chromium/base/test/UrlUtils.java (right):
> 
>
https://codereview.chromium.org/10913074/diff/11003/base/android/javatests/sr...
> base/android/javatests/src/org/chromium/base/test/UrlUtils.java:16: * @param
> path Pathname relative to <chrome_src>/chrome/test/data/android/device_files/
> See https://chromiumcodereview.appspot.com/10911097/#msg7 -- base/ shouldn't
be
> referring to paths in chrome/....
> 
>
https://codereview.chromium.org/10913074/diff/11003/base/android/javatests/sr...
> base/android/javatests/src/org/chromium/base/test/UrlUtils.java:23: *
Construct
> a suitable URL for loading a test data file from the hosts's
> nit: remove one s, and make less ambiguous: the local host's

Removed these files as they were only added to get AwSettingsTest to compile. 
Commented out the broken lines there instead.

[joth, 2012-09-05 23:21:15]

patch set 4: lgtm

skimmed most the files as I already reviewed downstream.

[commit-bot: I haz the power, 2012-09-05 23:22:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedchoc@chromium.org/10913074/4003

[commit-bot: I haz the power, 2012-09-06 01:59:47]

Failed to apply patch for
android_webview/javatests/src/org/chromium/android_webview/test/AndroidWebViewTestBase.java:
While running patch -p1 --forward --force;
patching file
android_webview/javatests/src/org/chromium/android_webview/test/AndroidWebViewTestBase.java
Hunk #1 FAILED at 158.
1 out of 1 hunk FAILED -- saving rejects to file
android_webview/javatests/src/org/chromium/android_webview/test/AndroidWebViewTestBase.java.rej

[commit-bot: I haz the power, 2012-09-06 16:34:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedchoc@chromium.org/10913074/12004

[commit-bot: I haz the power, 2012-09-06 19:13:19]

Try job failure for 10913074-12004 (retry) (previous was lost) (retry) on
mac_rel for step "sync_integration_tests" (clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-09-06 20:06:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedchoc@chromium.org/10913074/12004

[commit-bot: I haz the power, 2012-09-06 20:06:45]

Failed to apply patch for
android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java:
While running patch -p1 --forward --force;
patching file
android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java
Hunk #1 FAILED at 9.
Hunk #2 FAILED at 388.
2 out of 2 hunks FAILED -- saving rejects to file
android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java.rej

[commit-bot: I haz the power, 2012-09-06 20:13:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedchoc@chromium.org/10913074/4032

[commit-bot: I haz the power, 2012-09-06 23:49:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tedchoc@chromium.org/10913074/4033

[commit-bot: I haz the power, 2012-09-07 05:43:13]

Change committed as 155331