1477002 - AccessibilityTest times out with USE_MULTIPROCESS=false

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P2)

Description

[Eitan Isaacson [:eeejay]]

Attachment: Disable e10s in AccessibilityTest

If the tests are run with e10s off, they time out after the first test passes.

If the tests are run individually, they all pass. We should somehow figure out how to run this suite both with e10s on and off on a regular basis.

Comment 1

[Eitan Isaacson [:eeejay]]

I think I figured this out! patch soon.

Comment 2

[Eitan Isaacson [:eeejay]]

Attachment: Use message broadcaster to manage jsat content scripts. r?yzen

This patch does several things:
 1. When "domwindowopened" is dispatched it often doesn't have a
document yet, so we need to wait for it to load before determining if we
should attach it.
 2. Instead of managing individual message managers use a broadcaster
and load delayed scripts. This makes new window additions more robust.
 3. A content script now doesn't need a ready/start message but
initializes in-line. This added more complexity which we don't need. All
the info that we passed to it in AccessFu:Start can be gotten in other
ways (also, Services.appinfo.ID now works in child processes, so no need
for that).
 4. Tweaked the tests to support inline frame script initilization.
 5. Removed the scroll callback from content-script.js that was not used
anymore.

Comment 3

[Yura Zenevich [:yzen] (please use "needinfo")]

Doesn't seem to apply any more.

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Use message broadcaster to manage jsat content scripts. r?yzen

This patch does several things:
 1. When "domwindowopened" is dispatched it often doesn't have a
document yet, so we need to wait for it to load before determining if we
should attach it.
 2. Instead of managing individual message managers use a broadcaster
and load delayed scripts. This makes new window additions more robust.
 3. A content script now doesn't need a ready/start message but
initializes in-line. This added more complexity which we don't need. All
the info that we passed to it in AccessFu:Start can be gotten in other
ways (also, Services.appinfo.ID now works in child processes, so no need
for that).
 4. Tweaked the tests to support inline frame script initilization.
 5. Removed the scroll callback from content-script.js that was not used
anymore.

Comment 5

[Eitan Isaacson [:eeejay]]

Did you try the one I just attached?

Comment 6

[Chris Peterson [:cpeterson]]

P2 because GeckoView uses e10s by default so this doesn't block Focus+GeckoView.

Comment 7

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8994639 [details] [diff] [review]
Use message broadcaster to manage jsat content scripts. r?yzen

Review of attachment 8994639 [details] [diff] [review]:
-----------------------------------------------------------------

This is great, thanks!

::: accessible/jsat/AccessFu.jsm
@@ +28,5 @@
>    SET_SELECTION: "GeckoView:AccessibilitySetSelection",
>    CLIPBOARD: "GeckoView:AccessibilityClipboard",
>  };
>  
> +const ACCESSFU_MESSAGE = {

This should go into Constants.jsm

@@ +33,5 @@
> +  PRESENT: "AccessFu:Present",
> +  DOSCROLL: "AccessFu:DoScroll",
> +};
> +
> +const FRAME_SCRIPT = "chrome://global/content/accessibility/content-script.js";

Perhaps this too since we do use this in jsatcommon

::: accessible/jsat/Utils.jsm
@@ +399,5 @@
>      let message = (typeof(args[0]) === "function" ? args[0]() : args).join(" ");
>      message = "[" + Utils.ScriptName + "] " + this._LEVEL_NAMES[aLogLevel + 1] +
>        " " + message + "\n";
>      dump(message);
> +    // Note: used for testing purposes. If |this.useConsoleService| is true, also log

nit: this comment should probably move to where useConsoleService is declared (L391).

::: accessible/jsat/content-script.js
@@ +16,2 @@
>    "resource://gre/modules/accessibility/ContentControl.jsm");
>  ChromeUtils.defineModuleGetter(this, "Roles",

Roles is no longer used as well as Presentation.

::: mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt
@@ +357,5 @@
>  
>      @Test fun testMoveByLine() {
>          var nodeId = AccessibilityNodeProvider.HOST_VIEW_ID
>          sessionRule.session.loadTestPath(LOREM_IPSUM_HTML_PATH)
> +        waitForInitialFocus();

There are now more tests fun's could you update those too?

Comment 8

[Eitan Isaacson [:eeejay]]

Attachment: Use message broadcaster to manage jsat content scripts.

This patch does several things:
 1. When "domwindowopened" is dispatched it often doesn't have a
document yet, so we need to wait for it to load before determining if we
should attach it.
 2. Instead of managing individual message managers use a broadcaster
and load delayed scripts. This makes new window additions more robust.
 3. A content script now doesn't need a ready/start message but
initializes in-line. This added more complexity which we don't need. All
the info that we passed to it in AccessFu:Start can be gotten in other
ways (also, Services.appinfo.ID now works in child processes, so no need
for that).
 4. Tweaked the tests to support inline frame script initilization.
 5. Removed the scroll callback from content-script.js that was not used
anymore.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/beb25a105a98
Use message broadcaster to manage jsat content scripts. r=yzen

Comment 10

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/beb25a105a98