Move *Picker.java and *PickerDialog.java to ui/android/java/... - Part1

Move *Picker.java and *PickerDialog.java to ui/android/java/...

There is no reason for the following files to live in content/ layer
and they should all be moved to ui/ layer.

BUG=302114
Committed: https://crrev.com/a91f4578e0de51430919d2982b906e59dcb227d3
Cr-Commit-Position: refs/heads/master@{#296637}

[ankit, 2014-09-16 16:42:28]

ankit2.kumar@samsung.com changed reviewers:
+ aurimas@chromium.org, miguelg@chromium.org

[ankit, 2014-09-16 16:42:29]

@aurmias, @miguel
PTAL

[aurimas (slooooooooow), 2014-09-16 16:52:06]

https://chromiumcodereview.appspot.com/574673003/diff/20001/content/public/an...
File
content/public/android/java/src/org/chromium/content/browser/input/InputDialogContainer.java
(right):

https://chromiumcodereview.appspot.com/574673003/diff/20001/content/public/an...
content/public/android/java/src/org/chromium/content/browser/input/InputDialogContainer.java:42:
public class InputDialogContainer {
What is the reason not to move this file along with the others?

https://chromiumcodereview.appspot.com/574673003/diff/20001/ui/android/java/r...
File ui/android/java/res/layout/date_time_picker_dialog.xml (right):

https://chromiumcodereview.appspot.com/574673003/diff/20001/ui/android/java/r...
ui/android/java/res/layout/date_time_picker_dialog.xml:2: <!-- Copyright (c)
2012 The Chromium Authors. All rights reserved.
Please update the copyright header to remove (c).

[ankit, 2014-09-17 14:57:57]

@aurimas
PTAL new patch

https://codereview.chromium.org/574673003/diff/20001/content/public/android/j...
File
content/public/android/java/src/org/chromium/content/browser/input/InputDialogContainer.java
(right):

https://codereview.chromium.org/574673003/diff/20001/content/public/android/j...
content/public/android/java/src/org/chromium/content/browser/input/InputDialogContainer.java:42:
public class InputDialogContainer {
On 2014/09/16 16:52:06, aurimas wrote:
> What is the reason not to move this file along with the others?

Moved this file to ui/layer

https://codereview.chromium.org/574673003/diff/20001/ui/android/java/res/layo...
File ui/android/java/res/layout/date_time_picker_dialog.xml (right):

https://codereview.chromium.org/574673003/diff/20001/ui/android/java/res/layo...
ui/android/java/res/layout/date_time_picker_dialog.xml:2: <!-- Copyright (c)
2012 The Chromium Authors. All rights reserved.
On 2014/09/16 16:52:06, aurimas wrote:
> Please update the copyright header to remove (c).

Done.

[aurimas (slooooooooow), 2014-09-17 15:58:38]

We are getting closer.

All these classes that you are moving are very closely related, instead of
putting them under org.chromium.ui package, let put it under
org.chromium.ui.picker so it is all nice and isolated.

https://codereview.chromium.org/574673003/diff/100001/content/public/android/...
File
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java
(right):

https://codereview.chromium.org/574673003/diff/100001/content/public/android/...
content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java:25:
import org.chromium.ui.InputDialogContainer;
Insert new line after this.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/InputDialogContainer.java (right):

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:209: protected
void showPickerDialog(final int dialogType,
What is the reason for all the visibility (public/protected) changes in this and
other files? Can we also move the code that is requiring all these changes?

[ankit, 2014-09-18 13:05:20]

@Ted
PTAL new patch.
I have inlined reason for visibility changes. 
As per my understanding ImeAdapter should not be moved. But files like 
InputDialogContainerTest.java and DateTimeChooserAndroid.java can be moved.
I am planning to move all test files(like InputDialogContainerTest,
AutofillPopupTest) 
along with DateTimeChooserAndroid.java and create a javatests folder 
under ui/android layer in a different CL.
Please share your opinion.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/InputDialogContainer.java (right):

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:35: public
interface InputActionDelegate {
DateTimeChooserAndroid.java is implementing this interface.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:55: public static
void initializeInputTypes(int textInputTypeDate,
DateTimeChooserAndroid.java is using this method.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:67: public static
boolean isDialogInputType(int type) {
ImeAdapter is using this method.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:73: public
InputDialogContainer(Context context, InputActionDelegate inputActionDelegate) {
InputDialogContainerForTests is extending this class.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:78: public void
showPickerDialog(final int dialogType, double dialogValue,
Overridden by InputDialogContainerTest.java

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:196: public void
showDialog(final int type, final double value,
DateTimeChooserAndroid.java is using this method.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:209: protected
void showPickerDialog(final int dialogType,
On 2014/09/17 15:58:38, aurimas wrote:
> What is the reason for all the visibility (public/protected) changes in this
and
> other files? Can we also move the code that is requiring all these changes?

This funtions is overridden by InputDialogCotainerTest.java.

[ankit, 2014-09-18 13:07:38]

@aurimas
PTAL new patch.
I have inlined reason for visibility changes. 
As per my understanding ImeAdapter should not be moved. But files like 
InputDialogContainerTest.java and DateTimeChooserAndroid.java can be moved.
I am planning to move all test files(like InputDialogContainerTest,
AutofillPopupTest) along with DateTimeChooserAndroid.java and create a 
javatests folder under ui/android layer in a different CL.
Please share your opinion.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
File ui/android/java/src/org/chromium/ui/InputDialogContainer.java (right):

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:35: public
interface InputActionDelegate {
DateTimeChooserAndroid.java is implementing this interface.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:55: public static
void initializeInputTypes(int textInputTypeDate,
DateTimeChooserAndroid.java is using this method.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:67: public static
boolean isDialogInputType(int type) {
ImeAdapter is using this method.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:73: public
InputDialogContainer(Context context, InputActionDelegate inputActionDelegate) {
InputDialogContainerForTests is extending this class.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:78: public void
showPickerDialog(final int dialogType, double dialogValue,
Overridden by InputDialogContainerTest.java

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:196: public void
showDialog(final int type, final double value,
DateTimeChooserAndroid.java is using this method.

https://codereview.chromium.org/574673003/diff/100001/ui/android/java/src/org...
ui/android/java/src/org/chromium/ui/InputDialogContainer.java:209: protected
void showPickerDialog(final int dialogType,
On 2014/09/17 15:58:38, aurimas wrote:
> What is the reason for all the visibility (public/protected) changes in this
and
> other files? Can we also move the code that is requiring all these changes?

This funtions is overridden by InputDialogCotainerTest.java.

[aurimas (slooooooooow), 2014-09-18 17:01:27]

On 2014/09/18 13:07:38, ankit wrote:
> @aurimas
> PTAL new patch.
> I have inlined reason for visibility changes. 
> As per my understanding ImeAdapter should not be moved.
That is correct. That bug was only intended for moving Picker related classes.
Text input as well as GamePad classes should stay where they are now.

> But files like 
> InputDialogContainerTest.java and DateTimeChooserAndroid.java can be moved.
> I am planning to move all test files(like InputDialogContainerTest,
> AutofillPopupTest) along with DateTimeChooserAndroid.java and create a 
> javatests folder under ui/android layer in a different CL.
> Please share your opinion.
That sounds good as long as we remove unnecessary public/protect modifiers on
the follow up patch.

[aurimas (slooooooooow), 2014-09-18 17:02:25]

lgtm

[aurimas (slooooooooow), 2014-09-18 17:52:15]

On 2014/09/18 17:02:25, aurimas wrote:
> lgtm

Please add a ui/android/java/src/org/chromium/ui/picker/OWNERS file and add
miguelg@chromium.org and aurimas@chromium.org to it.

[aurimas (slooooooooow), 2014-09-18 17:53:00]

aurimas@chromium.org changed reviewers:
+ tedchoc@chromium.org

[aurimas (slooooooooow), 2014-09-18 17:53:01]

+tedchoc@chromium.org for owners review.

[Ted C, 2014-09-18 18:05:47]

lgtm

[ankit, 2014-09-19 05:35:31]

On 2014/09/18 18:05:47, Ted C wrote:
> lgtm

Thanks.
I have added OWNERS file.

[ankit, 2014-09-19 05:35:39]

The CQ bit was checked by ankit2.kumar@samsung.com

[commit-bot: I haz the power, 2014-09-19 05:36:01]

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

[commit-bot: I haz the power, 2014-09-19 05:46:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-19 05:46:11]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ankit, 2014-09-19 05:49:32]

ankit2.kumar@samsung.com changed reviewers:
+ benm@chromium.org

[ankit, 2014-09-19 05:49:33]

@benm
PTAL.
Need owner approval for R.java

[ankit, 2014-09-22 10:30:07]

On 2014/09/19 05:49:33, ankit wrote:
> @benm
> PTAL.
> Need owner approval for R.java

@benm
PTAL

[benm (inactive), 2014-09-22 15:17:45]

lgtm

lgtm

sorry for the delay

[ankit, 2014-09-23 03:43:55]

On 2014/09/22 15:17:45, benm wrote:
> lgtm
> 
> lgtm
> 
> sorry for the delay

No problem.
Thanks.

[ankit, 2014-09-23 03:44:01]

The CQ bit was checked by ankit2.kumar@samsung.com

[commit-bot: I haz the power, 2014-09-23 03:44:57]

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

[commit-bot: I haz the power, 2014-09-23 03:55:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-23 03:55:31]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[ankit, 2014-09-23 13:02:26]

The CQ bit was checked by ankit2.kumar@samsung.com

[commit-bot: I haz the power, 2014-09-23 13:03:13]

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

[commit-bot: I haz the power, 2014-09-23 13:26:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-23 13:26:21]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[ankit, 2014-09-23 14:55:11]

On 2014/09/23 13:26:21, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   android_chromium_gn_compile_rel on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

@aurimas
Build is failing at |android_chromium_gn_compile_rel| and
|android_chromium_gn_compile_dbg| with below error(compilation is not
successful).
ImeAdapter.java:25: error: package org.chromium.ui.picker does not exist
import org.chromium.ui.picker.InputDialogContainer;

But for some changes it's passing for other android build bots(compilation is
successful).
Please suggest if I have missed out some changes which is causing this build
break.

[aurimas (slooooooooow), 2014-09-24 17:13:33]

aurimas@chromium.org changed reviewers:
+ cjhopman@chromium.org

[aurimas (slooooooooow), 2014-09-24 17:13:34]

We have two build systems that we have to support. It seems like you need to set
up GN to work correctly.

+cjhopman@ who can give you some guidance

[cjhopman, 2014-09-24 17:30:56]

On 2014/09/24 17:13:34, aurimas wrote:
> We have two build systems that we have to support. It seems like you need to
set
> up GN to work correctly.
> 
> +cjhopman@ who can give you some guidance

If you try this again, I think it should work. Before
https://codereview.chromium.org/569923003/ we were specifying all of ui_java's
files, but now we just specify the directory like in gyp.

[ankit, 2014-09-25 04:08:34]

The CQ bit was checked by ankit2.kumar@samsung.com

[commit-bot: I haz the power, 2014-09-25 04:09:27]

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

[commit-bot: I haz the power, 2014-09-25 04:20:56]

Committed patchset #10 (id:180001) as 620e946a2dac1ed735a2bf502f450b7134e9ff0c

[commit-bot: I haz the power, 2014-09-25 04:21:38]

Patchset 10 (id:??) landed as
https://crrev.com/a91f4578e0de51430919d2982b906e59dcb227d3
Cr-Commit-Position: refs/heads/master@{#296637}

[benm (inactive), 2014-09-25 10:01:49]

On 2014/09/25 04:21:38, I haz the power (commit-bot) wrote:
> Patchset 10 (id:??) landed as
> https://crrev.com/a91f4578e0de51430919d2982b906e59dcb227d3
> Cr-Commit-Position: refs/heads/master@{#296637}

Hm, this seems to have broken the downstream webview build.


2014-09-25
09:04:03:external/chromium_org/android_webview/../ui/android/java/src/org/chromium/ui/picker/TwoFieldDatePicker.java:107:
error: cannot find symbol
2014-09-25 09:04:03:        mYearSpinner = (NumberPicker)
findViewById(R.id.year);
2014-09-25 09:04:03:                                                       ^
2014-09-25 09:04:03:  symbol:   variable year
2014-09-25 09:04:03:  location: class id

and several more similar errors; the id resources didn't get moved from
content/R.java into ui/R.java

I'm not sure why the AOSP bot didn't catch this error.

[ankit, 2014-09-25 10:24:32]

On 2014/09/25 10:01:49, benm wrote:
> On 2014/09/25 04:21:38, I haz the power (commit-bot) wrote:
> > Patchset 10 (id:??) landed as
> > https://crrev.com/a91f4578e0de51430919d2982b906e59dcb227d3
> > Cr-Commit-Position: refs/heads/master@{#296637}
> 
> Hm, this seems to have broken the downstream webview build.
> 
> 
> 2014-09-25
>
09:04:03:external/chromium_org/android_webview/../ui/android/java/src/org/chromium/ui/picker/TwoFieldDatePicker.java:107:
> error: cannot find symbol
> 2014-09-25 09:04:03:        mYearSpinner = (NumberPicker)
> findViewById(R.id.year);
> 2014-09-25 09:04:03:                                                       ^
> 2014-09-25 09:04:03:  symbol:   variable year
> 2014-09-25 09:04:03:  location: class id
> 
> and several more similar errors; the id resources didn't get moved from
> content/R.java into ui/R.java
> 
> I'm not sure why the AOSP bot didn't catch this error.

@benm
Please let me know how can I see all the error so that I can fix these.

[ankit, 2014-09-25 11:07:41]

On 2014/09/25 10:24:32, ankit wrote:
> On 2014/09/25 10:01:49, benm wrote:
> > On 2014/09/25 04:21:38, I haz the power (commit-bot) wrote:
> > > Patchset 10 (id:??) landed as
> > > https://crrev.com/a91f4578e0de51430919d2982b906e59dcb227d3
> > > Cr-Commit-Position: refs/heads/master@{#296637}
> > 
> > Hm, this seems to have broken the downstream webview build.
> > 
> > 
> > 2014-09-25
> >
>
09:04:03:external/chromium_org/android_webview/../ui/android/java/src/org/chromium/ui/picker/TwoFieldDatePicker.java:107:
> > error: cannot find symbol
> > 2014-09-25 09:04:03:        mYearSpinner = (NumberPicker)
> > findViewById(R.id.year);
> > 2014-09-25 09:04:03:                                                       ^
> > 2014-09-25 09:04:03:  symbol:   variable year
> > 2014-09-25 09:04:03:  location: class id
> > 
> > and several more similar errors; the id resources didn't get moved from
> > content/R.java into ui/R.java
> > 
> > I'm not sure why the AOSP bot didn't catch this error.
> 
> @benm
> Please let me know how can I see all the error so that I can fix these.

@benm
I tried to trigger |android_aosp| build with latest code. But build was
successful.
If I have missed out moving of some resources id. Please let me know the list of
resources id
so that I can move those.

[benm (inactive), 2014-09-25 11:17:11]

On 2014/09/25 11:07:41, ankit wrote:
> On 2014/09/25 10:24:32, ankit wrote:
> > On 2014/09/25 10:01:49, benm wrote:
> > > On 2014/09/25 04:21:38, I haz the power (commit-bot) wrote:
> > > > Patchset 10 (id:??) landed as
> > > > https://crrev.com/a91f4578e0de51430919d2982b906e59dcb227d3
> > > > Cr-Commit-Position: refs/heads/master@{#296637}
> > > 
> > > Hm, this seems to have broken the downstream webview build.
> > > 
> > > 
> > > 2014-09-25
> > >
> >
>
09:04:03:external/chromium_org/android_webview/../ui/android/java/src/org/chromium/ui/picker/TwoFieldDatePicker.java:107:
> > > error: cannot find symbol
> > > 2014-09-25 09:04:03:        mYearSpinner = (NumberPicker)
> > > findViewById(R.id.year);
> > > 2014-09-25 09:04:03:                                                      
^
> > > 2014-09-25 09:04:03:  symbol:   variable year
> > > 2014-09-25 09:04:03:  location: class id
> > > 
> > > and several more similar errors; the id resources didn't get moved from
> > > content/R.java into ui/R.java
> > > 
> > > I'm not sure why the AOSP bot didn't catch this error.
> > 
> > @benm
> > Please let me know how can I see all the error so that I can fix these.
> 
> @benm
> I tried to trigger |android_aosp| build with latest code. But build was
> successful.
> If I have missed out moving of some resources id. Please let me know the list
of
> resources id
> so that I can move those.

Thanks ankit, mkosiba@ is looking at it from our side. We think the right fix
may be just to remove these R.java files altogether.