1459301 - Fix and add more onNewSession tests

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P2)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

Fix the existing onNewSession tests and add a few more involving onNewSession behavior.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1459301 - 1. Wait on sessions opened through onNewSession response;

The session returned through the onNewSession response is opened
internally by GV. GeckoSessionTestRule should trap that and wait on the
newly opened session, just like we do for any other session, so that
this internal open operation is transparent to the test.

Review commit: https://reviewboard.mozilla.org/r/242882/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242882/

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1459301 - 2. Allow nested loopUntilIdle calls;

There may be cases where loopUntilIdle calls are nested. Allow these
calls by canceling the previous timeout when we set a new timeout. This
patch also adds some general optimizations to loopUntilIdle, so we don't
create a bunch of objects with every call.

Review commit: https://reviewboard.mozilla.org/r/242884/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242884/

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1459301 - 3. Fix and add onNewSession tests;

Use RDP instead of synthesizeTap to fix the onNewSession tests. Also add
some additional onNewSession tests, including setting / not-setting of
the window opener.

Review commit: https://reviewboard.mozilla.org/r/242886/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242886/

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8974542 [details]
Bug 1459301 - 1. Wait on sessions opened through onNewSession response;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242882/diff/1-2/

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8974543 [details]
Bug 1459301 - 2. Allow nested loopUntilIdle calls;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242884/diff/1-2/

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8974544 [details]
Bug 1459301 - 3. Fix and add onNewSession tests;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242886/diff/1-2/

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8974542 [details]
Bug 1459301 - 1. Wait on sessions opened through onNewSession response;

https://reviewboard.mozilla.org/r/242882/#review249276

The about:blank stuff is such a giant PITA. We should try to fix that in Gecko some day if possible.

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8974543 [details]
Bug 1459301 - 2. Allow nested loopUntilIdle calls;

https://reviewboard.mozilla.org/r/242884/#review249280

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8974544 [details]
Bug 1459301 - 3. Fix and add onNewSession tests;

https://reviewboard.mozilla.org/r/242886/#review249282

Nice!

::: mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt:362
(Diff revision 2)
> -        sessionRule.delegateDuringNextWait(object : Callbacks.NavigationDelegate {
> +        sessionRule.session.evaluateJS("window.open('newSession_child.html', '_blank')")
> +
> +        sessionRule.session.waitUntilCalled(object : Callbacks.NavigationDelegate {
> +            @AssertCalled(count = 1, order = [1])
> +            override fun onLoadRequest(session: GeckoSession, uri: String, where: Int, flags: Int, response: GeckoResponse<Boolean>) {
> +                assertThat("URI should be correct", uri, endsWith(NEW_SESSION_CHILD_HTML_PATH))

We could make assertions about the flags here too.

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8974542 [details]
Bug 1459301 - 1. Wait on sessions opened through onNewSession response;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242882/diff/2-3/

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8974543 [details]
Bug 1459301 - 2. Allow nested loopUntilIdle calls;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242884/diff/2-3/

Comment 12

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8974544 [details]
Bug 1459301 - 3. Fix and add onNewSession tests;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/242886/diff/2-3/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1b3a0ac4aa60
1. Wait on sessions opened through onNewSession response; r=snorp
https://hg.mozilla.org/integration/autoland/rev/c0c7fa0ae459
2. Allow nested loopUntilIdle calls; r=snorp
https://hg.mozilla.org/integration/autoland/rev/0b3b40464bbd
3. Fix and add onNewSession tests; r=snorp

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1b3a0ac4aa60
https://hg.mozilla.org/mozilla-central/rev/c0c7fa0ae459
https://hg.mozilla.org/mozilla-central/rev/0b3b40464bbd

Comment 15

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: 4. Follow up to fix tests for local runs (v1)

Manually turn off the popup blocker for all link-clicking tests so local
runs of the tests pass.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ddc3b915c34
4. Follow up to fix tests for local runs; r=me

Comment 17

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/6ddc3b915c34