Make Chrome on Android progress bar accessible.

Make Chrome on Android progress bar accessible.

BUG=608512
Committed: https://crrev.com/6156d89a02285efbd053bb7922d2432295a6872d
Cr-Commit-Position: refs/heads/master@{#394928}

[dmazzoni, 2016-05-02 22:01:46]

Description was changed from

==========
Make Chrome on Android progress bar accessible.

BUG=
==========

to

==========
Make Chrome on Android progress bar accessible.

BUG=608512
==========

[dmazzoni, 2016-05-02 22:01:59]

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

[Ted C, 2016-05-02 23:06:19]

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

[Ted C, 2016-05-02 23:06:20]

+mdjones

Should we throttle this to some minimum amount of progress between updates?  We
are going to start animating this very soon and this could get really spammy. 
Maybe we should require jumps of at least 5 percent?  Should we throttle on fast
pages so we don't tell progress any more often than some threshold?

[mdjones, 2016-05-03 01:45:57]

On 2016/05/02 23:06:20, Ted C (OOO 5.3 - 5.9) wrote:
> +mdjones
> 
> Should we throttle this to some minimum amount of progress between updates? 
We
> are going to start animating this very soon and this could get really spammy. 
> Maybe we should require jumps of at least 5 percent?  Should we throttle on
fast
> pages so we don't tell progress any more often than some threshold?

That's pretty close to what I was thinking. We could have a combination of
milestones and minimum time between updates. For example, update the user every
5 seconds unless the progress jumps 10%+. We also probably want to update the
user when load stops and finishes.

Alternatively, this code could be moved into ToolBarProgressBar to entirely
avoid the animation.

[dmazzoni, 2016-05-03 05:15:46]

On 2016/05/03 01:45:57, mdjones wrote:
> On 2016/05/02 23:06:20, Ted C (OOO 5.3 - 5.9) wrote:
> > +mdjones
> > 
> > Should we throttle this to some minimum amount of progress between updates? 
> We
> > are going to start animating this very soon and this could get really
spammy. 
> > Maybe we should require jumps of at least 5 percent?  Should we throttle on
> fast
> > pages so we don't tell progress any more often than some threshold?
> 
> That's pretty close to what I was thinking. We could have a combination of
> milestones and minimum time between updates. For example, update the user
every
> 5 seconds unless the progress jumps 10%+. We also probably want to update the
> user when load stops and finishes.
> 
> Alternatively, this code could be moved into ToolBarProgressBar to entirely
> avoid the animation.

Isn't ToolBarProgressBar a subclass? How would that avoid the animation?

It looks like ToolBarProgressBar already limits it to only once every 50 ms
(PROGRESS_FRAME_TIME_CAP_MS).

Note that an update should NOT mean that it's necessarily announced. It's
configured to play a rising sound now. TalkBack and other services should have
their own throttling in place, so while we should clearly do some limited
throttling, I don't think it's our job to come up with the perfect user-facing
heuristics.

I'm not totally clear on the different ProgressBar classes, though. Where would
be the best place to put it logically?

[mdjones, 2016-05-03 15:39:24]

On 2016/05/03 05:15:46, dmazzoni wrote:
> On 2016/05/03 01:45:57, mdjones wrote:
> > On 2016/05/02 23:06:20, Ted C (OOO 5.3 - 5.9) wrote:
> > > +mdjones
> > > 
> > > Should we throttle this to some minimum amount of progress between
updates? 
> > We
> > > are going to start animating this very soon and this could get really
> spammy. 
> > > Maybe we should require jumps of at least 5 percent?  Should we throttle
on
> > fast
> > > pages so we don't tell progress any more often than some threshold?
> > 
> > That's pretty close to what I was thinking. We could have a combination of
> > milestones and minimum time between updates. For example, update the user
> every
> > 5 seconds unless the progress jumps 10%+. We also probably want to update
the
> > user when load stops and finishes.
> > 
> > Alternatively, this code could be moved into ToolBarProgressBar to entirely
> > avoid the animation.
> 
> Isn't ToolBarProgressBar a subclass? How would that avoid the animation?
> 
> It looks like ToolBarProgressBar already limits it to only once every 50 ms
> (PROGRESS_FRAME_TIME_CAP_MS).
> 
> Note that an update should NOT mean that it's necessarily announced. It's
> configured to play a rising sound now. TalkBack and other services should have
> their own throttling in place, so while we should clearly do some limited
> throttling, I don't think it's our job to come up with the perfect user-facing
> heuristics.
> 
> I'm not totally clear on the different ProgressBar classes, though. Where
would
> be the best place to put it logically?


The current default behavior of the progress bar is to jump to the new position
when updated. We are in the process of releasing a change that makes the
progress bar animate smoothly. The mechanism for this is the child class
(ToolbarProgressBar) sending many more, fine grained progress updates to the
base class (ClipDrawableProgressBar). So, if you only want the big updates to
progress, the place to put this code would be in the child class.

[dmazzoni, 2016-05-19 18:23:18]

Moved to ToolbarProgressBar.

[mdjones, 2016-05-19 22:20:16]

lgtm

[Ted C, 2016-05-19 22:40:58]

owners - rs lgtm

[dmazzoni, 2016-05-19 22:51:09]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2016-05-19 22:52:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1916583002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1916583002/40001

[commit-bot: I haz the power, 2016-05-20 00:34:47]

Description was changed from

==========
Make Chrome on Android progress bar accessible.

BUG=608512
==========

to

==========
Make Chrome on Android progress bar accessible.

BUG=608512
==========

[commit-bot: I haz the power, 2016-05-20 00:34:48]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-20 00:36:06]

Description was changed from

==========
Make Chrome on Android progress bar accessible.

BUG=608512
==========

to

==========
Make Chrome on Android progress bar accessible.

BUG=608512

Committed: https://crrev.com/6156d89a02285efbd053bb7922d2432295a6872d
Cr-Commit-Position: refs/heads/master@{#394928}
==========

[commit-bot: I haz the power, 2016-05-20 00:36:07]

Patchset 3 (id:??) landed as
https://crrev.com/6156d89a02285efbd053bb7922d2432295a6872d
Cr-Commit-Position: refs/heads/master@{#394928}