586506 - unregisterOpenPage should not come before registerOpenPage when removeTab is called before/during onLocationChange

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Marco Bonardo [:mak]]

Attachment: patch v1.0

I've found this other glitch in switch-to-tab code that is causing a failure when i push bug 546253, since that bug is checking about:blank from previous tests too.

also makes the test wait for proper visit notification (to be sure data is in the db before looking for it).

Will need approval to land with bug 546253

Comment 1

[Marco Bonardo [:mak]]

to explain, lastURI is setup when page is registered, so it's a safer check to unregister.

Comment 2

[Marco Bonardo [:mak]]

Comment on attachment 465011 [details] [diff] [review]
patch v1.0

Discussed with dao, the problem with this approach is that we would still fail if someone calls removeTab in a sub onLocationChange listener because lastURI is setup later. But at the same time if we set lastURI before the listeners will be unable to know previous uri.

So, the only best solution I can think of is to have a specific .registeredOpenURI property for this scope.

Comment 3

[Marco Bonardo [:mak]]

Attachment: patch v1.1

use a new property

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 465673 [details] [diff] [review]
patch v1.1

s/registeredOpenURI/_registeredOpenURI/

I haven't really reviewed the test, but this can be a plain function thanks to bug 538920:

>+  let visitObserver = {
>+    observe: function(aSubject, aTopic, aData)

Comment 5

[Marco Bonardo [:mak]]

Attachment: patch v1.2

addressed comment

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 465681 [details] [diff] [review]
patch v1.2

>+    Services.obs.addObserver(
>+      function (aSubject, aTopic, aData) {
>+        if (url != aSubject.QueryInterface(Ci.nsIURI).spec)
>+          return;
>+        Services.obs.removeObserver(this, aTopic);

'this' needs to be arguments.callee instead

Comment 7

[Marco Bonardo [:mak]]

wtf, yeah, I hate last minute changes (doing in seconds)

Comment 8

[Marco Bonardo [:mak]]

Attachment: patch v1.3

Comment 9

[Marco Bonardo [:mak]]

Attachment: patch v1.4

with correct commit message

Comment 10

[Jeff Muizelaar [:jrmuizel]]

http://hg.mozilla.org/mozilla-central/rev/25e0830abbe4