[Android] On calling openNewTab, pass the disposition instead of incognito.

[Android] On calling openNewTab, pass the disposition instead of incognito.

Currently, when openNewTab is called, we pass a boolean incognito variable that
indicates if it should be opened in incognito mode. This was not consistent with
the desktop. For example, NEW_FOREGROUND_TAB and NEW_BACKGROUND_TAB open incognito
tab if the current tab is incognito.

So this CL instead passes the actual disposition so that Java side has the full
information required to be consistent with the desktop.

BUG=268283
BUG=268414
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215973

[Kibeom Kim (inactive), 2013-08-06 01:09:45]

Bo told me about .template, I'll work on that.

[Kibeom Kim (inactive), 2013-08-06 03:20:41]

Changed to .template

[joth, 2013-08-06 04:04:21]

mostly looks good.

https://codereview.chromium.org/22314005/diff/14001/components/web_contents_d...
File
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java
(right):

https://codereview.chromium.org/22314005/diff/14001/components/web_contents_d...
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java:51:
public void openNewTab(String url, String extraHeaders, byte[] postData, int
disposition) {
could you add a comment saying which class to use to get the constants

https://codereview.chromium.org/22314005/diff/14001/components/web_contents_d...
File components/web_contents_delegate_android/web_contents_delegate_android.cc
(left):

https://codereview.chromium.org/22314005/diff/14001/components/web_contents_d...
components/web_contents_delegate_android/web_contents_delegate_android.cc:99:
params.browser_initiated_post_data.get()->size());
could you put these .get() back? rsleevi was purposely adding them not long ago
as part of a transition in implicit bool operator.
(unless that transition is done now? I didn't see any PSA)

https://codereview.chromium.org/22314005/diff/14001/ui/ui.gyp
File ui/ui.gyp (right):

https://codereview.chromium.org/22314005/diff/14001/ui/ui.gyp#newcode974
ui/ui.gyp:974: 'target_name': 'window_open_disposition_java',
you'll need to add this to some other files to keep android_aosp happy (I added
the try job for you)

see https://chromiumcodereview.appspot.com/17084002 for an example fix after
https://chromiumcodereview.appspot.com/15907012 landed.

[Kibeom Kim (inactive), 2013-08-06 04:36:01]

https://codereview.chromium.org/22314005/diff/14001/components/web_contents_d...
File
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java
(right):

https://codereview.chromium.org/22314005/diff/14001/components/web_contents_d...
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java:51:
public void openNewTab(String url, String extraHeaders, byte[] postData, int
disposition) {
On 2013/08/06 04:04:21, joth wrote:
> could you add a comment saying which class to use to get the constants

Done.

https://codereview.chromium.org/22314005/diff/14001/components/web_contents_d...
File components/web_contents_delegate_android/web_contents_delegate_android.cc
(left):

https://codereview.chromium.org/22314005/diff/14001/components/web_contents_d...
components/web_contents_delegate_android/web_contents_delegate_android.cc:99:
params.browser_initiated_post_data.get()->size());
On 2013/08/06 04:04:21, joth wrote:
> could you put these .get() back? rsleevi was purposely adding them not long
ago
> as part of a transition in implicit bool operator.
> (unless that transition is done now? I didn't see any PSA)

Done.

https://codereview.chromium.org/22314005/diff/14001/ui/ui.gyp
File ui/ui.gyp (right):

https://codereview.chromium.org/22314005/diff/14001/ui/ui.gyp#newcode974
ui/ui.gyp:974: 'target_name': 'window_open_disposition_java',
On 2013/08/06 04:04:21, joth wrote:
> you'll need to add this to some other files to keep android_aosp happy (I
added
> the try job for you)
> 
> see https://chromiumcodereview.appspot.com/17084002 for an example fix after
> https://chromiumcodereview.appspot.com/15907012 landed.

Done.

[joth, 2013-08-06 04:47:25]

lgtm

https://codereview.chromium.org/22314005/diff/19001/components/web_contents_d...
File
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java
(right):

https://codereview.chromium.org/22314005/diff/19001/components/web_contents_d...
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java:54:
*                    ui/android/java/WindowOpenDisposition.template .
nit: you can word wrap at 100 but I think the long path names are hampering you.

maybe sufficient to say 

The new tab disposition as per the constants in
org.chromium.ui.WindowOpenDisposition (See window_open_disposition_list.h for
the enumeration definitions).

[Kibeom Kim (inactive), 2013-08-06 05:09:54]

sky, ben -- could you take a look at ui/* files? (trivial changes) Thanks.

https://codereview.chromium.org/22314005/diff/19001/components/web_contents_d...
File
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java
(right):

https://codereview.chromium.org/22314005/diff/19001/components/web_contents_d...
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java:54:
*                    ui/android/java/WindowOpenDisposition.template .
On 2013/08/06 04:47:25, joth wrote:
> nit: you can word wrap at 100 but I think the long path names are hampering
you.
> 
> maybe sufficient to say 
> 
> The new tab disposition as per the constants in
> org.chromium.ui.WindowOpenDisposition (See window_open_disposition_list.h for
> the enumeration definitions).

Done.

https://codereview.chromium.org/22314005/diff/19001/components/web_contents_d...
components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java:54:
*                    ui/android/java/WindowOpenDisposition.template .
On 2013/08/06 04:47:25, joth wrote:
> nit: you can word wrap at 100 but I think the long path names are hampering
you.
> 
> maybe sufficient to say 
> 
> The new tab disposition as per the constants in
> org.chromium.ui.WindowOpenDisposition (See window_open_disposition_list.h for
> the enumeration definitions).

Done.

[sky, 2013-08-06 14:20:08]

ui LGTM

https://codereview.chromium.org/22314005/diff/26001/ui/base/window_open_dispo...
File ui/base/window_open_disposition.h (right):

https://codereview.chromium.org/22314005/diff/26001/ui/base/window_open_dispo...
ui/base/window_open_disposition.h:1: // Copyright 2013 The Chromium Authors. All
rights reserved.
You don't need to update copyright year.

[commit-bot: I haz the power, 2013-08-06 17:34:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kkimlabs@chromium.org/22314005/43001

[commit-bot: I haz the power, 2013-08-06 19:26:48]

Retried try job too often on mac_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-08-06 19:29:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kkimlabs@chromium.org/22314005/43001

[commit-bot: I haz the power, 2013-08-06 21:01:54]

Change committed as 215973