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

Issue 1479673003: Verify resource provider sync tokens before sending to parent. (Closed)

Created:
5 years ago by David Yen
Modified:
4 years, 11 months ago
Reviewers:
piman
CC:
chromium-reviews, cc-bugs_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Verify resource provider sync tokens before sending to parent. R=piman@chromium.org BUG=514815 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/882d69d97b5c43b7579bc610722dbadd328ac63e Cr-Commit-Position: refs/heads/master@{#367727}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Verify new sync token in mailbox holder #

Patch Set 3 : Insert sync token after transfer all resources again #

Patch Set 4 : rebase #

Patch Set 5 : Fix verify sync token chromium order #

Patch Set 6 : No longer insert dummy fence syncs, just use EnsureWorkVisible() #

Patch Set 7 : Fixed merge error #

Patch Set 8 : Fixed unit test so it doesn't expect dummy fence sync #

Unified diffs Side-by-side diffs Delta from patch set Stats (+239 lines, -59 lines) Patch
M cc/resources/resource_provider.cc View 1 2 3 4 5 2 chunks +24 lines, -2 lines 0 comments Download
M cc/resources/resource_provider_unittest.cc View 1 2 3 4 5 6 29 chunks +77 lines, -33 lines 0 comments Download
M cc/test/test_gles2_interface.h View 1 2 1 chunk +5 lines, -1 line 0 comments Download
M cc/test/test_gles2_interface.cc View 1 2 1 chunk +19 lines, -0 lines 0 comments Download
M cc/test/test_web_graphics_context_3d.h View 1 2 3 4 5 2 chunks +5 lines, -2 lines 0 comments Download
M cc/test/test_web_graphics_context_3d.cc View 1 2 3 4 5 3 chunks +25 lines, -3 lines 0 comments Download
M gpu/command_buffer/client/gles2_implementation.cc View 1 2 3 4 5 2 chunks +20 lines, -12 lines 0 comments Download
M gpu/command_buffer/client/gles2_implementation_unittest.cc View 1 2 3 4 5 6 7 3 chunks +64 lines, -6 lines 0 comments Download

Messages

Total messages: 35 (11 generated)
David Yen
5 years ago (2015-11-26 01:04:48 UTC) #1
piman
https://codereview.chromium.org/1479673003/diff/1/cc/resources/resource_provider.cc File cc/resources/resource_provider.cc (right): https://codereview.chromium.org/1479673003/diff/1/cc/resources/resource_provider.cc#newcode1152 cc/resources/resource_provider.cc:1152: unverified_sync_tokens.push_back(new_sync_token.GetData()); Actually, the VerifySyncTokensCHROMIUM below will tag the local ...
5 years ago (2015-11-26 02:32:44 UTC) #3
David Yen
https://codereview.chromium.org/1479673003/diff/1/cc/resources/resource_provider.cc File cc/resources/resource_provider.cc (right): https://codereview.chromium.org/1479673003/diff/1/cc/resources/resource_provider.cc#newcode1152 cc/resources/resource_provider.cc:1152: unverified_sync_tokens.push_back(new_sync_token.GetData()); On 2015/11/26 02:32:44, piman (slow to review) wrote: ...
5 years ago (2015-12-08 00:57:56 UTC) #4
piman
lgtm
5 years ago (2015-12-08 02:27:33 UTC) #5
David Yen
On 2015/12/08 02:27:33, piman (OOO back 12-11) wrote: > lgtm In fixing the unit tests, ...
5 years ago (2015-12-09 23:21:37 UTC) #6
piman
Good point. LGTM, and thanks for the test.
5 years ago (2015-12-11 06:34:17 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1479673003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1479673003/60001
5 years ago (2015-12-16 01:08:04 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_dbg_recipe on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) cast_shell_android on ...
5 years ago (2015-12-16 02:48:30 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1479673003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1479673003/60001
5 years ago (2015-12-16 06:03:06 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/156917)
5 years ago (2015-12-16 07:04:43 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1479673003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1479673003/60001
5 years ago (2015-12-16 17:13:28 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/157105)
5 years ago (2015-12-16 18:08:56 UTC) #20
David Yen
On 2015/12/16 18:08:56, commit-bot: I haz the power wrote: > Try jobs failed on following ...
5 years ago (2015-12-16 21:13:04 UTC) #21
piman
On Thu, Dec 17, 2015 at 6:13 AM, <dyen@chromium.org> wrote: > On 2015/12/16 18:08:56, commit-bot: ...
5 years ago (2015-12-17 06:16:39 UTC) #22
David Yen
On 2015/12/17 06:16:39, piman (in KST timezone) wrote: > On Thu, Dec 17, 2015 at ...
5 years ago (2015-12-17 06:31:09 UTC) #23
piman
On Thu, Dec 17, 2015 at 3:31 PM, <dyen@chromium.org> wrote: > On 2015/12/17 06:16:39, piman ...
5 years ago (2015-12-17 06:44:47 UTC) #24
David Yen
On 2015/12/17 06:44:47, piman (in KST timezone) wrote: > On Thu, Dec 17, 2015 at ...
5 years ago (2015-12-17 06:54:30 UTC) #25
piman
On Thu, Dec 17, 2015 at 3:44 PM, Antoine Labour <piman@chromium.org> wrote: > > > ...
5 years ago (2015-12-17 06:57:00 UTC) #26
David Yen
On 2015/12/17 06:57:00, piman (in KST timezone) wrote: > On Thu, Dec 17, 2015 at ...
5 years ago (2015-12-17 15:40:23 UTC) #27
David Yen
On 2015/12/17 15:40:23, David Yen wrote: > On 2015/12/17 06:57:00, piman (in KST timezone) wrote: ...
4 years, 11 months ago (2016-01-05 21:17:30 UTC) #28
piman
lgtm
4 years, 11 months ago (2016-01-05 22:39:41 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1479673003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1479673003/140001
4 years, 11 months ago (2016-01-05 22:51:49 UTC) #32
commit-bot: I haz the power
Committed patchset #8 (id:140001)
4 years, 11 months ago (2016-01-06 00:50:24 UTC) #33
commit-bot: I haz the power
4 years, 11 months ago (2016-01-06 00:52:18 UTC) #35
Message was sent while issue was closed.
Patchset 8 (id:??) landed as
https://crrev.com/882d69d97b5c43b7579bc610722dbadd328ac63e
Cr-Commit-Position: refs/heads/master@{#367727}

Powered by Google App Engine
This is Rietveld 408576698