Fix Samsung's Lollipop devices DatePicker crash

Fix Samsung's Lollipop devices DatePicker crash
System default string resouce R.string.item_is_selected with expected
pattern %s is changed to %d on Samsung's 5.x  devices
(S4, S5, Note 3, Core Prime etc.). This small change causes
String.format() to throw exception when SimpleMonthView's content description
is requested (for example Accessibility/TalkBack is ON).
In case of Chromium this view is used only at DatePickerDialog.
The solution is to suppress this exception on 5.x devices and give
fixed string to view.

This fix is based on: http://stackoverflow.com/q/28618405
R=mlamouri@chromium.org, leandrogracia@chromium.org, tkent@chromium.org
BUG= crbug.com/710950

Review-Url: https://codereview.chromium.org/2790423002
Cr-Commit-Position: refs/heads/master@{#467945}
Committed: https://chromium.googlesource.com/chromium/src/+/5db6d1582b5525b719efa7fe0206a8b820edd0bc

[mlamouri (slow - plz ping), 2017-04-04 15:01:35]

Dropping myself from reviewer as I don't think I'm the right person to review
this.

[tkent, 2017-04-05 01:48:54]

tkent@chromium.org changed reviewers:
+ tedchoc@chromium.org
- mlamouri@chromium.org

[tkent, 2017-04-05 01:50:07]

+tedchoc.

I also don't understand this change, and can't review it.

[Ted C, 2017-04-05 16:54:30]

On 2017/04/05 01:50:07, tkent wrote:
> +tedchoc.
> 
> I also don't understand this change, and can't review it.

Can you please file a crbug with the exact crash stack for this?

We'd like to confirm this on our end and have a means to test this
before we introduce something like this.

[Ilya Tskhay, 2017-04-05 18:27:06]

On 2017/04/05 16:54:30, Ted C wrote:
> On 2017/04/05 01:50:07, tkent wrote:
> > +tedchoc.
> > 
> > I also don't understand this change, and can't review it.
> 
> Can you please file a crbug with the exact crash stack for this?
> 
> We'd like to confirm this on our end and have a means to test this
> before we introduce something like this.

Here is an issue https://bugs.chromium.org/p/chromium/issues/detail?id=708702

[Ilya Tskhay, 2017-04-05 18:27:27]

Description was changed from

==========
Fix Samsung's Lollipop devices DatePicker crash
System default string resouce R.string.item_is_selected with expected
pattern %s is changed to %d on Samsung's 5.x  devices
(S4, S5, Note 3, Core Prime etc.). This small change causes
String.format() to throw exception when SimpleMonthView's content description
is requested (for example Accessibility/TalkBack is ON).
In case of Chromium this view is used only at DatePickerDialog.
The solution is to suppress this exception on 5.x devices and give
fixed string to view.

This fix is based on: http://stackoverflow.com/q/28618405
R=mlamouri@chromium.org, leandrogracia@chromium.org, tkent@chromium.org
BUG=
==========

to

==========
Fix Samsung's Lollipop devices DatePicker crash
System default string resouce R.string.item_is_selected with expected
pattern %s is changed to %d on Samsung's 5.x  devices
(S4, S5, Note 3, Core Prime etc.). This small change causes
String.format() to throw exception when SimpleMonthView's content description
is requested (for example Accessibility/TalkBack is ON).
In case of Chromium this view is used only at DatePickerDialog.
The solution is to suppress this exception on 5.x devices and give
fixed string to view.

This fix is based on: http://stackoverflow.com/q/28618405
R=mlamouri@chromium.org, leandrogracia@chromium.org, tkent@chromium.org
BUG=
==========

[Ilya Tskhay, 2017-04-06 09:00:45]

Description was changed from

==========
Fix Samsung's Lollipop devices DatePicker crash
System default string resouce R.string.item_is_selected with expected
pattern %s is changed to %d on Samsung's 5.x  devices
(S4, S5, Note 3, Core Prime etc.). This small change causes
String.format() to throw exception when SimpleMonthView's content description
is requested (for example Accessibility/TalkBack is ON).
In case of Chromium this view is used only at DatePickerDialog.
The solution is to suppress this exception on 5.x devices and give
fixed string to view.

This fix is based on: http://stackoverflow.com/q/28618405
R=mlamouri@chromium.org, leandrogracia@chromium.org, tkent@chromium.org
BUG=
==========

to

==========
Fix Samsung's Lollipop devices DatePicker crash
System default string resouce R.string.item_is_selected with expected
pattern %s is changed to %d on Samsung's 5.x  devices
(S4, S5, Note 3, Core Prime etc.). This small change causes
String.format() to throw exception when SimpleMonthView's content description
is requested (for example Accessibility/TalkBack is ON).
In case of Chromium this view is used only at DatePickerDialog.
The solution is to suppress this exception on 5.x devices and give
fixed string to view.

This fix is based on: http://stackoverflow.com/q/28618405
R=mlamouri@chromium.org, leandrogracia@chromium.org, tkent@chromium.org
BUG=https://bugs.chromium.org/p/chromium/issues/detail?id=708702
==========

[Ilya Tskhay, 2017-04-06 09:01:01]

Description was changed from

==========
Fix Samsung's Lollipop devices DatePicker crash
System default string resouce R.string.item_is_selected with expected
pattern %s is changed to %d on Samsung's 5.x  devices
(S4, S5, Note 3, Core Prime etc.). This small change causes
String.format() to throw exception when SimpleMonthView's content description
is requested (for example Accessibility/TalkBack is ON).
In case of Chromium this view is used only at DatePickerDialog.
The solution is to suppress this exception on 5.x devices and give
fixed string to view.

This fix is based on: http://stackoverflow.com/q/28618405
R=mlamouri@chromium.org, leandrogracia@chromium.org, tkent@chromium.org
BUG=https://bugs.chromium.org/p/chromium/issues/detail?id=708702
==========

to

==========
Fix Samsung's Lollipop devices DatePicker crash
System default string resouce R.string.item_is_selected with expected
pattern %s is changed to %d on Samsung's 5.x  devices
(S4, S5, Note 3, Core Prime etc.). This small change causes
String.format() to throw exception when SimpleMonthView's content description
is requested (for example Accessibility/TalkBack is ON).
In case of Chromium this view is used only at DatePickerDialog.
The solution is to suppress this exception on 5.x devices and give
fixed string to view.

This fix is based on: http://stackoverflow.com/q/28618405
R=mlamouri@chromium.org, leandrogracia@chromium.org, tkent@chromium.org
BUG= https://bugs.chromium.org/p/chromium/issues/detail?id=708702
==========

[Ted C, 2017-04-12 17:24:39]

tedchoc@chromium.org changed reviewers:
+ boliu@chromium.org
- leandrogracia@chromium.org

[Ted C, 2017-04-12 17:24:40]

General naming/placement thoughts, but overall this seems OK (in a begrudging,
grumbly, sad we have to do this, but frighteningly neat, sort of way)

+boliu since I believe this will impact webview as well.

https://codereview.chromium.org/2790423002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/picker/FixedStringResourceContext.java
(right):

https://codereview.chromium.org/2790423002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/picker/FixedStringResourceContext.java:23:
* Workaround for Samsung Lolipop devices that may crash due to wrong string
resource supplied
s/Lolipop/Lollipop

https://codereview.chromium.org/2790423002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/picker/FixedStringResourceContext.java:26:
class FixedStringResourceContext extends ContextWrapper {
Instead of making this a new class, I'd put it inside of DateTimePickerDialog. 
We want to make sure this is never used outside of that class (even within this
package).

We should also be more explicit with the name here:

WorkaroundContextForSamsungLDateTimeBug or something.  We want to make sure no
one thinks this should be used for anything else.

https://codereview.chromium.org/2790423002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/picker/FixedStringResourceContext.java:71:
template = template.replaceAll(Pattern.quote("%" + conversion), "%s")
Honestly, I'd be tempted to just return the template with something like:

'"Invalid template: %s", template'

But I guess this is fine.

[boliu, 2017-04-12 17:47:06]

is this really still a problem? our crash reporting says we haven't seen
anything like this since m50. maybe samsung fixed their bug already

[boliu, 2017-04-12 18:14:00]

On 2017/04/12 17:47:06, boliu wrote:
> is this really still a problem? our crash reporting says we haven't seen
> anything like this since m50. maybe samsung fixed their bug already

Never mind, yeah it's still happening under a new signature. This is the most
frequent signature: crbug.com/710950. Please update your BUG to that instead

I haven't read this, but this looks super complicated and is going to be a huge
maintenance burden. Is it anything simpler we can do even if it's a worse user
experience, like maybe catch an exception somewhere and not pop up the dialog at
at all? It's still way better than crashing..

[Ilya Tskhay, 2017-04-13 06:27:23]

Description was changed from

==========
Fix Samsung's Lollipop devices DatePicker crash
System default string resouce R.string.item_is_selected with expected
pattern %s is changed to %d on Samsung's 5.x  devices
(S4, S5, Note 3, Core Prime etc.). This small change causes
String.format() to throw exception when SimpleMonthView's content description
is requested (for example Accessibility/TalkBack is ON).
In case of Chromium this view is used only at DatePickerDialog.
The solution is to suppress this exception on 5.x devices and give
fixed string to view.

This fix is based on: http://stackoverflow.com/q/28618405
R=mlamouri@chromium.org, leandrogracia@chromium.org, tkent@chromium.org
BUG= https://bugs.chromium.org/p/chromium/issues/detail?id=708702
==========

to

==========
Fix Samsung's Lollipop devices DatePicker crash
System default string resouce R.string.item_is_selected with expected
pattern %s is changed to %d on Samsung's 5.x  devices
(S4, S5, Note 3, Core Prime etc.). This small change causes
String.format() to throw exception when SimpleMonthView's content description
is requested (for example Accessibility/TalkBack is ON).
In case of Chromium this view is used only at DatePickerDialog.
The solution is to suppress this exception on 5.x devices and give
fixed string to view.

This fix is based on: http://stackoverflow.com/q/28618405
R=mlamouri@chromium.org, leandrogracia@chromium.org, tkent@chromium.org
BUG= crbug.com/710950
==========

[Ilya Tskhay, 2017-04-13 06:46:34]

On 2017/04/12 18:14:00, boliu wrote:
> On 2017/04/12 17:47:06, boliu wrote:
> > is this really still a problem? our crash reporting says we haven't seen
> > anything like this since m50. maybe samsung fixed their bug already
> 
> Never mind, yeah it's still happening under a new signature. This is the most
> frequent signature: crbug.com/710950. Please update your BUG to that instead
> 
> I haven't read this, but this looks super complicated and is going to be a
huge
> maintenance burden. Is it anything simpler we can do even if it's a worse user
> experience, like maybe catch an exception somewhere and not pop up the dialog
at
> at all? It's still way better than crashing..

To catch exception somewhere else we'll need to modify
android.widget.SimpleMonthView 
that is buried under android.widget.DatePicker which is located at chromium's
DateTimePickerDialog. I don't think that it would be the best way to solve 
this rare device/platform-specific problem.

[Ilya Tskhay, 2017-04-13 07:31:49]

The CQ bit was checked by i-ts@yandex-team.ru

[Ilya Tskhay, 2017-04-13 07:31:50]

The CQ bit was unchecked by i-ts@yandex-team.ru

[Ilya Tskhay, 2017-04-13 07:39:31]

FixedStringResourceContext renamed and moved inside DateTimePickerDialog

https://codereview.chromium.org/2790423002/diff/1/content/public/android/java...
File
content/public/android/java/src/org/chromium/content/browser/picker/FixedStringResourceContext.java
(right):

https://codereview.chromium.org/2790423002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/picker/FixedStringResourceContext.java:23:
* Workaround for Samsung Lolipop devices that may crash due to wrong string
resource supplied
On 2017/04/12 17:24:40, Ted C wrote:
> s/Lolipop/Lollipop

Done.

https://codereview.chromium.org/2790423002/diff/1/content/public/android/java...
content/public/android/java/src/org/chromium/content/browser/picker/FixedStringResourceContext.java:26:
class FixedStringResourceContext extends ContextWrapper {
On 2017/04/12 17:24:40, Ted C wrote:
> Instead of making this a new class, I'd put it inside of DateTimePickerDialog.

> We want to make sure this is never used outside of that class (even within
this
> package).
> 
> We should also be more explicit with the name here:
> 
> WorkaroundContextForSamsungLDateTimeBug or something.  We want to make sure no
> one thinks this should be used for anything else.

Done.

[Ted C, 2017-04-13 15:50:21]

lgtm

I do agree with Bo that this is more complicated than I'd like, but aside from
my suggestion of not trying to fix the strings in the failure case I don't have
any suggestion to do this with less code (and we've got something that is a
better user experience with this already so I don't feel strongly that we should
undo that.  If we have any problems with this though, I will be pretty quick to
dumb it down).

[Ilya Tskhay, 2017-04-28 09:08:51]

The CQ bit was checked by i-ts@yandex-team.ru

[commit-bot: I haz the power, 2017-04-28 09:09:13]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-28 09:54:26]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493370531783130,
"parent_rev": "4e5ec214cc5236df4b01515442722093956ac8f1", "commit_rev":
"5db6d1582b5525b719efa7fe0206a8b820edd0bc"}

[commit-bot: I haz the power, 2017-04-28 09:54:41]

Description was changed from

==========
Fix Samsung's Lollipop devices DatePicker crash
System default string resouce R.string.item_is_selected with expected
pattern %s is changed to %d on Samsung's 5.x  devices
(S4, S5, Note 3, Core Prime etc.). This small change causes
String.format() to throw exception when SimpleMonthView's content description
is requested (for example Accessibility/TalkBack is ON).
In case of Chromium this view is used only at DatePickerDialog.
The solution is to suppress this exception on 5.x devices and give
fixed string to view.

This fix is based on: http://stackoverflow.com/q/28618405
R=mlamouri@chromium.org, leandrogracia@chromium.org, tkent@chromium.org
BUG= crbug.com/710950
==========

to

==========
Fix Samsung's Lollipop devices DatePicker crash
System default string resouce R.string.item_is_selected with expected
pattern %s is changed to %d on Samsung's 5.x  devices
(S4, S5, Note 3, Core Prime etc.). This small change causes
String.format() to throw exception when SimpleMonthView's content description
is requested (for example Accessibility/TalkBack is ON).
In case of Chromium this view is used only at DatePickerDialog.
The solution is to suppress this exception on 5.x devices and give
fixed string to view.

This fix is based on: http://stackoverflow.com/q/28618405
R=mlamouri@chromium.org, leandrogracia@chromium.org, tkent@chromium.org
BUG= crbug.com/710950

Review-Url: https://codereview.chromium.org/2790423002
Cr-Commit-Position: refs/heads/master@{#467945}
Committed:
https://chromium.googlesource.com/chromium/src/+/5db6d1582b5525b719efa7fe0206...
==========

[commit-bot: I haz the power, 2017-04-28 09:54:42]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5db6d1582b5525b719efa7fe0206...