|
|
Created:
3 years, 6 months ago by Saman Sami Modified:
3 years, 6 months ago CC:
blink-reviews, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, dshwang, drott+blinkwatch_chromium.org, krit, fmalita+watch_chromium.org, kinuko+watch, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionIgnore calls to SatisfySequence if CanvasSurfaceLayerBridge is gone
Since SatisfySequence is post-tasked, under certain race conditions it's
possible that CanvasSurfaceLayerBridge is gone when it's called. Simply
ignore the call in that case. Note that RequireSequence is not PostTasked
so I expect bridge_ to exist.
BUG=720188
Review-Url: https://codereview.chromium.org/2911643004
Cr-Commit-Position: refs/heads/master@{#475378}
Committed: https://chromium.googlesource.com/chromium/src/+/d5bf6c6f5651782d2fe6db6194d7e9776e2aff35
Patch Set 1 #
Total comments: 2
Messages
Total messages: 20 (13 generated)
Description was changed from ========== Ignore calls to SatisfySequence in OffscreenCanvasSurfaceReferenceFactory if CanvasSurfaceLayerBridge is gone BUG=720188 ========== to ========== Ignore calls to SatisfySequence if CanvasSurfaceLayerBridge is gone BUG=720188 ==========
The CQ bit was checked by samans@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 ========== Ignore calls to SatisfySequence if CanvasSurfaceLayerBridge is gone BUG=720188 ========== to ========== Ignore calls to SatisfySequence if CanvasSurfaceLayerBridge is gone Since SatisfySequence is post-tasked, under certain race conditions it's possible that CanvasSurfaceLayerBridge is gone when it's called. Simply ignore the call in that case. Note that RequireSequence is not PostTasked so I expect bridge_ to exist. BUG=720188 ==========
samans@chromium.org changed reviewers: + jbroman@chromium.org
Description was changed from ========== Ignore calls to SatisfySequence if CanvasSurfaceLayerBridge is gone Since SatisfySequence is post-tasked, under certain race conditions it's possible that CanvasSurfaceLayerBridge is gone when it's called. Simply ignore the call in that case. Note that RequireSequence is not PostTasked so I expect bridge_ to exist. BUG=720188 ========== to ========== Ignore calls to SatisfySequence if CanvasSurfaceLayerBridge is gone Since SatisfySequence is post-tasked, under certain race conditions it's possible that CanvasSurfaceLayerBridge is gone when it's called. Simply ignore the call in that case. Note that RequireSequence is not PostTasked so I expect bridge_ to exist. BUG=720188 ==========
fsamuel@chromium.org changed reviewers: + fsamuel@chromium.org
https://codereview.chromium.org/2911643004/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp (right): https://codereview.chromium.org/2911643004/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:48: bridge_->SatisfyCallback(sequence); maybe do this for require too?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
jbroman@chromium.org changed reviewers: + junov@chromium.org
This lgtm, but junov is probably better-acquainted with the expected lifetime of the bridge.
This makes total sense IMHO. lgtm
Thanks for the lgtms. https://codereview.chromium.org/2911643004/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp (right): https://codereview.chromium.org/2911643004/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/graphics/CanvasSurfaceLayerBridge.cpp:48: bridge_->SatisfyCallback(sequence); On 2017/05/26 20:47:57, Fady Samuel wrote: > maybe do this for require too? I expect bridge_ to exist in RequireSequence. SatisfySequence is PostTasked so lifetimes get messed up.
The CQ bit was checked by samans@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 1, "attempt_start_ts": 1496080108915270, "parent_rev": "dbb248a23fbf31019a4612268577b0b2371b2aa4", "commit_rev": "d5bf6c6f5651782d2fe6db6194d7e9776e2aff35"}
Message was sent while issue was closed.
Description was changed from ========== Ignore calls to SatisfySequence if CanvasSurfaceLayerBridge is gone Since SatisfySequence is post-tasked, under certain race conditions it's possible that CanvasSurfaceLayerBridge is gone when it's called. Simply ignore the call in that case. Note that RequireSequence is not PostTasked so I expect bridge_ to exist. BUG=720188 ========== to ========== Ignore calls to SatisfySequence if CanvasSurfaceLayerBridge is gone Since SatisfySequence is post-tasked, under certain race conditions it's possible that CanvasSurfaceLayerBridge is gone when it's called. Simply ignore the call in that case. Note that RequireSequence is not PostTasked so I expect bridge_ to exist. BUG=720188 Review-Url: https://codereview.chromium.org/2911643004 Cr-Commit-Position: refs/heads/master@{#475378} Committed: https://chromium.googlesource.com/chromium/src/+/d5bf6c6f5651782d2fe6db6194d7... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/d5bf6c6f5651782d2fe6db6194d7... |