The default bug view has changed. See this FAQ.

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

RESOLVED FIXED

Status

()

Firefox
Session Restore
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: mmm, Assigned: mmm)

Tracking

({intermittent-failure})

Trunk
x86_64
Mac OS X
intermittent-failure
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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: 625257

Updated

6 years ago
Blocks: 438871
Created attachment 505985 [details] [diff] [review]
Rework test to not use session save events.

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 hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
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+
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Created attachment 506519 [details] [diff] [review]
Renaming and removing state tracker from test.

Will check-in pending a run through try.
Attachment #505985 - Attachment is obsolete: true
Attachment #506519 - Flags: review+
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
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.
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Must've forgotten to close this as resolved when I committed the patch.

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