Closed Bug 627834 Opened 13 years ago Closed 13 years ago

Intermittent failure in browser_625257.js | newly created tab should be in save state

Categories

(Firefox :: Session Restore, defect)

Product:
Firefox
Component:
Session Restore
Platform:
x86_64
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: mmm, Assigned: mmm)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

Renaming and removing state tracker from test.
4.19 KB, patch
mmm
: review+
Details | Diff | Splinter Review 
Looks like this is a result from bug 625257.
Talked to zpao and we think that a session save state is being triggered from a previous test, or from changing the session save interval.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1295634590.1295637530.19591.gz

TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_625257.js | newly created tab should exist by now
JavaScript strict warning: chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_625257.js, line 76: reference to undefined property tab.linkedBrowser.__SS_data
ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_625257.js | newly created tab should be in save state
TEST-INFO | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_625257.js | Console message: [JavaScript Warning: "reference to undefined property tab.linkedBrowser.__SS_data" {file: "chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_625257.js" line: 76}]
++DOMWINDOW == 350 (0x145377148) [serial = 3932] [outer = 0x14159ec50]
TEST-PASS | chrome://mochitests/content/browser/browser/components/sessionstore/test/browser/browser_6252
Whiteboard: [orange]
Blocks: 438871
With this patch, the test manually does a save state but after the tab has loaded. This is to ensure that the tab has a history.count > 0, so that a save state will create a cache.

Because we do not wait for a save state, we no longer have to change any preferences!
Assignee: nobody → mars.martian+bugmail
Status: NEW → ASSIGNED
Attachment #505985 - Flags: review?(paul)
Comment on attachment 505985 [details] [diff] [review]
Rework test to not use session save events.

First thing's first: we don't need the state setting or checking. The test is pretty linear now and with the listners being attached mostly as needed, then we shouldn't get into weird states.

>-// First, the newly created blank tab should trigger a save state.
>-function test_bug625257_1() {
>+function test_bug625257_1(aEvent) {

Let's call this firstOnLoad or something and rename onLoad similarly.

>+  tab.linkedBrowser.removeEventListener("load", test_bug625257_1, true);
>+
>+  let uri = aEvent.target.location;
>+  is(uri, "about:blank", "first load should be for about:blank");
>+
>+  // Trigger a save state.
>+  ss.getBrowserState();
>+
>   is(gBrowser.tabs[1], tab, "newly created tab should exist by now");
>   ok(tab.linkedBrowser.__SS_data, "newly created tab should be in save state");
> 
>   tab.linkedBrowser.loadURI(URI_TO_LOAD);
>   state = 0;
> }
> 
> let tabsListener = {
>   onLocationChange: function onLocationChange(aBrowser) {
>     gBrowser.removeTabsProgressListener(tabsListener);
>-    is(state, 0, "should be the first listener event");
>+    is(state, 0, "should occur after about:blank load");
>     state++;

Since we're getting rid of the state checks, you can just check that the currentURI is correct

> 
>     // Since we are running in the context of tabs listeners, we do not
>     // want to disrupt other tabs listeners.
>     executeSoon(function() {
>       tab.linkedBrowser.removeEventListener("load", onLoad, true);

This listener will not have been added yet so nothing to remove here.

Otherwise it looks good and takes out the dependency on the save notification.
Attachment #505985 - Flags: review?(paul) → review+
Will check-in pending a run through try.
Attachment #505985 - Attachment is obsolete: true
Attachment #506519 - Flags: review+
Everything looked good on TryServer.
Pushed on mozilla-central: http://hg.mozilla.org/mozilla-central/rev/5d466f5defc9

Will keep bug open until branches merge and we can expect to not see this anymore.
Must've forgotten to close this as resolved when I committed the patch.

http://hg.mozilla.org/mozilla-central/rev/5d466f5defc9
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: