Fix fading overlay events

Fix fading overlay events

Previously the FadingViewObserver had events triggered by setAlpha
to avoid null checks required by Android K. This turns out to have
issues with calling the "change" event too many times. As a result
the list of obscuring views in ChromeActivity gets the same view
added to it multiple times, breaking accessibility.

This change does two things. First, the event system for the fading
view is changed to work based on View's onVisibilityChanged event.
The observers are null checked to avoid issues on Android K. Second,
the list of obscuring views has been changed to be a set so that it
will not contain multiple references to the same view.

BUG=689203
Review-Url: https://codereview.chromium.org/2709883002
Cr-Commit-Position: refs/heads/master@{#452190}
Committed: https://chromium.googlesource.com/chromium/src/+/e18b88a5d18f93520fe9e97851e14582b0eb5e50

[mdjones, 2017-02-21 22:25:36]

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

[mdjones, 2017-02-21 22:25:37]

ptal

[gone, 2017-02-22 18:59:59]

lgtm

https://codereview.chromium.org/2709883002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/FadingBackgroundView.java
(right):

https://codereview.chromium.org/2709883002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/widget/FadingBackgroundView.java:104:
// This is added for the exclusive purpose of testing on Android K.
if that's the case, maybe you shouldn't early-return and instead wrap the for
loop in the conditional?

[mdjones, 2017-02-22 20:08:05]

The CQ bit was checked by mdjones@chromium.org

[mdjones, 2017-02-22 20:08:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/2709883002/#ps20001
(title: "rearrange null check")

[mdjones, 2017-02-22 20:08:11]

https://codereview.chromium.org/2709883002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/widget/FadingBackgroundView.java
(right):

https://codereview.chromium.org/2709883002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/widget/FadingBackgroundView.java:104:
// This is added for the exclusive purpose of testing on Android K.
On 2017/02/22 18:59:59, dfalcantara (load balance plz) wrote:
> if that's the case, maybe you shouldn't early-return and instead wrap the for
> loop in the conditional?

Done.

[commit-bot: I haz the power, 2017-02-22 20:09:01]

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-02-22 20:48:53]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487794085705680,
"parent_rev": "977c1dd577174ab7e8b9fd7686a4fa84011511d5", "commit_rev":
"e18b88a5d18f93520fe9e97851e14582b0eb5e50"}

[commit-bot: I haz the power, 2017-02-22 20:49:57]

Description was changed from

==========
Fix fading overlay events

Previously the FadingViewObserver had events triggered by setAlpha
to avoid null checks required by Android K. This turns out to have
issues with calling the "change" event too many times. As a result
the list of obscuring views in ChromeActivity gets the same view
added to it multiple times, breaking accessibility.

This change does two things. First, the event system for the fading
view is changed to work based on View's onVisibilityChanged event.
The observers are null checked to avoid issues on Android K. Second,
the list of obscuring views has been changed to be a set so that it
will not contain multiple references to the same view.

BUG=689203
==========

to

==========
Fix fading overlay events

Previously the FadingViewObserver had events triggered by setAlpha
to avoid null checks required by Android K. This turns out to have
issues with calling the "change" event too many times. As a result
the list of obscuring views in ChromeActivity gets the same view
added to it multiple times, breaking accessibility.

This change does two things. First, the event system for the fading
view is changed to work based on View's onVisibilityChanged event.
The observers are null checked to avoid issues on Android K. Second,
the list of obscuring views has been changed to be a set so that it
will not contain multiple references to the same view.

BUG=689203

Review-Url: https://codereview.chromium.org/2709883002
Cr-Commit-Position: refs/heads/master@{#452190}
Committed:
https://chromium.googlesource.com/chromium/src/+/e18b88a5d18f93520fe9e97851e1...
==========

[commit-bot: I haz the power, 2017-02-22 20:49:58]

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