Bug 120593 - [Texmap] Reloading a webgl page doesn't work
Summary: [Texmap] Reloading a webgl page doesn't work
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Linux
: P2 Normal
Assignee: ChangSeok Oh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-02 00:54 PDT by ChangSeok Oh
Modified: 2013-09-23 12:55 PDT (History)
11 users (show)

See Also:


Attachments
Patch (3.50 KB, patch)
2013-09-05 06:21 PDT, ChangSeok Oh
no flags Details | Formatted Diff | Diff
Patch (6.19 KB, patch)
2013-09-06 11:51 PDT, ChangSeok Oh
no flags Details | Formatted Diff | Diff
Patch (6.16 KB, patch)
2013-09-09 00:54 PDT, ChangSeok Oh
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (460.90 KB, application/zip)
2013-09-09 02:41 PDT, Build Bot
no flags Details
Patch (6.12 KB, patch)
2013-09-09 07:11 PDT, ChangSeok Oh
noam: review+
noam: commit-queue-
Details | Formatted Diff | Diff
Patch (5.68 KB, patch)
2013-09-23 10:55 PDT, ChangSeok Oh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description ChangSeok Oh 2013-09-02 00:54:58 PDT
In WK2, I could not reload a webgl page such as http://learningwebgl.com/lessons/lesson01
Comment 1 ChangSeok Oh 2013-09-05 06:21:10 PDT
Created attachment 210606 [details]
Patch
Comment 2 ChangSeok Oh 2013-09-05 06:27:02 PDT
I saw this issue in WK1, WK2 Gtk port. But I guess other ports using texturemapper have the same issue.
Comment 3 Noam Rosenthal 2013-09-06 00:31:57 PDT
Comment on attachment 210606 [details]
Patch

Layout test?
Comment 4 ChangSeok Oh 2013-09-06 01:04:32 PDT
(In reply to comment #3)
> (From update of attachment 210606 [details])
> Layout test?

Oh.. is this required a new layout test even no functionality changed? 
O.K Let me prepare one.
Comment 5 ChangSeok Oh 2013-09-06 11:51:18 PDT
Created attachment 210777 [details]
Patch
Comment 6 Noam Rosenthal 2013-09-06 22:50:42 PDT
Comment on attachment 210777 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210777&action=review

> LayoutTests/fast/canvas/webgl/webgl-reload-crash.html:25
> +          setTimeout(function() { location.reload(); }, 500);

Can we make this test run a bit faster? would it still crash with a zero timeout?
Comment 7 ChangSeok Oh 2013-09-09 00:54:16 PDT
Created attachment 211016 [details]
Patch
Comment 8 ChangSeok Oh 2013-09-09 00:57:08 PDT
Comment on attachment 210777 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210777&action=review

>> LayoutTests/fast/canvas/webgl/webgl-reload-crash.html:25
>> +          setTimeout(function() { location.reload(); }, 500);
> 
> Can we make this test run a bit faster? would it still crash with a zero timeout?

Yes. The crash is still there with setTimeout(.., 0)
Comment 9 Build Bot 2013-09-09 02:41:22 PDT
Comment on attachment 211016 [details]
Patch

Attachment 211016 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1733268

New failing tests:
fast/workers/termination-with-port-messages.html
Comment 10 Build Bot 2013-09-09 02:41:24 PDT
Created attachment 211020 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.4
Comment 11 ChangSeok Oh 2013-09-09 07:05:36 PDT
(In reply to comment #9)
> (From update of attachment 211016 [details])
> Attachment 211016 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.appspot.com/results/1733268
> 
> New failing tests:
> fast/workers/termination-with-port-messages.html

I think this is a false alarm. I tested it on my mac-mountainlion.
To make sure, let me upload the patch again.
Comment 12 ChangSeok Oh 2013-09-09 07:11:14 PDT
Created attachment 211039 [details]
Patch
Comment 13 ChangSeok Oh 2013-09-13 01:51:24 PDT
(In reply to comment #12)
> Created an attachment (id=211039) [details]
> Patch

Noam, review please?
Comment 14 Noam Rosenthal 2013-09-23 08:07:53 PDT
Comment on attachment 211039 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=211039&action=review

> Source/WebCore/ChangeLog:14
> +        m_contentLayer of GraphicsLayerTextureMapper could be a dangling pointer while destroying
> +        Document. GraphicsContextPrivate is destroyed before GraphicsLayerTextureMapper.
> +        So using its method such as setClient could be invalid in destructor of
> +        GraphicsLayerTextureMapper. I add platformLayerWillBeDestroyed to TextureMapperPlatformLayer::Client
> +        to let the client know that the m_contentLayer will be destroyed so that the client make
> +        its contentLayer set 0. With this way, we can avoid for m_contentsLayer to be
> +        a dangling pointer.

Change to:
Let GraphicsLayerTextureMapper know it needs to detach the platform layer when a GraphicsContext3D is destroyed.
Comment 15 ChangSeok Oh 2013-09-23 10:55:51 PDT
Created attachment 212367 [details]
Patch
Comment 16 WebKit Commit Bot 2013-09-23 12:01:36 PDT
Comment on attachment 212367 [details]
Patch

Clearing flags on attachment: 212367

Committed r156282: <http://trac.webkit.org/changeset/156282>