Fix AccessibilityExtensionLoader's handling of locked screen

Fix AccessibilityExtensionLoader's handling of locked screen

Previously, accessibility extensions required content scripts to access locked screen UI. With the introduction of chrome.automation, this is no longer necessary. An extension running in the signed in profile can easily access locked screen UI.

We therefore can:
- remove enabling an extension in the locked screen ui's profile (e.g. adding ChromeVox as a component extension).
- stop injection of content scripts into the locked screen's web ui.

TEST=navigate to locked screen with ChromeVox on. Verify things work. Toggle ChromeVox off and on. Verify things still work.
BUG=676467, 611736
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2617633004
Cr-Commit-Position: refs/heads/master@{#442970}
Committed: https://chromium.googlesource.com/chromium/src/+/4702f8e80cde7762dc897963ab69080e38093b97

[David Tseng, 2017-01-05 23:28:32]

Description was changed from

==========
Fix AccessibilityExtensionLoader's handling of locked screen

Previously,

accessibility extensions required content scripts to access locked screen UI.
With the introduction of chrome.automation, this is no longer necessary. An
extension running in the signed in profile can easily access locked screen UI.

We therefore can:
- remove enabling an extension in the locked screen ui's profile (e.g. adding
ChromeVox as a component extension).
- stop injection of content scripts into the locked screen's web ui.

TEST=navigate to locked screen with ChromeVox on. Verify things work. Toggle
ChromeVox off and on. Verify things still work.
BUG=
==========

to

==========
Fix AccessibilityExtensionLoader's handling of locked screen

Previously,

accessibility extensions required content scripts to access locked screen UI.
With the introduction of chrome.automation, this is no longer necessary. An
extension running in the signed in profile can easily access locked screen UI.

We therefore can:
- remove enabling an extension in the locked screen ui's profile (e.g. adding
ChromeVox as a component extension).
- stop injection of content scripts into the locked screen's web ui.

TEST=navigate to locked screen with ChromeVox on. Verify things work. Toggle
ChromeVox off and on. Verify things still work.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2017-01-05 23:30:17]

Description was changed from

==========
Fix AccessibilityExtensionLoader's handling of locked screen

Previously,

accessibility extensions required content scripts to access locked screen UI.
With the introduction of chrome.automation, this is no longer necessary. An
extension running in the signed in profile can easily access locked screen UI.

We therefore can:
- remove enabling an extension in the locked screen ui's profile (e.g. adding
ChromeVox as a component extension).
- stop injection of content scripts into the locked screen's web ui.

TEST=navigate to locked screen with ChromeVox on. Verify things work. Toggle
ChromeVox off and on. Verify things still work.
BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix AccessibilityExtensionLoader's handling of locked screen

Previously,

accessibility extensions required content scripts to access locked screen UI.
With the introduction of chrome.automation, this is no longer necessary. An
extension running in the signed in profile can easily access locked screen UI.

We therefore can:
- remove enabling an extension in the locked screen ui's profile (e.g. adding
ChromeVox as a component extension).
- stop injection of content scripts into the locked screen's web ui.

TEST=navigate to locked screen with ChromeVox on. Verify things work. Toggle
ChromeVox off and on. Verify things still work.
BUG=676467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2017-01-05 23:30:17]

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

[David Tseng, 2017-01-06 15:47:00]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2017-01-06 15:47:08]

The CQ bit was unchecked by dtseng@chromium.org

[David Tseng, 2017-01-06 15:47:24]

The CQ bit was checked by dtseng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 15:47:35]

Dry run: 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-01-06 17:57:01]

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

[commit-bot: I haz the power, 2017-01-06 17:57:02]

Dry run: This issue passed the CQ dry run.

[dmazzoni, 2017-01-09 19:16:51]

Description was changed from

==========
Fix AccessibilityExtensionLoader's handling of locked screen

Previously,

accessibility extensions required content scripts to access locked screen UI.
With the introduction of chrome.automation, this is no longer necessary. An
extension running in the signed in profile can easily access locked screen UI.

We therefore can:
- remove enabling an extension in the locked screen ui's profile (e.g. adding
ChromeVox as a component extension).
- stop injection of content scripts into the locked screen's web ui.

