Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2469413002:
gpu shader cache: Make ShaderClearHelper non-refcounted. (Closed)
Descriptiongpu shader cache: Make ShaderClearHelper non-refcounted.
Make ShaderCacheFactory own the ShaderClearHelper objects. The only
use of the ref-count was for the helper object deal with the factory
destroying it. But the helper objects knows when that happens, so make
it deal with that explicitly.
BUG=643746
Committed: https://crrev.com/39292a014665bbbb39ed7f8cc2b7a5423d2c5696
Cr-Commit-Position: refs/heads/master@{#429761}
Patch Set 1 #Patch Set 2 : . #Patch Set 3 : tot merge #Patch Set 4 : . #
Total comments: 2
Patch Set 5 : update map before Clear() #Messages
Total messages: 28 (21 generated)
The CQ bit was checked by sadrul@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by sadrul@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by sadrul@chromium.org
Description was changed from ========== gpu shader cache: ... BUG=... ========== to ========== gpu shader cache: Make ShaderClearHelper non-refcounted. Make ShaderCacheFactory own the ShaderClearHelper objects. The only use of the ref-count was for the helper object deal with the factory destroying it. But the helper objects knows when that happens, so make it deal with that explicitly. BUG=643746 ==========
The CQ bit was checked by sadrul@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== gpu shader cache: Make ShaderClearHelper non-refcounted. Make ShaderCacheFactory own the ShaderClearHelper objects. The only use of the ref-count was for the helper object deal with the factory destroying it. But the helper objects knows when that happens, so make it deal with that explicitly. BUG=643746 ========== to ========== gpu shader cache: Make ShaderClearHelper non-refcounted. Make ShaderCacheFactory own the ShaderClearHelper objects. The only use of the ref-count was for the helper object deal with the factory destroying it. But the helper objects knows when that happens, so make it deal with that explicitly. BUG=643746 ==========
sadrul@chromium.org changed reviewers: + piman@chromium.org
https://codereview.chromium.org/2469413002/diff/60001/content/browser/gpu/sha... File content/browser/gpu/shader_disk_cache.cc (right): https://codereview.chromium.org/2469413002/diff/60001/content/browser/gpu/sha... content/browser/gpu/shader_disk_cache.cc:471: helper->Clear(); Is it possible that Clear() will reenter CacheCleared() ? It seems like the code allows it. If so, we want to keep it after adding the helper to the map.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by sadrul@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2469413002/diff/60001/content/browser/gpu/sha... File content/browser/gpu/shader_disk_cache.cc (right): https://codereview.chromium.org/2469413002/diff/60001/content/browser/gpu/sha... content/browser/gpu/shader_disk_cache.cc:471: helper->Clear(); On 2016/11/03 18:01:38, piman wrote: > Is it possible that Clear() will reenter CacheCleared() ? It seems like the code > allows it. If so, we want to keep it after adding the helper to the map. Oh nice! You are right. Done. Thanks!
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by sadrul@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== gpu shader cache: Make ShaderClearHelper non-refcounted. Make ShaderCacheFactory own the ShaderClearHelper objects. The only use of the ref-count was for the helper object deal with the factory destroying it. But the helper objects knows when that happens, so make it deal with that explicitly. BUG=643746 ========== to ========== gpu shader cache: Make ShaderClearHelper non-refcounted. Make ShaderCacheFactory own the ShaderClearHelper objects. The only use of the ref-count was for the helper object deal with the factory destroying it. But the helper objects knows when that happens, so make it deal with that explicitly. BUG=643746 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== gpu shader cache: Make ShaderClearHelper non-refcounted. Make ShaderCacheFactory own the ShaderClearHelper objects. The only use of the ref-count was for the helper object deal with the factory destroying it. But the helper objects knows when that happens, so make it deal with that explicitly. BUG=643746 ========== to ========== gpu shader cache: Make ShaderClearHelper non-refcounted. Make ShaderCacheFactory own the ShaderClearHelper objects. The only use of the ref-count was for the helper object deal with the factory destroying it. But the helper objects knows when that happens, so make it deal with that explicitly. BUG=643746 Committed: https://crrev.com/39292a014665bbbb39ed7f8cc2b7a5423d2c5696 Cr-Commit-Position: refs/heads/master@{#429761} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/39292a014665bbbb39ed7f8cc2b7a5423d2c5696 Cr-Commit-Position: refs/heads/master@{#429761} |
