Move Context::sync to Display::sync.

Move Context::sync to Display::sync.

This is a workaround for an Intel bug.

The crash looks like this:

0x5f816c53	 [d3d9.dll	 + 0x00036c53]	CQuery::~CQuery()
0x5f816bec	 [d3d9.dll	 + 0x00036bec]	CQuery::`vector deleting
destructor'(unsigned int)
0x5f7e8129	 [d3d9.dll	 + 0x00008129]	CBaseObject::~CBaseObject()
0x5f9e19c7	 [libglesv2.dll	 + 0x000319c7]	gl::Fence::`vector deleting
destructor'(unsigned int)
0x5f9de78e	 [libglesv2.dll	 - context.cpp:975]	gl::Context::deleteFence(unsigned
int)
0x5f9e1491	 [libglesv2.dll	 - context.cpp:198]	gl::Context::~Context()
0x5f9e182f	 [libglesv2.dll	 - context.cpp:3936]	glDestroyContext
0x717654ec	 [libegl.dll	 -
display.cpp:749]	egl::Display::destroyContext(gl::Context *)
0x7176a3da	 [libegl.dll	 - libegl.cpp:907]	eglDestroyContext
0x64fbaf33	 [chrome.dll	 - gl_context_egl.cc:75]	gfx::GLContextEGL::Destroy()

This is a crash in deleteFence but the same could happen in flush. Fix for the former to follow.

The vendor ID is always 8086 (Intel). Not an XP issue - it's happening on Win
7.

Tested by stepping through a call to glFinish in the debugger and ensuring that
it sleeps but eventually returns.
Committed: http://code.google.com/p/angleproject/source/detail?r=940

[apatrick1, 2012-01-03 21:50:57]

Hi Daniel,

Please review.

Thanks,

Al

[dgkoch, 2012-01-05 17:30:15]

I wonder if this is similar to the bug the Firefox guys ran into:?
http://code.google.com/p/angleproject/issues/detail?id=232

http://codereview.appspot.com/5503094/diff/1/src/libEGL/Display.cpp
File src/libEGL/Display.cpp (right):

http://codereview.appspot.com/5503094/diff/1/src/libEGL/Display.cpp#newcode78
src/libEGL/Display.cpp:78: mQuery = NULL;
I'd prefer if this was called mEventQuery

http://codereview.appspot.com/5503094/diff/1/src/libEGL/Display.cpp#newcode461
src/libEGL/Display.cpp:461: if (result == D3DERR_OUTOFVIDEOMEMORY || result ==
E_OUTOFMEMORY || result == D3DERR_NOTAVAILABLE)
FYI  - this is a material change from before. This means that on ANGLE will not
run on drivers that don't expose EVENT queries, whereas previously it would run,
but wouldn't be able to implement flush/finish properly.  The only card I'm
aware of that may fall into this category is the Intel GMA 900 (i910/i915
chipset).  This isn't necessarily a bad thing since I wouldn't really have high
expectations of those cards, but it's something to be aware of, in case those
are currently not-blacklisted.

http://codereview.appspot.com/5503094/diff/1/src/libEGL/Display.cpp#newcode893
src/libEGL/Display.cpp:893: // Keep polling, but allow other threads to do
something useful first
This is missing the device loss check.  See Issue 262.

http://codereview.appspot.com/5503094/diff/1/src/libEGL/Display.cpp#newcode894
src/libEGL/Display.cpp:894: Sleep(0);
This is missing a device loss check.  See issue 262

http://codereview.appspot.com/5503094/diff/1/src/libEGL/Display.cpp#newcode898
src/libEGL/Display.cpp:898: 
It may make more sense to keep the device loss check in this function, and
either make the return bool or void.

http://codereview.appspot.com/5503094/diff/1/src/libGLESv2/Context.cpp
File src/libGLESv2/Context.cpp (left):

http://codereview.appspot.com/5503094/diff/1/src/libGLESv2/Context.cpp#oldcod...
src/libGLESv2/Context.cpp:2855: Sleep(0);
This must be old source - it's missing the fix from r924.

[apatrick1, 2012-01-05 20:29:25]

Everything changed! Rebased and addressed issues.

I tested by calling glFinish from one of the samples and simulating lost context
in sync() and failure to create mEventQuery.

I don't think this is related to the issue mozilla is having, though we have
similar crashes.

[dgkoch, 2012-01-10 14:58:45]

http://codereview.appspot.com/5503094/diff/5001/src/libEGL/Display.cpp
File src/libEGL/Display.cpp (right):

http://codereview.appspot.com/5503094/diff/5001/src/libEGL/Display.cpp#newcod...
src/libEGL/Display.cpp:474: mDevice->CreateQuery(D3DQUERYTYPE_EVENT,
&mEventQuery);
initializeDevice will get called after a device reset so this could end up
leaking the previous event query. (I know you handle it in resetDevice, but I'd
feel better if there was also a check here, just in case something gets moved
around at a later point).

http://codereview.appspot.com/5503094/diff/5001/src/libEGL/Display.cpp#newcod...
src/libEGL/Display.cpp:920: switch (result)
can't you use isDeviceLostError(result) here?  We use it in Surface.cpp.

[apatrick1, 2012-01-10 19:06:14]

Done.

[dgkoch, 2012-01-10 21:54:39]

LGTM

(might want to scrub the description somewhat before committing though)