TEST=navigate to locked screen with ChromeVox on. Verify things work. Toggle
ChromeVox off and on. Verify things still work.
BUG=676467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix AccessibilityExtensionLoader's handling of locked screen

Previously, accessibility extensions required content scripts to access locked
screen UI. With the introduction of chrome.automation, this is no longer
necessary. An extension running in the signed in profile can easily access
locked screen UI.

We therefore can:
- remove enabling an extension in the locked screen ui's profile (e.g. adding
ChromeVox as a component extension).
- stop injection of content scripts into the locked screen's web ui.

TEST=navigate to locked screen with ChromeVox on. Verify things work. Toggle
ChromeVox off and on. Verify things still work.
BUG=676467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dmazzoni, 2017-01-09 19:17:19]

This is fantastic! So happy to delete all of this code.
Just one possible concern, see below.

https://codereview.chromium.org/2617633004/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/accessibility/accessibility_extension_loader.cc
(left):

https://codereview.chromium.org/2617633004/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/accessibility/accessibility_extension_loader.cc:270: if
(!init_script_str_.empty()) {
init_script_str is unused, now. If it's really not needed, you
can remove the places where it's set. However, I wonder if we
might still need to set "window.INJECTED_AFTER_LOAD = true"
so that Classic can distinguish between the case where it's
enabled dynamically and when a page loads. In particular,
Classic ought to announce when a page loads, but switching
to Classic shouldn't announce the same way a page load does.

Perhaps we could solve this differently by having Classic
query the background page to determine whether it should
announce a page load or not.

[David Tseng, 2017-01-09 21:38:56]

dmazzoni@chromium.org writes:



>
>
>
>
https://codereview.chromium.org/2617633004/diff/20001/chrome/browser/chromeos...
> File
> chrome/browser/chromeos/accessibility/accessibility_extension_loader.cc
> (left):
>
>
https://codereview.chromium.org/2617633004/diff/20001/chrome/browser/chromeos...
> chrome/browser/chromeos/accessibility/accessibility_extension_loader.cc:270:
> if (!init_script_str_.empty()) {
> init_script_str is unused, now. If it's really not needed, you
> can remove the places where it's set.

Done.

 However, I wonder if we
> might still need to set "window.INJECTED_AFTER_LOAD = true"
> so that Classic can distinguish between the case where it's
> enabled dynamically and when a page loads. In particular,
> Classic ought to announce when a page loads, but switching
> to Classic shouldn't announce the same way a page load does.
>

The injection codepath uses 

cvox.ChromeVoxBackground.prototype.injectChromeVoxIntoTabs

which already sets INJECTED_AFTER_LOAD.
Is that the one you're looking at?

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[David Tseng, 2017-01-10 00:12:08]

dtseng@chromium.org changed reviewers:
+ fsamuel@chromium.org

[David Tseng, 2017-01-10 00:12:09]

+ fsamuel for c/b/guest_view

[dmazzoni, 2017-01-10 00:14:31]

lgtm

[David Tseng, 2017-01-10 23:41:36]

dtseng@chromium.org changed reviewers:
+ lfg@chromium.org

[David Tseng, 2017-01-10 23:41:36]

+ lfg for c/b/guest_view (just in case fsamuel is unavailable). Thanks.

[lfg, 2017-01-11 01:19:39]

On 2017/01/10 23:41:36, David Tseng wrote:
> + lfg for c/b/guest_view (just in case fsamuel is unavailable). Thanks.

lgtm

Can you also check if this test still matters?
https://cs.chromium.org/chromium/src/chrome/browser/apps/guest_view/web_view_...

[David Tseng, 2017-01-11 17:12:27]

Description was changed from

==========
Fix AccessibilityExtensionLoader's handling of locked screen

Previously, accessibility extensions required content scripts to access locked
screen UI. With the introduction of chrome.automation, this is no longer
necessary. An extension running in the signed in profile can easily access
locked screen UI.

We therefore can:
- remove enabling an extension in the locked screen ui's profile (e.g. adding
ChromeVox as a component extension).
- stop injection of content scripts into the locked screen's web ui.

TEST=navigate to locked screen with ChromeVox on. Verify things work. Toggle
ChromeVox off and on. Verify things still work.
BUG=676467
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix AccessibilityExtensionLoader's handling of locked screen

Previously, accessibility extensions required content scripts to access locked
screen UI. With the introduction of chrome.automation, this is no longer
necessary. An extension running in the signed in profile can easily access
locked screen UI.

We therefore can:
- remove enabling an extension in the locked screen ui's profile (e.g. adding
ChromeVox as a component extension).
- stop injection of content scripts into the locked screen's web ui.

