Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
476193002:
gpu: ContextProvider::BindToCurrentThread() should be called one time. (Closed)
Descriptiongpu: ContextProvider::BindToCurrentThread() should be called one time.
BUG=404121
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290152
Patch Set 1 #
Total comments: 5
Messages
Total messages: 12 (0 generated)
jamesr@, could you review? This change is extracted from https://codereview.chromium.org/470973002/
Calling it more than once should have no effect, so there is not logic change. This CL is just for readability. https://code.google.com/p/chromium/codesearch#chromium/src/cc/output/context_...
https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... File content/renderer/render_thread_impl.cc (right): https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... content/renderer/render_thread_impl.cc:1180: if (shared_main_thread_contexts_ && it's never null here right?
https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... File content/renderer/render_thread_impl.cc (right): https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... content/renderer/render_thread_impl.cc:1180: if (shared_main_thread_contexts_ && On 2014/08/15 16:23:54, danakj wrote: > it's never null here right? No, above factory method can return NULL.
https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... File content/renderer/render_thread_impl.cc (right): https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... content/renderer/render_thread_impl.cc:1180: if (shared_main_thread_contexts_ && On 2014/08/15 16:50:16, dshwang wrote: > On 2014/08/15 16:23:54, danakj wrote: > > it's never null here right? > > No, above factory method can return NULL. But ContextProviderCommandBuffer::Create won't if the context it's given is not null. And CreateOffscreenContext3d can't return NULL. I think. Do you agree?
https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... File content/renderer/render_thread_impl.cc (right): https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... content/renderer/render_thread_impl.cc:1180: if (shared_main_thread_contexts_ && On 2014/08/15 16:52:35, danakj wrote: > On 2014/08/15 16:50:16, dshwang wrote: > > On 2014/08/15 16:23:54, danakj wrote: > > > it's never null here right? > > > > No, above factory method can return NULL. > > But ContextProviderCommandBuffer::Create won't if the context it's given is not > null. And CreateOffscreenContext3d can't return NULL. I think. Do you agree? IMO CreateOffscreenContext3d() can return NULL. For example, it s called during gpu process down. https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r...
https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... File content/renderer/render_thread_impl.cc (right): https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... content/renderer/render_thread_impl.cc:1180: if (shared_main_thread_contexts_ && On 2014/08/15 17:28:39, dshwang wrote: > On 2014/08/15 16:52:35, danakj wrote: > > On 2014/08/15 16:50:16, dshwang wrote: > > > On 2014/08/15 16:23:54, danakj wrote: > > > > it's never null here right? > > > > > > No, above factory method can return NULL. > > > > But ContextProviderCommandBuffer::Create won't if the context it's given is > not > > null. And CreateOffscreenContext3d can't return NULL. I think. Do you agree? > > IMO CreateOffscreenContext3d() can return NULL. For example, it > s called during gpu process down. > https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r... Ah okay ya, I had to go one deeper. LGTM
On 2014/08/15 17:30:43, danakj wrote: > https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... > File content/renderer/render_thread_impl.cc (right): > > https://codereview.chromium.org/476193002/diff/1/content/renderer/render_thre... > content/renderer/render_thread_impl.cc:1180: if (shared_main_thread_contexts_ && > On 2014/08/15 17:28:39, dshwang wrote: > > On 2014/08/15 16:52:35, danakj wrote: > > > On 2014/08/15 16:50:16, dshwang wrote: > > > > On 2014/08/15 16:23:54, danakj wrote: > > > > > it's never null here right? > > > > > > > > No, above factory method can return NULL. > > > > > > But ContextProviderCommandBuffer::Create won't if the context it's given is > > not > > > null. And CreateOffscreenContext3d can't return NULL. I think. Do you agree? > > > > IMO CreateOffscreenContext3d() can return NULL. For example, it > > s called during gpu process down. > > > https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r... > > Ah okay ya, I had to go one deeper. LGTM Thank you for lgtm! jamesr@, could you review as owner?
lgtm
The CQ bit was checked by dongseong.hwang@intel.com
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/476193002/1
Message was sent while issue was closed.
Committed patchset #1 (1) as 290152 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
