Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(164)

Issue 1151163002: Oilpan: eagerly finalize WebGLRenderingContext objects. (Closed)

Created:
5 years, 7 months ago by sof
Modified:
5 years, 6 months ago
Reviewers:
oilpan-reviews, haraken
CC:
blink-reviews, dshwang, blink-reviews-html_chromium.org, dglazkov+blink, Rik, Justin Novosad
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Oilpan: eagerly finalize WebGLRenderingContext objects. To simplify the destruction of WebGLRenderingContextBase-derived objects along with their WebGLContextObjects, insist that the rendering context is finalized first. It may then detach itself from the context objects, before their finalizers are later run. Such a finalization ordering avoids the need for the ref-counted WebGLSharedWebGraphics3D abstraction which supported arbitrary finalization orders. R=haraken BUG=482838 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196437

Patch Set 1 #

Total comments: 7

Patch Set 2 : eagerly finalize and simplify instead #

Patch Set 3 : add explanatory comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+6 lines, -171 lines) Patch
M Source/core/core.gypi View 1 2 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/html/canvas/WebGL2RenderingContextBase.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/html/canvas/WebGL2RenderingContextBase.cpp View 1 2 chunks +0 lines, -2 lines 0 comments Download
M Source/core/html/canvas/WebGLContextObject.h View 1 2 chunks +0 lines, -4 lines 0 comments Download
M Source/core/html/canvas/WebGLContextObject.cpp View 1 2 chunks +0 lines, -11 lines 0 comments Download
M Source/core/html/canvas/WebGLRenderingContext.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/html/canvas/WebGLRenderingContextBase.h View 1 2 4 chunks +4 lines, -9 lines 0 comments Download
M Source/core/html/canvas/WebGLRenderingContextBase.cpp View 1 2 9 chunks +0 lines, -41 lines 0 comments Download
D Source/core/html/canvas/WebGLSharedWebGraphicsContext3D.h View 1 1 chunk +0 lines, -48 lines 0 comments Download
D Source/core/html/canvas/WebGLSharedWebGraphicsContext3D.cpp View 1 1 chunk +0 lines, -54 lines 0 comments Download

Messages

