Bug 112156

Summary: New context extensions restored improperly
Product: WebKit Reporter: R S <rsarvaria>
Component: WebGLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, esprehn+autocc, kbr, mkwst, ojan.autocc, rwlbuis, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

R S
Reported 2013-03-12 09:10:32 PDT
When a context is lost and then restored, a new GraphicsContext3D object is created. initializeNewContext() is run to set up the new context, but setupFlags is not run. This means that the extensions are not reloaded and therefore will not work after restoration.
Attachments
Patch (1.47 KB, patch)
2013-03-12 09:30 PDT, R S 		no flags
DetailsFormatted DiffDiff
Patch (1.47 KB, patch)
2013-03-12 10:38 PDT, R S 		no flags
DetailsFormatted DiffDiff
Patch (1.54 KB, patch)
2013-03-13 09:06 PDT, R S 		no flags
DetailsFormatted DiffDiff
Patch (1.61 KB, patch)
2013-03-15 15:00 PDT, R S 		no flags
DetailsFormatted DiffDiff
Patch (1.90 KB, patch)
2013-03-15 15:10 PDT, R S 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
R S
Comment 1 2013-03-12 09:30:30 PDT
Created attachment 192752 [details] Patch
R S
Comment 2 2013-03-12 10:38:50 PDT
Created attachment 192762 [details] Patch
Mike West
Comment 3 2013-03-13 05:12:50 PDT
Comment on attachment 192762 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192762&action=review > Source/WebCore/ChangeLog:8 > + Sets up extensions after new context is created for restoration Please add a bit more context here: why is this change necessary? > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:5825 > + setupFlags(); Does this produce any web-visible change? If so, it would be excellent to add a test to ensure that the now-correct behavior doesn't regress.
R S
Comment 4 2013-03-13 09:06:16 PDT
Created attachment 192933 [details] Patch
R S
Comment 5 2013-03-13 13:19:21 PDT
(In reply to comment #3) > Does this produce any web-visible change? If so, it would be excellent to add a test to ensure that the now-correct behavior doesn't regress. So far as I can tell it does not produce a web-visible change.
Rob Buis
Comment 6 2013-03-15 14:52:53 PDT
Comment on attachment 192933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192933&action=review According to mvujovic and achicu this change makes sense, so this just needs the text explanation before it can go in. > Source/WebCore/ChangeLog:9 > + Previous extensions set up is lost when context is initially lost. This needs a line about why the test can't be made.
R S
Comment 7 2013-03-15 15:00:12 PDT
Created attachment 193376 [details] Patch
Rob Buis
Comment 8 2013-03-15 15:06:57 PDT
Comment on attachment 193376 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193376&action=review Please explain a bit better. > Source/WebCore/ChangeLog:9 > + Previous extensions set up is lost when context is initially lost. Add empty line here. > Source/WebCore/ChangeLog:10 > + Cannot create a test as change is not visible from the web. On irc you gave a more in-depth reason, please combine with this remark.
R S
Comment 9 2013-03-15 15:10:57 PDT
Created attachment 193380 [details] Patch
Rob Buis
Comment 10 2013-03-15 15:13:51 PDT
Comment on attachment 193380 [details] Patch LGTM.
WebKit Review Bot
Comment 11 2013-03-15 15:56:42 PDT
Comment on attachment 193380 [details] Patch Clearing flags on attachment: 193380 Committed r145955: <http://trac.webkit.org/changeset/145955>
WebKit Review Bot
Comment 12 2013-03-15 15:56:46 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.