Closed
Bug 998821
Opened 10 years ago
Closed 10 years ago
Handle XShmCreateImage failure without crashing
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: roc, Assigned: roc)
Details
Attachments
(1 file, 1 obsolete file)
871 bytes,
patch
|
karlt
:
review+
|
Details | Diff | Splinter Review |
This is a little cleaner, avoids doing some unnecessary work if XShm is not available, and makes FF work with rr when usage of XRender is disabled.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8409496 -
Flags: review?(karlt)
Comment 2•10 years ago
|
||
Comment on attachment 8409496 [details] [diff] [review] fix > if (!attachOk || xerror) { >- // Assume XShm isn't available, and don't attempt to use it >- // again. >- gShmAvailable = false; The presence of the MIT-SHM extension in the server is a necessary condition to use XShm, but not sufficient, so please leave this code. (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #0) > This is a little cleaner, avoids doing some unnecessary work if XShm is not > available, Skips some work, yes, but adds a round trip to the server. > and makes FF work with rr when usage of XRender is disabled. It's reasonable to add code for the sake of rr, but please add a comment explaining why it is there. This could also be fixed in rr, by overriding XInitExtension() or XQueryExtension() instead of XShmQueryExtension() so that all XShm entry points see that MIT-SHM is not available.
Attachment #8409496 -
Flags: review?(karlt) → review-
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Karl Tomlinson (:karlt) from comment #2) > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #0) > > This is a little cleaner, avoids doing some unnecessary work if XShm is not > > available, > > Skips some work, yes, but adds a round trip to the server. Oh it does? In that case I shouldn't do this, then. > This could also be fixed in rr, by overriding XInitExtension() or > XQueryExtension() instead of XShmQueryExtension() so that all XShm entry > points see that MIT-SHM is not available. OK, I'll try that instead. Thanks!
Comment 4•10 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3) > > Skips some work, yes, but adds a round trip to the server. > > Oh it does? Sorry, I checked this out. XShmQueryExtension() will do a round trip the first time it is called, but XShmAttach() will do this additional round trip if XShmQueryExtension() has not been called, so adding XShmQueryExtension() makes no difference to the number of round trips. Go ahead with this approach if you like.
Assignee | ||
Comment 5•10 years ago
|
||
Actually I think I'll just override XShmCreateImage in rr as well, making it return null.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
Assignee | ||
Comment 6•10 years ago
|
||
Actually, let me repurpose this bug...
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Summary: Check for XShm extension before calling XShmCreateImage → Handle XShmCreateImage failure without crashing
Assignee | ||
Comment 7•10 years ago
|
||
Attachment #8409496 -
Attachment is obsolete: true
Attachment #8409999 -
Flags: review?(karlt)
Updated•10 years ago
|
Attachment #8409999 -
Flags: review?(karlt) → review+
Assignee | ||
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/229386171230
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/229386171230
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•