Total messages: 28 (4 generated)
sof
please take a look.
5 years, 7 months ago (2015-05-21 21:03:18 UTC) #2
haraken
Thanks for working on this! It seems I need some more time to understand the ...
5 years, 7 months ago (2015-05-22 01:09:48 UTC) #3
haraken
On 2015/05/22 01:09:48, haraken wrote: > Thanks for working on this! > > It seems ...
5 years, 7 months ago (2015-05-22 01:10:48 UTC) #4
sof
On 2015/05/22 01:10:48, haraken wrote: > On 2015/05/22 01:09:48, haraken wrote: > > Thanks for ...
5 years, 7 months ago (2015-05-22 05:10:47 UTC) #5
haraken
(Will take a look at this within today.)
5 years, 7 months ago (2015-05-26 05:15:32 UTC) #6
haraken
Sorry about delayed review. Now I understand how the destruction of the WebGL objects are ...
5 years, 7 months ago (2015-05-26 11:23:10 UTC) #8
sof
https://codereview.chromium.org/1151163002/diff/1/Source/core/html/canvas/WebGLContextGroup.cpp File Source/core/html/canvas/WebGLContextGroup.cpp (right): https://codereview.chromium.org/1151163002/diff/1/Source/core/html/canvas/WebGLContextGroup.cpp#newcode86 Source/core/html/canvas/WebGLContextGroup.cpp:86: if (!Heap::willObjectBeLazilySwept(context)) On 2015/05/26 11:23:10, haraken wrote: > > ...
5 years, 7 months ago (2015-05-26 11:29:47 UTC) #9
haraken
https://codereview.chromium.org/1151163002/diff/1/Source/core/html/canvas/WebGLContextGroup.cpp File Source/core/html/canvas/WebGLContextGroup.cpp (right): https://codereview.chromium.org/1151163002/diff/1/Source/core/html/canvas/WebGLContextGroup.cpp#newcode86 Source/core/html/canvas/WebGLContextGroup.cpp:86: if (!Heap::willObjectBeLazilySwept(context)) On 2015/05/26 11:29:47, sof wrote: > On ...
5 years, 7 months ago (2015-05-26 11:36:08 UTC) #10
haraken
static bool willObjectBeLazilySwept() { BasePage* page = pageFromObject(objectPointer); if (page->hasBeenSwept()) return false; return !ObjectAliveTrait<T>::isHeapObjectAlive(s_markingVisitor, const_cast<T*>(objectPointer)); ...
5 years, 7 months ago (2015-05-26 11:40:49 UTC) #11
sof
On 2015/05/26 11:40:49, haraken wrote: > static bool willObjectBeLazilySwept() { > BasePage* page = pageFromObject(objectPointer); ...
5 years, 7 months ago (2015-05-26 11:47:17 UTC) #12
haraken
On 2015/05/26 11:47:17, sof wrote: > On 2015/05/26 11:40:49, haraken wrote: > > static bool ...
5 years, 7 months ago (2015-05-26 12:00:31 UTC) #13
sof
On 2015/05/26 12:00:31, haraken wrote: > On 2015/05/26 11:47:17, sof wrote: > > On 2015/05/26 ...
5 years, 7 months ago (2015-05-26 12:13:41 UTC) #14
haraken
On 2015/05/26 12:13:41, sof wrote: > On 2015/05/26 12:00:31, haraken wrote: > > On 2015/05/26 ...
5 years, 7 months ago (2015-05-26 12:31:21 UTC) #15
sof
On 2015/05/26 12:31:21, haraken wrote: > On 2015/05/26 12:13:41, sof wrote: > > On 2015/05/26 ...
5 years, 7 months ago (2015-05-26 12:35:25 UTC) #16
haraken
https://codereview.chromium.org/1151163002/diff/1/Source/core/html/canvas/WebGLSharedObject.cpp File Source/core/html/canvas/WebGLSharedObject.cpp (right): https://codereview.chromium.org/1151163002/diff/1/Source/core/html/canvas/WebGLSharedObject.cpp#newcode51 Source/core/html/canvas/WebGLSharedObject.cpp:51: deleteObject(nullptr); On 2015/05/26 11:23:10, haraken wrote: > > This ...
5 years, 7 months ago (2015-05-26 12:50:12 UTC) #17
haraken
Also I noticed that Timers won't hit the problem (by accident) because Timer::fired() should not ...
5 years, 7 months ago (2015-05-26 14:57:47 UTC) #18
haraken
On 2015/05/26 14:57:47, haraken wrote: > Also I noticed that Timers won't hit the problem ...
5 years, 7 months ago (2015-05-26 15:00:26 UTC) #19
sof
On 2015/05/26 12:50:12, haraken wrote: > https://codereview.chromium.org/1151163002/diff/1/Source/core/html/canvas/WebGLSharedObject.cpp > File Source/core/html/canvas/WebGLSharedObject.cpp (right): > > https://codereview.chromium.org/1151163002/diff/1/Source/core/html/canvas/WebGLSharedObject.cpp#newcode51 > ...
5 years, 7 months ago (2015-05-26 18:30:54 UTC) #20
sof
On 2015/05/26 18:30:54, sof wrote: > On 2015/05/26 12:50:12, haraken wrote: > > > https://codereview.chromium.org/1151163002/diff/1/Source/core/html/canvas/WebGLSharedObject.cpp ...
5 years, 6 months ago (2015-06-01 18:43:51 UTC) #21
sof
On 2015/06/01 18:43:51, sof wrote: > On 2015/05/26 18:30:54, sof wrote: > > On 2015/05/26 ...
5 years, 6 months ago (2015-06-03 14:27:01 UTC) #22
haraken
LGTM It would be helpful to add a brief comment about why it needs to ...
5 years, 6 months ago (2015-06-03 14:31:29 UTC) #23
sof
On 2015/06/03 14:31:29, haraken wrote: > LGTM > > It would be helpful to add ...
5 years, 6 months ago (2015-06-03 20:48:38 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1151163002/40001
5 years, 6 months ago (2015-06-03 20:50:13 UTC) #27
commit-bot: I haz the power
5 years, 6 months ago (2015-06-03 22:23:54 UTC) #28
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196437

Powered by Google App Engine
This is Rietveld 408576698