Closed Bug 604458 Opened 14 years ago Closed 14 years ago

Intermittent failure in browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | correct URL for normal mode - Got about:blank, expected about:robots

Categories

(Firefox Graveyard :: Panorama, defect, P3)

x86_64
Linux
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ehsan.akhgari, Assigned: iangilman)

References

Details

(Keywords: intermittent-failure, Whiteboard: [qa-])

Attachments

(1 file)

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1287084415.1287085150.14449.gz Rev3 Fedora 12x64 mozilla-central opt test mochitest-other on 2010/10/14 12:26:55 TEST-START | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | we start with 1 tab TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | we now have 2 tabs TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | Tab View is visible TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | Tab View is no longer visible TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | private browsing is on TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | we have 1 tab in private browsing TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | correct URL for private browsing TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | Tab View is visible again TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | Console message: TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | private browsing is off TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | we have 2 tabs in normal mode TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | correct URL for normal mode TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | correct URL for normal mode TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | Tab View is not visible TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | Tab View is still not visible TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | private browsing is on TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | we have 1 tab in private browsing TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | correct URL for private browsing TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | Console message: TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | private browsing is off TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | we have 2 tabs in normal mode TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | correct URL for normal mode TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | correct URL for normal mode - Got about:blank, expected about:robots TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | we finish with one tab TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | we finish with private browsing off TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | we finish with Tab View not visible TEST-INFO | chrome://mochitests/content/browser/browser/base/content/test/tabview/browser_tabview_privatebrowsing.js | Test took 1.585s to complete
Assignee: nobody → ian
OS: Linux → Windows CE
Priority: -- → P3
This failure (expecting "about:robots" but getting "about:blank") seems like we're not waiting long enough for the tabs to load. The routine we use to wait is this: function afterAllTabsLoaded(callback) { let stillToLoad = 0; function onLoad() { this.removeEventListener("load", onLoad, true); stillToLoad--; if (!stillToLoad) callback(); } for (let a = 0; a < gBrowser.tabs.length; a++) { let browser = gBrowser.tabs[a].linkedBrowser; if (browser.webProgress.isLoadingDocument) { stillToLoad++; browser.addEventListener("load", onLoad, true); } } } Ehsan, any thoughts on what may be going wrong? Is it possible we have a race condition here? If so, how would we fix it? Maybe .isLoadingDocument isn't a reliable way of knowing whether the tab still needs to load? What if it needs to load but hasn't started loading yet?
The problem with this function is that it doesn't check which load event it's handling. The PB service internally opens an about:blank tab first and then restores the session, so I'm suspecting that you're getting that load event instead of the load event that you expect to receive. Why don't you just listen for private-browsing-transition-complete?
(In reply to comment #6) > The problem with this function is that it doesn't check which load event it's > handling. The PB service internally opens an about:blank tab first and then > restores the session, so I'm suspecting that you're getting that load event > instead of the load event that you expect to receive. But we're only subscribing to the tabs we know about; we'd never get the load event for an additional tab. I suppose not checking which load event would be a problem if we got multiple load events for the same tab, but even then we're removing the handler after every event, so not even that should happen, right? > Why don't you just listen for private-browsing-transition-complete? Does this fire after all of the tabs have finished loading? We need them to finish loading before we continue the test. Also, it would be nice to have a nice solid afterAllTabsLoaded function if we can, as it's something we can make use of in other tests as well.
Blocks: 597043
(In reply to comment #8) > (In reply to comment #6) > > The problem with this function is that it doesn't check which load event it's > > handling. The PB service internally opens an about:blank tab first and then > > restores the session, so I'm suspecting that you're getting that load event > > instead of the load event that you expect to receive. > > But we're only subscribing to the tabs we know about; we'd never get the load > event for an additional tab. Session store reuses already open tabs, so you never know! > I suppose not checking which load event would be a problem if we got multiple > load events for the same tab, but even then we're removing the handler after > every event, so not even that should happen, right? Yes, and since you're not checking for the location in the load event, you don't really know which one you've handled. > > Why don't you just listen for private-browsing-transition-complete? > > Does this fire after all of the tabs have finished loading? We need them to > finish loading before we continue the test. Yes. > Also, it would be nice to have a nice solid afterAllTabsLoaded function if we > can, as it's something we can make use of in other tests as well. Well, you can use sessionstore-browser-state-restored for other tests.
(In reply to comment #9) > > > Why don't you just listen for private-browsing-transition-complete? > > > > Does this fire after all of the tabs have finished loading? We need them to > > finish loading before we continue the test. > > Yes. Actually, it doesn't fire after all the load events have been dispatched. It is fired after the restoration process is finished. And I assume that you can listen to load event handlers from that point if needed.
Attached patch patch v1Splinter Review
Ok, now waiting for private-browsing-transition-complete before waiting for tabs to load. As a side note, should we be observing private-browsing-transition-complete in our main PB handling code in tabview/ui.js? Right now we're observing private-browsing-change-granted, private-browsing, and sessionstore-browser-state-restored.
Attachment #484513 - Flags: feedback?(ehsan)
Status: NEW → ASSIGNED
(In reply to comment #12) > As a side note, should we be observing private-browsing-transition-complete in > our main PB handling code in tabview/ui.js? Right now we're observing > private-browsing-change-granted, private-browsing, and > sessionstore-browser-state-restored. You should be observing private-browsing-transition-complete instead of sessionstore-browser-state-restored. Please file a separate bug for that though.
Comment on attachment 484513 [details] [diff] [review] patch v1 r=me (as the owner of the PB module).
Attachment #484513 - Flags: feedback?(ehsan) → review+
(In reply to comment #13) > You should be observing private-browsing-transition-complete instead of > sessionstore-browser-state-restored. Please file a separate bug for that > though. Done. Bug 605935.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [orange] → [orange][qa-]
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
OS: Windows CE → Linux
Moving to b9
Blocks: 598154
No longer blocks: 597043
Depends on: 623330
bugspam (moving b9 to b10)
Blocks: 608028
bugspam (removing b9)
No longer blocks: 598154
I believe this is fixed by a combination of bug 624265 and bug 610242.
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Whiteboard: [orange][qa-] → [qa-]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: