731155 - getTab(aIndex) helper function from controller causes errors in Mozmill /testPrivateBrowsing tests for default branch

Status: VERIFIED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Maniac Vlad Florin (:vladmaniac)]

Build ID: 
Build identifier: Mozilla/5.0 (X11; Linux i686; rv:13.0) Gecko/20120227 Firefox/13.0a1
--------------------------------------------------------------------------------------
Mozmill Version: 1.5.9
--------------------------------------------------------------------------------------
Firefox Version: 13
--------------------------------------------------------------------------------------
Tests affected: 
/testPrivateBrowsing/testTabRestoration.js 	
/testPrivateBrowsing/testStartStopPBMode.js 	
/testPrivateBrowsing/testCloseWindow.js
--------------------------------------------------------------------------------------
Error: 
TimeoutError("controller.waitForPageLoad(): Timeout waiting for page loaded.")@resource://mozmill/modules/utils.js:429 waitFor((function () {return this.isLoaded(win);}),"controller.waitForPageLoad(): Timeout waiting for page loaded.",30000,100,[object Object])@resource://mozmill/modules/utils.js:467 ((function () {return this.isLoaded(win);}),"controller.waitForPageLoad(): Timeout waiting for page loaded.",30000,(void 0),[object Object])@resource://mozmill/modules/controller.js:648 ([object Proxy])@resource://mozmill/modules/controller.js:1291 testStartStopPrivateBrowsingMode()@resource://mozmill/modules/frame.js -> file:///c:/docume~1/mozilla/locals~1/temp/tmpluui_m.mozmill-tests/tests/functional/testPrivateBrowsing/testStartStopPBMode.js:118 (testStartStopPrivateBrowsingMode)@resource://mozmill/modules/frame.js:557 ([object Object])@resource://mozmill/modules/frame.js:626 ([object Object])@resource://mozmill/modules/frame.js:669 ("c:\\docume~1\\mozilla\\locals~1\\temp\\tmpluui_m.mozmill-tests\\tests\\functional\\testPrivateBrowsing\\testStartStopPBMode.js")@resource://mozmill/modules/frame.js:506 ("c:\\docume~1\\mozilla\\locals~1\\temp\\tmpluui_m.mozmill-tests\\tests\\functional\\testPrivateBrowsing\\testStartStopPBMode.js")@resource://mozmill/modules/frame.js:681 ((function (filename, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestFile(filename);runner.end();return true;}),[object Proxy])@resource://jsbridge/modules/server.js:179 ("f729aff3-615e-11e1-83be-005056f3c7a8",(function (filename, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestFile(filename);runner.end();return true;}),[object Proxy])@resource://jsbridge/modules/server.js:183 
--------------------------------------------------------------------------------------
Started failing: 2012-02-27
--------------------------------------------------------------------------------------
Firefox Build failing first: 
Built from http://hg.mozilla.org/mozilla-central/rev/d1b2fd680235
--------------------------------------------------------------------------------------
Code snippet: 
    dump("\n\n Page number OK " + LOCAL_TEST_PAGES.length + "\n\n");
    //XXX: This line of code is failing 
    controller.waitForPageLoad(controller.tabs.getTab(i));
--------------------------------------------------------------------------------------

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

At which state does the failure happen? That would be helpful information which is still missing. Are the right amount of tabs open? Does the contained page load correctly?

Comment 2

[Remus Pop (:RemusPop)]

The right amount of tabs are open.
The problem is with controller.tabs.getTab(i). It does not get the tab with index i.
While using tabBrowser class and it's getTab function, waitForPageLoad works correctly and I get a fail in elementslib: "this._view is undefined".

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

tabBrowser.getTab() doesn't work here because it doesn't get the tab document, but the visual tab. So not sure how waitForPageLoad would work at all.

Comment 4

[Remus Pop (:RemusPop)]

So, when doing --
window.gBrowser.browsers[0].contentDocument I get Error: Permission denied to access property 'gBrowser'. For comparison, release works fine, and looking at the reports I bet Aurora is fine with this property too.

Comment 5

[Remus Pop (:RemusPop)]

After restarting the browser, window.gBrowser.browsers[0].contentDocument works fine now.

Comment 6

[Remus Pop (:RemusPop)]

Somehow win.mozmillDocumentLoaded is false and never gets true, where win is the window object.

Comment 7

[Remus Pop (:RemusPop)]

I think that mozmillDocumentLoaded property is set to false after Private Browsing turns off, but not sure why.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Is this across platforms and only for Firefox 13? A regression?

Comment 9

[Remus Pop (:RemusPop)]

Yes, I've seen it across platforms.
Reports are here http://bit.ly/xTDlUy

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So please check what has been changed between the builds from Feb 26 and 27.

Comment 11

[Remus Pop (:RemusPop)]

Ok, found the bad boy: bug 711193.
Tried with setting "browser.sessionstore.restore_on_demand" to false in the setup module and the test passed.

What fix should we apply here? Should we bring up the tab before controller.waitForPageLoad (this is where we fail) ?

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Yeah, we should switch through all the open tabs.

Comment 13

[Remus Pop (:RemusPop)]

Attachment: patch v1 (default)

Because tabs only load their content when clicked, we must click on them before waiting for the content.
For testCloseWindow a new instantiation of tabBrowser must be made because we have a different controller.

Comment 14

[Maniac Vlad Florin (:vladmaniac)]

Comment on attachment 603688 [details] [diff] [review]
patch v1 (default)

I see only a nit: variable declarations must be separated from callbacks or other code blocks but 
I will not - this fix for that, because the initial test file was not compliant with these code 
standards. 

+ from me, over to Anthony to approve this fix

Comment 15

[u279076]

Comment on attachment 603688 [details] [diff] [review]
patch v1 (default)

>   // Check if all local pages were re-loaded and show their content
>   for (var i = 0; i < LOCAL_TEST_PAGES.length; i++) {
>+    tabBrowser = new tabs.tabBrowser(controller);

This line should not be inside the for loop, otherwise it needlessly gets reinitialized with every iteration.

Comment 16

[Remus Pop (:RemusPop)]

Attachment: patch v2 (default) [landed]

Moved the line before the for loop.

Comment 17

[u279076]

Comment on attachment 604347 [details] [diff] [review]
patch v2 (default) [landed]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/47451d564fff (default)

Comment 18

[u279076]

Please verify with tomorrow's testrun.

Comment 19

[Maniac Vlad Florin (:vladmaniac)]

http://mozmill-release.blargon7.com/#/functional/reports?branch=13.0&platform=All&from=2012-03-12&to=2012-03-12 

No errors - verified as the patch was intended only for the default branch