talos needs to be retrofitted for e10s

RESOLVED FIXED

Status

defect
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: jmaher, Assigned: jmaher)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

9 years ago
currently talos uses quit.js and this doesn't work when running inside of e10s.  We need to look through all the talos code and make sure we proxy everything to chrome process.

Updated

9 years ago
Blocks: mobile-pool

Comment 1

9 years ago
Is there an equivalent bug for unit tests?
Looks like we have false greens on maemo devices on unit tests (bug 579184); the browser isn't actually launched.
(Assignee)

Comment 2

9 years ago
this patch requires the mobile_profile.zip (which I will upload next) and works on fennec desktop linux.  This changes the profile for mobile from base_profile to mobile_profile and mobile_profile has an extension which adds an overlay (same technique as mochitest)
Assignee: nobody → jmaher
Attachment #461104 - Flags: review?(aki)
(Assignee)

Comment 3

9 years ago
this is the new profile directory.
Attachment #461106 - Flags: review?(aki)
(Assignee)

Updated

9 years ago
Depends on: 582997

Comment 4

9 years ago
Comment on attachment 461106 [details]
the new mobile_profile with the talos e10s extension (1.0)

I'll be staging this and the other patches shortly.

One thing about this zip -- you included the CVS subdirectories, which means any changes or cvs actions on this directory tree will affect the directory you copied it from (base_profile) instead of the new subdirectory tree you want to create.  So you want to remove mobile_profile/bookmarkbackups/CVS, mobile_profile/Cache/CVS and mobile_profile/CVS.

Updated

9 years ago
Attachment #461106 - Attachment is patch: false
Attachment #461106 - Attachment mime type: text/plain → application/octet-stream

Comment 5

9 years ago
Comment on attachment 461104 [details] [diff] [review]
core changes required to get ts, twinopen working on talos (1.0)

Things are looking good here, other than fennecmark (bug 582997).
The mobile config changes are easy peasy to r+.
Have you verified that the other changes won't break desktop?
Attachment #461104 - Flags: review?(anodelman)

Comment 6

9 years ago
Comment on attachment 461106 [details]
the new mobile_profile with the talos e10s extension (1.0)

This appears to work.
Do you know if this will conflict at all with the new pageloader xpi? I'm putting that in distribution/bundles (bug 580698) and that also has a quit.js.

Other than that question and the CVS dirs, this looks good to me.
(Assignee)

Comment 7

9 years ago
I don't know much about the pageloader xpi stuff.  It could conflict, but when is that landing?  This solution is more of a temporary shim until we have a more complete solution in place.

As for the CVS dirs, I have updated the patch to just have the extensions directory (all other dirs just had CVS cruft in there)
Attachment #461106 - Attachment is obsolete: true
Attachment #461638 - Flags: review?(aki)
Attachment #461106 - Flags: review?(aki)

Comment 8

9 years ago
(In reply to comment #7)
> I don't know much about the pageloader xpi stuff.  It could conflict, but when
> is that landing?

I'm testing it in conjunction with this.
I can switch over to that pageloader at any point.

Updated

9 years ago
Attachment #461638 - Attachment is patch: false
Attachment #461638 - Attachment mime type: text/plain → application/octet-stream

Comment 9

9 years ago
Comment on attachment 461638 [details]
the new mobile_profile with the talos e10s extension (2.0)

Did you delete the zip before zipping everything up? zip will add to an archive, not overwrite it.

new-host-3:foo asasaki$ unzip ../mobile_profile.zip 
Archive:  ../mobile_profile.zip
  inflating: mobile_profile/localstore.rdf  
  inflating: mobile_profile/prefs.js  
   creating: mobile_profile/bookmarkbackups/
   creating: mobile_profile/bookmarkbackups/CVS/
 extracting: mobile_profile/bookmarkbackups/CVS/Entries  
  inflating: mobile_profile/bookmarkbackups/CVS/Repository  
 extracting: mobile_profile/bookmarkbackups/CVS/Root  
   creating: mobile_profile/Cache/
   creating: mobile_profile/Cache/CVS/
 extracting: mobile_profile/Cache/CVS/Entries  
 extracting: mobile_profile/Cache/CVS/Repository  
 extracting: mobile_profile/Cache/CVS/Root  
   creating: mobile_profile/CVS/
  inflating: mobile_profile/CVS/Entries  
  inflating: mobile_profile/CVS/Entries.Log  
 extracting: mobile_profile/CVS/Repository  
 extracting: mobile_profile/CVS/Root  
   creating: mobile_profile/extensions/
   creating: mobile_profile/extensions/electrolysis@mozilla.org/
   creating: mobile_profile/extensions/electrolysis@mozilla.org/chrome/
   creating: mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/
  inflating: mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc-overlay.xul  
  inflating: mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc.js  
  inflating: mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/quit.js  
  inflating: mobile_profile/extensions/electrolysis@mozilla.org/chrome.manifest  
  inflating: mobile_profile/extensions/electrolysis@mozilla.org/install.rdf
(Assignee)

Comment 10

9 years ago
updated .zip to be just the few files we need for the mobile_profile.
Attachment #461638 - Attachment is obsolete: true
Attachment #463239 - Flags: review?(aki)
Attachment #461638 - Flags: review?(aki)
Comment on attachment 463239 [details]
[checked in]the new mobile_profile with the talos e10s extension (2.1)

/me changes zip from patch->application/octet-stream again :)
Attachment #463239 - Attachment is patch: false
Attachment #463239 - Attachment mime type: text/plain → application/octet-stream
Comment on attachment 463239 [details]
[checked in]the new mobile_profile with the talos e10s extension (2.1)