TEST=navigate to locked screen with ChromeVox on. Verify things work. Toggle
ChromeVox off and on. Verify things still work.
BUG=676467,611736
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[David Tseng, 2017-01-11 17:20:46]

The CQ bit was checked by dtseng@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 17:21:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[David Tseng, 2017-01-11 17:28:38]

On 2017/01/11 01:19:39, lfg wrote:
> On 2017/01/10 23:41:36, David Tseng wrote:
> > + lfg for c/b/guest_view (just in case fsamuel is unavailable). Thanks.
> 
> lgtm
> 
> Can you also check if this test still matters?
>
https://cs.chromium.org/chromium/src/chrome/browser/apps/guest_view/web_view_...

Thanks, good suggestion.
I think the test can safely be removed as ChromeVox no longer uses content
scripts to access <webview>; ChromeVox tests should be at a higher level (i.e.
interactive ui tests) to help lessen flakes.

[lfg, 2017-01-11 18:15:55]

lfg@chromium.org changed reviewers:
+ wjmaclean@chromium.org

[lfg, 2017-01-11 18:15:56]

Thanks. +wjmaclean for chrome/ test removal.

[wjmaclean, 2017-01-11 18:22:28]

On 2017/01/11 18:15:56, lfg wrote:
> Thanks. +wjmaclean for chrome/ test removal.

lgtm

[commit-bot: I haz the power, 2017-01-11 18:36:18]

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

[commit-bot: I haz the power, 2017-01-11 18:36:19]

Dry run: This issue passed the CQ dry run.

[David Tseng, 2017-01-11 18:45:20]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2017-01-11 18:45:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org, lfg@chromium.org
Link to the patchset: https://codereview.chromium.org/2617633004/#ps60001
(title: "Test removals.")

[commit-bot: I haz the power, 2017-01-11 18:45:49]

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-01-11 18:54:18]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1484160320064100,
"parent_rev": "eed4ea8fb01d92a623f3d912d3ea904e7740e13f", "commit_rev":
"4702f8e80cde7762dc897963ab69080e38093b97"}

[commit-bot: I haz the power, 2017-01-11 18:54:45]

Description was changed from

==========
Fix AccessibilityExtensionLoader's handling of locked screen

Previously, accessibility extensions required content scripts to access locked
screen UI. With the introduction of chrome.automation, this is no longer
necessary. An extension running in the signed in profile can easily access
locked screen UI.

We therefore can:
- remove enabling an extension in the locked screen ui's profile (e.g. adding
ChromeVox as a component extension).
- stop injection of content scripts into the locked screen's web ui.

TEST=navigate to locked screen with ChromeVox on. Verify things work. Toggle
ChromeVox off and on. Verify things still work.
BUG=676467,611736
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix AccessibilityExtensionLoader's handling of locked screen

Previously, accessibility extensions required content scripts to access locked
screen UI. With the introduction of chrome.automation, this is no longer
necessary. An extension running in the signed in profile can easily access
locked screen UI.

We therefore can:
- remove enabling an extension in the locked screen ui's profile (e.g. adding
ChromeVox as a component extension).
- stop injection of content scripts into the locked screen's web ui.

TEST=navigate to locked screen with ChromeVox on. Verify things work. Toggle
ChromeVox off and on. Verify things still work.
BUG=676467,611736
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2617633004
Cr-Commit-Position: refs/heads/master@{#442970}
Committed:
https://chromium.googlesource.com/chromium/src/+/4702f8e80cde7762dc897963ab69...
==========

[commit-bot: I haz the power, 2017-01-11 18:54:46]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/4702f8e80cde7762dc897963ab69...