1069757 - browser_urlbarCopying.js fails with e10s

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Bug-XXXXXXX-fix-urlbarCopying-test-under-e10s.-r-dao.patch

browser_urlbarCopying.js is currently disabled due to its use of the clipboard (bug 932651) - however, once that is fixed there is one remaining problem.  The test waits for a "load" event on the child <browser>, but this fires too early for the test - the currentURI hasn't been updated by that point.

This patch adds a web-progress listener and performs the check once onLocationChange fires.

Asking dao for review as he has touched this file relatively recently...

Comment 1

[Dão Gottwald [:dao]]

Comment on attachment 8492004 [details] [diff] [review]
0001-Bug-XXXXXXX-fix-urlbarCopying-test-under-e10s.-r-dao.patch

Why do we get the load event before onLocationChange? This seems backwards and like a potential source for confusion for production code...

Comment 2

[Mark Hammond [:markh] [:mhammond]]

(In reply to Dão Gottwald [:dao] from comment #1)
> Comment on attachment 8492004 [details] [diff] [review]
> 0001-Bug-XXXXXXX-fix-urlbarCopying-test-under-e10s.-r-dao.patch
> 
> Why do we get the load event before onLocationChange? This seems backwards
> and like a potential source for confusion for production code...

The load event is on the browser itself.  This load event isn't coordinated with events in the child process - ie, the load event on the content-process document could theoretically happen before or after the browser element's load event in the parent.

Does that clarify?

Comment 3

[Dão Gottwald [:dao]]

The load event on the browser is still about the content being loaded, which means it originally comes from the content process. Right?

Comment 4

[:Gavin Sharp [email: [e-mail]]]

I agree with dao, the load event should be fine for this. But it is possible there are multiple loads here and we're not waiting for the right one? That's a common issue when loading things in a new tab, for example.

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8492004 [details] [diff] [review]
0001-Bug-XXXXXXX-fix-urlbarCopying-test-under-e10s.-r-dao.patch

OK, no problems, we'll just leave this disabled in e10s for the time being.

Comment 6

[:Gijs (out for now; he/him)]

Attachment: re-enable browser_urlbarCopying.js in e10s mode,

Locally, this test runs fine in e10s at this point. Try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ade58da9cb1f

Comment 7

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8517059 [details] [diff] [review]
re-enable browser_urlbarCopying.js in e10s mode,

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

awesome!

Comment 8

[:Gijs (out for now; he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/80a1da90e093

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/80a1da90e093