I'm going to stamp this, since it can't make anything worse, and I think even if it does conflict with the new pageloader, I did see some successful talos runs with this.
Attachment #463239 - Flags: review?(aki) → review+
Comment on attachment 461104 [details] [diff] [review]
core changes required to get ts, twinopen working on talos (1.0)

This has wfm in testing (spotty, because of network flakiness, but I've seen greens).  However, I'm still concerned that quit.js and getInfo.html changes might affect desktop talos negatively, so keeping the Alice r?
Attachment #461104 - Flags: review?(aki) → review+

Updated

9 years ago
Duplicate of this bug: 575506
Have these changes been testing on staging talos?
They have not, afaik.
I would want at least a small run in staging before we consider changing the basic functionality.
I've agreed to do the staging for jmaher for this patch.
(Assignee)

Comment 19

9 years ago
updated patch to work better with firefox desktop (in non ipc mode).  Tested on linux desktop fennec (ipc on/off) and firefox (non ipc).  I did not do any tp tests or pageloader tests.

Alice if you could indicate when this might get a change at some talos staging that would be helpful.
Attachment #461104 - Attachment is obsolete: true
Attachment #461104 - Flags: review?(anodelman)
I will start a staging run on this tomorrow morning, so that I can leave my current staging test overnight to run to completion.
Staging run started.
So far I'm seeing green mixed in with quite a bit of orange related to failure to initialize the browser.  I'm trying to sort through it now and I'll work with jmaher tomorrow to see if this can be resolved.
(Assignee)

Comment 23

9 years ago
after looking over this code, it is the same code I am using in mochitest which has passed on all platforms and has been running in this fashion on all tinderbox unittests (for mochitest) for the last 7 weeks.  With that said there still could be issues with the difference in setup and how Firefox is being run.
From the test runs overnight the issue is only on mac talos (snow leopard/leopard).  The patch causes the browser to fail to be initialized and no tests are run.  As such this is r- for production landing.
(Assignee)

Comment 25

9 years ago
So I ran these tests on my MBP OSX and it completed a run (I didn't have the TP4 stuff setup), but ts, ts_cold, ts_shutdown, ts_places*, twinopen all ran and produced results (in the output/*.csv format.)

I did nothing to change my code, I just used the patches applied to this bug.  Could it be there was a bad build of OSX Firefox when testing these in staging?
I'm also seeing success when I manually ran on a single slave (talos-r3-leopard-002).  This is going to require a second staging run to determine what is going on.
We are getting some mixed results in staging, but our confidence is high enough that we'd like to land during a downtime with a close, close eye on mac talos results.
Checking in getInfo.html;
/cvsroot/mozilla/testing/performance/talos/getInfo.html,v  <--  getInfo.html
new revision: 1.4; previous revision: 1.3
done
Checking in mobile.config;
/cvsroot/mozilla/testing/performance/talos/mobile.config,v  <--  mobile.config
new revision: 1.6; previous revision: 1.5
done
RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/localstore.rdf,v
done
Checking in mobile_profile/localstore.rdf;
/cvsroot/mozilla/testing/performance/talos/mobile_profile/localstore.rdf,v  <--  localstore.rdf
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/prefs.js,v
done
Checking in mobile_profile/prefs.js;
/cvsroot/mozilla/testing/performance/talos/mobile_profile/prefs.js,v  <--  prefs.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome.manifest,v
done
Checking in mobile_profile/extensions/electrolysis@mozilla.org/chrome.manifest;
/cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome.manifest,v  <--  chrome.manifest
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/install.rdf,v
done
Checking in mobile_profile/extensions/electrolysis@mozilla.org/install.rdf;
/cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/install.rdf,v  <--  install.rdf
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc-overlay.xul,v
done
Checking in mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc-overlay.xul;
/cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc-overlay.xul,v  <--  ipc-overlay.xul
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc.js,v
done
Checking in mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc.js;
/cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/ipc.js,v  <--  ipc.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/quit.js,v
done
Checking in mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/quit.js;
/cvsroot/mozilla/testing/performance/talos/mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/quit.js,v  <--  quit.js
initial revision: 1.1
done
Checking in page_load_test/quit.js;
/cvsroot/mozilla/testing/performance/talos/page_load_test/quit.js,v  <--  quit.js
new revision: 1.2; previous revision: 1.1
done
Checking in startup_test/fennecmark/fennecmark.html;
/cvsroot/mozilla/testing/performance/talos/startup_test/fennecmark/fennecmark.html,v  <--  fennecmark.html
new revision: 1.2; previous revision: 1.1
done
Attachment #463239 - Attachment description: the new mobile_profile with the talos e10s extension (2.1) → [checked in]the new mobile_profile with the talos e10s extension (2.1)
Attachment #464577 - Attachment description: core changes required to get ts, twinopen working on talos (2.0) → [checked in]core changes required to get ts, twinopen working on talos (2.0)
Joel, John, Alice: you guys rock.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
See Also: → 1050706
You need to log in before you can comment on or make changes to this bug.