622229 - Change screen resolution of Mac 10.6 slaves to 1600x1200

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

+++ This bug was initially created as a clone of Bug #592903 +++

I discovered by accident while working on bug 615386 that we're not using WIDGET_LAYERS in reftests on 10.6.  There's a typo at

  http://hg.mozilla.org/try/file/6382e5da5720/layout/tools/reftest/reftest.js#l930

which causes

  JavaScript error: chrome://reftest/content/reftest.js, line 930: gBrowserRemote is not defined

to be printed and (!=) reftests to fail when we're not using WIDGET_LAYERS.  See

  http://tinderbox.mozilla.org/showlog.cgi?tree=MozillaTry&errorparser=unittest&logfile=1293742883.1293744396.15119.gz&buildtime=1293742883&buildname=Rev3%20MacOSX%20Snow%20Leopard%2010.6.2%20tryserver%20debug%20test%20reftest&fulltext=1#err1

It's not clear to me why only the 10.5 slaves where changed in bug 592903; we definitely want to test the usual drawing paths on 10.6 as well.  Looks like a miscommunication persisted starting around bug 580410 comment 14.

This also prevents us from reftest-ing out-of-process content, but is a lower priority than fixing the linux slaves since we have coverage on 10.5 and we're not shipping out-of-process content on mac anytime soon, unlike on linux where we already are shipping it.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Oh, didn't read far enough down --- looks like bug 600899 was indirectly breaking 10.6.  Now that that's fixed, anything else in the way?

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

We should set the desktop background to something plain at the same time, eg a solid color. Loading the 10.5 machines at high res with the default background takes simply ages from the bottom of the world.

Comment 3

[Joe Drew (not getting mail)]

Armen, this has become a priority again since we've disabled OpenGL on 10.5. As it is, we don't have any OS X OpenGL coverage.

REFTEST INFO | WARNING: USE_WIDGET_LAYERS disabled
REFTEST INFO | drawWindow flags = DRAWWINDOW_DRAW_CARET | DRAWWINDOW_DRAW_VIEW; window size = 801,998; test browser size = 800,1000

(At minimum we need the dock moved/autohidden, I think.)

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Rob proposed specially-enabling GL for 10.5 on tinderbox as a workaround.  Not sure what that would entail.

Comment 5

[Armen [:armenzg]]

joe we need approval fro drivers.
Would Tuesday/Wednesday work?
What has higher priority this or DirectX/Drivers deployment?

The dock is hidden at the bottom.

Changing the screen resolution and moving the dock to the right will take the following:
- deploy a new cscreen.resize.plist[1]/com.apple.windowserver.plist[2] on staging (create it on the ref machine)
- check results are good
- deploy cscreen.resize.plist/com.apple.windowserver.plist on production (no puppet changes are needed [3])
- make sure that file shows in all 3 different puppet locations

This will affect talos numbers for sure.

[1] http://production-puppet.build.mozilla.org/production/darwin10-i386/test/Library/Preferences/com.apple.windowserver.plist
[2] http://production-puppet.build.mozilla.org/production/darwin10-i386/test/Library/LaunchAgents/cscreen.resize.plist
[3] http://hg.mozilla.org/build/puppet-manifests/file/6ff0a47aa738/os/talos_osx.pp#l54

Comment 6

[Armen [:armenzg]]

Attachment: cscreen.plist - original file

Comment 7

[Armen [:armenzg]]

Attachment: cscreen.plist - change resolution to 1600x1200 on 10.6 machines

How does the change affect the reftest runs?
* Before the change:
REFTEST INFO | WARNING: USE_WIDGET_LAYERS disabled
REFTEST INFO | drawWindow flags = DRAWWINDOW_DRAW_CARET | DRAWWINDOW_DRAW_VIEW; window size = 801,998; test browser size = 800,1000

* After the change:
REFTEST INFO | drawWindow flags = DRAWWINDOW_DRAW_CARET | DRAWWINDOW_DRAW_VIEW | DRAWWINDOW_USE_WIDGET_LAYERS; window size = 801,1022; test browser size = 800,1000

I incorrectly mentioned com.apple.windowserver.plist but it is ~/Library/Preferences/com.apple.dock.plist that takes care of dock changes.
This currently not managed by puppet.
I tried to get it to work but I got puppet errors. 3.6.14's GO to build is imminent and I can't debug the desktop colour change.

Let's focus only on the screen resolution which is what we need.

Here is what I am testing:
[root@staging-puppet Preferences]# md5sum  /N/staging/darwin9-i386/test/Library/LaunchAgents/cscreen.resize.plist f7f260e7c8e7d24a889e7198c1709bca  /N/staging/darwin9-i386/test/Library/LaunchAgents/cscreen.resize.plist

Comment 8

[Chris Cooper [:coop] (he/him)]

Comment on attachment 505944 [details] [diff] [review]
cscreen.plist - change resolution to 1600x1200 on 10.6 machines

Fix looks fine, but isn't this file in VCS somewhere?

Comment 9

[Armen [:armenzg]]

Not that I know of.

We only have it in here AFAIK:
http://production-puppet.build.mozilla.org/production/darwin10-i386/test/Library/LaunchAgents/cscreen.resize.plist

TODO figure out where to store it.

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

It should be in a module in puppet-manifests.

Comment 11

[Armen [:armenzg]]

Attachment: screen resolution module

dustin is this correct?

Right now it happens that both Leopard and Snow Leopard machines share the same cscreen and cscreen.resize.plist. How could I differentiate both if I needed to?

Comment 12

[Armen [:armenzg]]

I checked results on staging and we are ready to go tomorrow.

If there is enough time I could test the patch I just attached and move to the modules style.

Comment 13

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 506772 [details] [diff] [review]
screen resolution module

I'd like to have modules named after their task, rather than the slaves that should be assigned to them.  So this should be 'screensize' or, perhaps more generally, 'gui' (in which case we can add things like autologin, background, etc. to it)

A UserName is redundant (and will cause a warning) in a LaunchAgent plist, since it always runs as the console user (cltbld in this case).

You can distinguish the plists in whatever way makes sense.  Personally, I would probably insert the relevant info in the middle, so that the files appear together, and have the same filename extension.  Note that plists are usually named with a reversed DNS name.  So IMHO the best would be
  puppet://gui/org.mozilla.build.cscreen-leopard.plist
  puppet://gui/org.mozilla.build.cscreen-snow.plist
This would come with a corresponding change to the Label key in the plist to 'org.mozilla.build.cscreen'.

also, note that you can use templates if the plists are only slightly different:
http://hg.mozilla.org/build/puppet-manifests/file/6ff0a47aa738/modules/buildslave/templates/org.mozilla.build.buildslave.plist.erb

Comment 14

[Armen [:armenzg]]

FTP the new screen resolution was turned live at 8AM PDT on Wednesday January 26th.

The reference machine has been updated with the new /Library/LaunchAgents/cscreen.resize.plist, it has been rebooted, have verified it through VNC and have been requested to be refreshed in bug 617105.

The documentation has been updated to track the new screen resolution:
https://wiki.mozilla.org/ReferencePlatforms/Test/SnowLeopard#Manage_dock_and_screen_resolution

I have to debug talos-r3-snow-014 on bug 629598.

We will keep moving cscreen.resize.plist to "modules" on another bug.

I will file another bug to change the background colour to something plain.

AFAIK there is nothing left on this bug to be done beside fixing the dependent bugs.