Bug 151135

Summary: [XvfbDriver] Fail to run all layout tests when X server started with -displayfd option
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, glenn, lforschler, mrobinson, ossy, pnormand, zan
Priority: P2 Keywords: Gtk
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Fix coding style
none
Updated patch darin: review+

Carlos Garcia Campos
Reported 2015-11-11 06:56:29 PST
Since my last debian update my xserver no longer uses the :<display-id> option, but -displayfd. The XvfbDriver uses the x server command line to check the displayas that are currently in use. This doesn't work when x server was satarted with -displayfd. This option is used to let the server find the display id available and it's written to the given file descriptor. With this option xord doesn't need to create the lock files in tmp either. The -displayfd option is also available in XVfb, so we could use it too when available. That would simplify the code, fixing also race conditions between we check for available displays and XVfb opens the connection, we wouldn't need to wait for 4 seconds after launching XVfb, and all lock files we are using won't be needed either.
Attachments
Patch (6.66 KB, patch)
2015-11-11 07:03 PST, Carlos Garcia Campos
no flags
Fix coding style (6.64 KB, patch)
2015-11-12 03:23 PST, Carlos Garcia Campos
no flags
Updated patch (13.41 KB, patch)
2015-11-16 02:20 PST, Carlos Garcia Campos
darin: review+
Carlos Garcia Campos
Comment 1 2015-11-11 06:57:34 PST
cgarcia 1245 1.6 2.5 1118548 416480 tty2 Sl+ 08:08 7:44 /usr/lib/xorg/Xorg vt2 -displayfd 3 -auth /run/user/1001/gdm/Xauthority -nolisten tcp -background none -noreset -keeptty -verbose 3 carlos 29750 0.0 0.2 479492 48224 tty3 Sl+ 11:27 0:02 /usr/lib/xorg/Xorg vt3 -displayfd 3 -auth /run/user/1000/gdm/Xauthority -nolisten tcp -background none -noreset -keeptty -verbose 3 this is how Xorg is run in my laptop
Carlos Garcia Campos
Comment 2 2015-11-11 07:03:01 PST
WebKit Commit Bot
Comment 3 2015-11-11 07:04:54 PST
Attachment 265284 [details] did not pass style-queue: ERROR: Tools/Scripts/webkitpy/port/xvfbdriver.py:82: multiple imports on one line [pep8/E401] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Carlos Garcia Campos
Comment 4 2015-11-12 03:23:45 PST
Created attachment 265377 [details] Fix coding style
Carlos Garcia Campos
Comment 5 2015-11-13 05:45:46 PST
ping reviewers, it's a bit annoying having to keep this patch around locally to be able to run the tests.
Zan Dobersek
Comment 6 2015-11-14 11:38:44 PST
Is running Xvfb via displayfd required or is that now supported because it can be run that way?
Carlos Garcia Campos
Comment 7 2015-11-15 23:39:10 PST
(In reply to comment #6) > Is running Xvfb via displayfd required or is that now supported because it > can be run that way? It's not required, but there's no easy way to get a list of display ids currently used when xorg itself is run with displayfd. However, using displayfd we don't need that list at all and it fixes other issues of the current model.
Zan Dobersek
Comment 8 2015-11-15 23:53:39 PST
Would it be possible to just switch to only using displayfd? How recent is this flag? Having two ways to run this isn't optimal. BUt if you add some test cases in xvfbdriver_unittest.py I guess it's not that big of a deal either.
Carlos Garcia Campos
Comment 9 2015-11-15 23:57:07 PST
(In reply to comment #8) > Would it be possible to just switch to only using displayfd? How recent is > this flag? > > Having two ways to run this isn't optimal. BUt if you add some test cases in > xvfbdriver_unittest.py I guess it's not that big of a deal either. I agree, and I thought about it, but since I was not sure (and didn't have much time) I took the safest approach. I'll check when displayfd was added.
Zan Dobersek
Comment 10 2015-11-15 23:58:51 PST
Would it be possible to just switch to only using displayfd? How recent is this flag? Having two ways to run this isn't optimal, but please at least add some test cases in xvfbdriver_unittest.py.
Carlos Garcia Campos
Comment 11 2015-11-16 02:20:55 PST
Created attachment 265579 [details] Updated patch The displayfd option is available in Xvfb since 3 years ago, so I've removed the old code and updated the unit tests.
Darin Adler
Comment 12 2015-11-17 08:18:06 PST
Comment on attachment 265579 [details] Updated patch rs=me
Carlos Garcia Campos
Comment 13 2015-11-18 01:05:26 PST
(In reply to comment #12) > Comment on attachment 265579 [details] > Updated patch > > rs=me Thanks Darin.
Carlos Garcia Campos
Comment 14 2015-11-18 01:05:40 PST
Carlos Garcia Campos
Comment 15 2015-11-18 01:28:48 PST
(In reply to comment #11) > Created attachment 265579 [details] > Updated patch > > The displayfd option is available in Xvfb since 3 years ago, so I've removed > the old code and updated the unit tests. I had forgotten that we build our own xserver, so we are not using Xvfb from the distro but from the jhbuild env. And the version we build 1.12.0 (released in March 2012) doesn't have the displayfd option. This shouldn't be a problem for EFL since they don't build xserver. So, I think it's better to just update xserver to a newer version or stop building our own.
Csaba Osztrogonác
Comment 16 2015-11-18 02:34:10 PST
(In reply to comment #14) > Committed r192568: <http://trac.webkit.org/changeset/192568> I have no idea how and why, but it broke the webkitpy test script on the Apple Mac bots.
Carlos Garcia Campos
Comment 17 2015-11-18 02:37:00 PST
(In reply to comment #16) > (In reply to comment #14) > > Committed r192568: <http://trac.webkit.org/changeset/192568> > > I have no idea how and why, but it broke the > webkitpy test script on the Apple Mac bots. Oh, I'll check that too.
Carlos Garcia Campos
Comment 18 2015-11-18 02:42:40 PST
I can reproduce it locally, I'm investigating.
Carlos Garcia Campos
Comment 19 2015-11-18 03:03:46 PST
Python tests should be fixed in r192569.
Note You need to log in before you can comment on or make changes to this bug.