[Android] Add ScrollView to DateTimePicker

[Android] Add ScrollView to DateTimePicker

On small screen devices, such as one with 800x480 resolution,
time picker does not render correctly, and sometimes causes a crash.

This adds a layer of ScrollView so that time picker can be accessed
and rendered correctly.

BUG=407543
Committed: https://crrev.com/43435213d9a2431bed1be8aec18dd8b7d452eadd
Cr-Commit-Position: refs/heads/master@{#297514}

[Changwan Ryu, 2014-09-26 07:41:20]

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

[Ted C, 2014-09-26 16:26:51]

https://codereview.chromium.org/604243002/diff/1/content/public/android/java/...
File content/public/android/java/res/layout-land/date_time_picker_dialog.xml
(left):

https://codereview.chromium.org/604243002/diff/1/content/public/android/java/...
content/public/android/java/res/layout-land/date_time_picker_dialog.xml:12:
android:orientation="horizontal"
This used to be horizontal and you changed it to vertical.  Was that
intentional?  (if it wasn't you might want to try a HorizontalScrollView)

Also, do you have any idea why we can't have the layout files in the same
location?

[Changwan Ryu, 2014-09-30 00:22:11]

https://codereview.chromium.org/604243002/diff/1/content/public/android/java/...
File content/public/android/java/res/layout-land/date_time_picker_dialog.xml
(left):

https://codereview.chromium.org/604243002/diff/1/content/public/android/java/...
content/public/android/java/res/layout-land/date_time_picker_dialog.xml:12:
android:orientation="horizontal"
On 2014/09/26 16:26:51, Ted C (OOO till 10.1) wrote:
> This used to be horizontal and you changed it to vertical.  Was that
> intentional?  (if it wasn't you might want to try a HorizontalScrollView)
> 
> Also, do you have any idea why we can't have the layout files in the same
> location?

My bad... Fixed now. Also changed gravity as well, and moved it under ui/. It's
part of UI component resources, so I think it makes more sense to move it there.

By the way, I realized that landscape/portrait is determined at the dialog
launch time, and assigned resources cannot change on orientation change once
it's shown. I've filed a separate bug (Bug 418835) so that it can be fixed with
a separate release schedule.

[Changwan Ryu, 2014-09-30 00:29:56]

Roping in more code reviewers as the main reviewer is OOO and we're targeting
M38. (Please check b/17293049 for more).

[Changwan Ryu, 2014-09-30 00:30:40]

changwan@chromium.org changed reviewers:
+ benm@chromium.org, miguelg@chromium.org, newt@chromium.org,
yfriedmanc@chromium.org

[newt (away), 2014-09-30 00:38:42]

This seems fine, though it's not at all clear how adding this ScrollView could
fix a crash. Do you have a crash stack or an idea of how the lack of ScrollView
could cause a crash?

(Typically, the Android view system is very forgiving if a view overflows its
parent. The view will simply be clipped. So I'm skeptical that this could cause
a crash, though a crash stack could convince me otherwise.)

[Changwan Ryu, 2014-09-30 00:58:47]

On 2014/09/30 00:38:42, newt wrote:
> This seems fine, though it's not at all clear how adding this ScrollView could
> fix a crash. Do you have a crash stack or an idea of how the lack of
ScrollView
> could cause a crash?
> 
> (Typically, the Android view system is very forgiving if a view overflows its
> parent. The view will simply be clipped. So I'm skeptical that this could
cause
> a crash, though a crash stack could convince me otherwise.)

Please check my bug update. crsym does not give meaningful stack trace and the
problem is not reproducible with this patch, so I suspect that it is an Android
issue. Also, as you can see in the attached image on the crbug the time picker
is not accessible, so this fixes usability issue as well.

[newt (away), 2014-09-30 01:11:33]

lgtm, though I think it's worth further investigating the cause of the crash.

[newt (away), 2014-09-30 01:12:09]

On 2014/09/30 01:11:33, newt wrote:
> lgtm, though I think it's worth further investigating the cause of the crash.

Did you try using src/third_party/android_platform/development/scripts/stack to
symbolize the crash stack? (I'm not familiar with crsym)

[Changwan Ryu, 2014-09-30 01:25:02]

On 2014/09/30 01:12:09, newt wrote:
> On 2014/09/30 01:11:33, newt wrote:
> > lgtm, though I think it's worth further investigating the cause of the
crash.
> 
> Did you try using src/third_party/android_platform/development/scripts/stack
to
> symbolize the crash stack? (I'm not familiar with crsym)

I got the following result:

signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
     r0 57599040  r1 00000a6b  r2 41633d40  r3 588d30b0
     r4 58612a2a  r5 575912d0  r6 415479d8  r7 00002070
     r8 4156a6c0  r9 41ef2ee8  sl 00000002  fp 00000000
     ip 00000070  sp becd74f8  lr 415b9cb1  pc 4156c2d4

Stack Trace:
  RELADDR   FUNCTION                                                            
                        FILE:LINE
  000222d4  <unknown>                                                           
                        /system/lib/libdvm.so
  00030f48  dvmMterpStd(Thread*)+76                                             
                        /system/lib/libdvm.so
  0002e5e0  dvmInterpret(Thread*, Method const*, JValue*)+184                   
                        /system/lib/libdvm.so
  00063af9  dvmInvokeMethod(Object*, Method const*, ArrayObject*, ArrayObject*,
ClassObject*, bool)+392  /system/lib/libdvm.so
  0006ba4b  <unknown>                                                           
                        /system/lib/libdvm.so
  000299e0  <unknown>                                                           
                        /system/lib/libdvm.so
  00030f48  dvmMterpStd(Thread*)+76                                             
                        /system/lib/libdvm.so
  0002e5e0  dvmInterpret(Thread*, Method const*, JValue*)+184                   
                        /system/lib/libdvm.so
  00063815  dvmCallMethodV(Thread*, Method const*, Object*, bool, JValue*,
std::__va_list)+336           /system/lib/libdvm.so
  0004cf17  <unknown>                                                           
                        /system/lib/libdvm.so
  0004e13f  <unknown>                                                           
                        /system/lib/libandroid_runtime.so
  0004ee97  android::AndroidRuntime::start(char const*, char const*)+354        
                        /system/lib/libandroid_runtime.so
  0000109b  <unknown>                                                           
                        /system/bin/app_process
  0000e5ab  __libc_init+50                                                      
                        /system/lib/libc.so
  00000db0  <unknown>                                                           
                        /system/bin/app_process

[Changwan Ryu, 2014-09-30 01:28:49]

On 2014/09/30 01:25:02, Changwan Ryu wrote:
> On 2014/09/30 01:12:09, newt wrote:
> > On 2014/09/30 01:11:33, newt wrote:
> > > lgtm, though I think it's worth further investigating the cause of the
> crash.
> > 
> > Did you try using src/third_party/android_platform/development/scripts/stack
> to
> > symbolize the crash stack? (I'm not familiar with crsym)
> 
> I got the following result:
> 
> signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
>      r0 57599040  r1 00000a6b  r2 41633d40  r3 588d30b0
>      r4 58612a2a  r5 575912d0  r6 415479d8  r7 00002070
>      r8 4156a6c0  r9 41ef2ee8  sl 00000002  fp 00000000
>      ip 00000070  sp becd74f8  lr 415b9cb1  pc 4156c2d4
> 
> Stack Trace:
>   RELADDR   FUNCTION                                                          
 
>                         FILE:LINE
>   000222d4  <unknown>                                                         
 
>                         /system/lib/libdvm.so
>   00030f48  dvmMterpStd(Thread*)+76                                           
 
>                         /system/lib/libdvm.so
>   0002e5e0  dvmInterpret(Thread*, Method const*, JValue*)+184                 
 
>                         /system/lib/libdvm.so
>   00063af9  dvmInvokeMethod(Object*, Method const*, ArrayObject*,
ArrayObject*,
> ClassObject*, bool)+392  /system/lib/libdvm.so
>   0006ba4b  <unknown>                                                         
 
>                         /system/lib/libdvm.so
>   000299e0  <unknown>                                                         
 
>                         /system/lib/libdvm.so
>   00030f48  dvmMterpStd(Thread*)+76                                           
 
>                         /system/lib/libdvm.so
>   0002e5e0  dvmInterpret(Thread*, Method const*, JValue*)+184                 
 
>                         /system/lib/libdvm.so
>   00063815  dvmCallMethodV(Thread*, Method const*, Object*, bool, JValue*,
> std::__va_list)+336           /system/lib/libdvm.so
>   0004cf17  <unknown>                                                         
 
>                         /system/lib/libdvm.so
>   0004e13f  <unknown>                                                         
 
>                         /system/lib/libandroid_runtime.so
>   0004ee97  android::AndroidRuntime::start(char const*, char const*)+354      
 
>                         /system/lib/libandroid_runtime.so
>   0000109b  <unknown>                                                         
 
>                         /system/bin/app_process
>   0000e5ab  __libc_init+50                                                    
 
>                         /system/lib/libc.so
>   00000db0  <unknown>                                                         
 
>                         /system/bin/app_process

I still need approval for content/public/.

yfriedman@ / benm@, could you take a look?

[Changwan Ryu, 2014-09-30 02:07:16]

changwan@chromium.org changed reviewers:
+ yfriedman@chromium.org
- yfriedmanc@chromium.org

[Yaron, 2014-09-30 17:29:24]

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

https://codereview.chromium.org/604243002/diff/20001/ui/android/java/res/layo...
ui/android/java/res/layout-land/date_time_picker_dialog.xml:9:
<HorizontalScrollView xmlns:android="http://schemas.android.com/apk/res/android"
There's already a ./ui/android/java/res/layout/date_time_picker_dialog.xml which
seems to match the content/ version. Should that be updated as well?

[Changwan Ryu, 2014-09-30 19:26:16]

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

https://codereview.chromium.org/604243002/diff/20001/ui/android/java/res/layo...
ui/android/java/res/layout-land/date_time_picker_dialog.xml:9:
<HorizontalScrollView xmlns:android="http://schemas.android.com/apk/res/android"
On 2014/09/30 17:29:23, Yaron wrote:
> There's already a ./ui/android/java/res/layout/date_time_picker_dialog.xml
which
> seems to match the content/ version. Should that be updated as well?

It's included in this cl. :) content resource is being updated and moved to ui.

[Yaron, 2014-09-30 20:11:19]

lgtm

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

https://codereview.chromium.org/604243002/diff/20001/ui/android/java/res/layo...
ui/android/java/res/layout-land/date_time_picker_dialog.xml:9:
<HorizontalScrollView xmlns:android="http://schemas.android.com/apk/res/android"
On 2014/09/30 19:26:16, Changwan Ryu wrote:
> On 2014/09/30 17:29:23, Yaron wrote:
> > There's already a ./ui/android/java/res/layout/date_time_picker_dialog.xml
> which
> > seems to match the content/ version. Should that be updated as well?
> 
> It's included in this cl. :) content resource is being updated and moved to
ui.

:S

[Changwan Ryu, 2014-09-30 20:47:28]

The CQ bit was checked by changwan@chromium.org

[commit-bot: I haz the power, 2014-09-30 20:48:58]

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

[commit-bot: I haz the power, 2014-09-30 21:32:32]

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

[commit-bot: I haz the power, 2014-09-30 21:32:33]

Try jobs failed on following builders:
  linux_gpu_triggered_tests on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu_trigger...)

[Changwan Ryu, 2014-09-30 21:53:31]

The CQ bit was checked by changwan@chromium.org

[commit-bot: I haz the power, 2014-09-30 21:55:08]

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

[commit-bot: I haz the power, 2014-09-30 21:57:03]

Committed patchset #2 (id:20001) as b407058ef6253603112f7d4ee822b51acfef5480

[commit-bot: I haz the power, 2014-09-30 21:58:25]

Patchset 2 (id:??) landed as
https://crrev.com/43435213d9a2431bed1be8aec18dd8b7d452eadd
Cr-Commit-Position: refs/heads/master@{#297514}