Remove incognito parameter from AwContents

Remove incognito parameter from AwContents

It is always false and not planned to be supported

BUG=


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

[boliu, 2013-01-05 04:54:04]

joth@ We are not supporting incognito in v1 but not sure about future plans? I
don't think it hurts to do this now.

mnaganov@ A lot of tests, mostly AwSettingsTest, explicitly test incognito mode,
and we never decided what to do with those when I dropped incognito support. I
vote for removing those tests. Currently they don't get us anything. Removing
them is also going to speed up our tests on the bots so we are less likely to
get booted off cq if there are capacity problems.

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

https://codereview.chromium.org/11785011/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:267:
public AwContents(ViewGroup containerView,
I assume we are keeping the old version because of keeping downstream compiling?
In that case, the way I would do this is:

* completely implement the new version of the constructor without using the old
one
* remove the old version locally and make sure all of upstream still
compiles/runs (potentially a lot of work)
* add the old version back and have it call the new version (and maybe even mark
it Deprecated if it doesn't break compile)

That way the fixed up would simply be removing the old constructor after
downstream switched to the new one.

[Kristian Monsen, 2013-01-05 05:25:47]

Updated constructors. Will run the bots just to check it works upstream.

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

https://codereview.chromium.org/11785011/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:267:
public AwContents(ViewGroup containerView,
On 2013/01/05 04:54:04, boliu wrote:
> I assume we are keeping the old version because of keeping downstream
compiling?
> In that case, the way I would do this is:
> 
> * completely implement the new version of the constructor without using the
old
> one
> * remove the old version locally and make sure all of upstream still
> compiles/runs (potentially a lot of work)
> * add the old version back and have it call the new version (and maybe even
mark
> it Deprecated if it doesn't break compile)
> 
> That way the fixed up would simply be removing the old constructor after
> downstream switched to the new one.

This constructor is never called upstream. Everything compiles and works fine
there. Have not run all the tests, will let the buildbots do that before
commiting :-)

I will swap the constructors around so it is clearer and the next patch will be
smaller.

[boliu, 2013-01-05 05:31:42]

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

https://codereview.chromium.org/11785011/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:267:
public AwContents(ViewGroup containerView,
On 2013/01/05 05:25:47, Kristian Monsen wrote:
> This constructor is never called upstream.

But it will be called after the old one is removed. I'm just saying do that work
now so we know all the implications of this refactor before commiting

[Kristian Monsen, 2013-01-05 08:15:25]

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

https://codereview.chromium.org/11785011/diff/1/android_webview/java/src/org/...
android_webview/java/src/org/chromium/android_webview/AwContents.java:267:
public AwContents(ViewGroup containerView,
On 2013/01/05 05:31:42, boliu wrote:
> On 2013/01/05 05:25:47, Kristian Monsen wrote:
> > This constructor is never called upstream.
> 
> But it will be called after the old one is removed. I'm just saying do that
work
> now so we know all the implications of this refactor before commiting

Compiled everything with old constructor commented out. Will run the tests
during the weekend.

[boliu, 2013-01-05 09:11:13]

https://codereview.chromium.org/11785011/diff/2010/android_webview/javatests/...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidWebViewTestBase.java
(right):

https://codereview.chromium.org/11785011/diff/2010/android_webview/javatests/...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidWebViewTestBase.java:229:
final boolean incognito,
Will be more work to remove this

[mnaganov (inactive), 2013-01-07 09:44:08]

On 2013/01/05 04:54:04, boliu wrote:
> joth@ We are not supporting incognito in v1 but not sure about future plans? I
> don't think it hurts to do this now.
> 
> mnaganov@ A lot of tests, mostly AwSettingsTest, explicitly test incognito
mode,
> and we never decided what to do with those when I dropped incognito support. I
> vote for removing those tests. Currently they don't get us anything. Removing
> them is also going to speed up our tests on the bots so we are less likely to
> get booted off cq if there are capacity problems.
>

Yes, that's more like a tradition since our early attempts of WebView
implementation. Removing (and restoring back) these tests is a purely mechanical
operation, so I think it's OK to remove them now.
 
>
https://codereview.chromium.org/11785011/diff/1/android_webview/java/src/org/...
> File android_webview/java/src/org/chromium/android_webview/AwContents.java
> (right):
> 
>
https://codereview.chromium.org/11785011/diff/1/android_webview/java/src/org/...
> android_webview/java/src/org/chromium/android_webview/AwContents.java:267:
> public AwContents(ViewGroup containerView,
> I assume we are keeping the old version because of keeping downstream
compiling?
> In that case, the way I would do this is:
> 
> * completely implement the new version of the constructor without using the
old
> one
> * remove the old version locally and make sure all of upstream still
> compiles/runs (potentially a lot of work)
> * add the old version back and have it call the new version (and maybe even
mark
> it Deprecated if it doesn't break compile)
> 
> That way the fixed up would simply be removing the old constructor after
> downstream switched to the new one.

[Kristian Monsen, 2013-01-07 17:35:00]

https://codereview.chromium.org/11785011/diff/2010/android_webview/javatests/...
File
android_webview/javatests/src/org/chromium/android_webview/test/AndroidWebViewTestBase.java
(right):

https://codereview.chromium.org/11785011/diff/2010/android_webview/javatests/...
android_webview/javatests/src/org/chromium/android_webview/test/AndroidWebViewTestBase.java:229:
final boolean incognito,
On 2013/01/05 09:11:14, boliu wrote:
> Will be more work to remove this

It was a bit of work, also removed what was a bunch of duplicate test functions.

[boliu, 2013-01-07 18:00:15]

lgtm but wait for Mikhail

wow that's a lot of tests removed

https://codereview.chromium.org/11785011/diff/1009/android_webview/javatests/...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java
(right):

https://codereview.chromium.org/11785011/diff/1009/android_webview/javatests/...
android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java:1084:
public void testJavaScriptEnabledNormal() throws Throwable {
We should also remove the "Normal" suffix in all the remaining tests here, or
change it to something like "WithTwoViews". Defer to Mikhail for advice.

[Kristian Monsen, 2013-01-07 19:37:49]

Joth: As this is a large change, I'd like to know if you think this is worth
doing to simplify the code or if you want to keep it there due to future plans.

https://codereview.chromium.org/11785011/diff/1009/android_webview/javatests/...
File
android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java
(right):

https://codereview.chromium.org/11785011/diff/1009/android_webview/javatests/...
android_webview/javatests/src/org/chromium/android_webview/test/AwSettingsTest.java:1084:
public void testJavaScriptEnabledNormal() throws Throwable {
On 2013/01/07 18:00:15, boliu wrote:
> We should also remove the "Normal" suffix in all the remaining tests here, or
> change it to something like "WithTwoViews". Defer to Mikhail for advice.

What do you think Mikhail? Any preferred naming? I implemented WithTwoViews as
that can easy be search and replaced later :-)

[joth, 2013-01-07 19:46:27]

lgtm

[mnaganov (inactive), 2013-01-08 09:45:17]

lgtm

[commit-bot: I haz the power, 2013-01-09 05:27:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kristianm@chromium.org/11785011/17007

[commit-bot: I haz the power, 2013-01-09 08:05:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kristianm@chromium.org/11785011/24003

[Kristian Monsen, 2013-01-09 08:07:02]

The last patch set fixes a crash in ArchiveTest that was there since getActivity
was called for the first time on the UI thread. If any off you think it should
be solved in a different way I'm happy to fix in a follow up CL.

[commit-bot: I haz the power, 2013-01-09 08:52:28]

Retried try job too often on android_dbg for step(s) build

[commit-bot: I haz the power, 2013-01-09 19:02:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kristianm@chromium.org/11785011/29003

[commit-bot: I haz the power, 2013-01-09 21:15:16]

Change committed as 175887