@piman: I think that RasterDecoder will process a contiguous chunk of commands that come through on a CommandBufferService::Flush.

Can we have BeginRaster/EndRaster straddle a flush boundary? Both in a common case or in the case of a compromised renderer. If so that may change what we can do here (e.g. more closely integrate the state trackers).

It is possible to have a context switch between Begin/EndRaster. Possibilities are:
1- out-of-space in the command buffer (or the transfer buffer) requiring the service side to make progress. CommandBufferHelper (or TransferBuffer) will cause a flush there
2- preemption by the scheduler, the command buffer infra only processes 20 commands at a time, and checks if a higher-priority context needs to run, and if so context-switches.
3- a malicious client can do whatever it wants, we have to handle it sanely, though things like losing the context in cases we don't want to handle is perfectly reasonable.

None of those are common, so we can fall back to a slow "do full reset" if that happens.

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/1480be07c40000

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/1480be07c40000

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/149248dbc40000

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/14eb47a7c40000

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/149248dbc40000

All of the attempts failed. See the individual attempts for details on each error.

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/16b08ae7c40000

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/14eb47a7c40000

All of the attempts failed. See the individual attempts for details on each error.

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/14a8163bc40000

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/16b08ae7c40000

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/14b038dbc40000

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14a8163bc40000

📍 Job complete. See results below.
https://pinpoint-dot-chromeperf.appspot.com/job/14b038dbc40000

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/16a0a067c40000

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/16a0a067c40000

Buildbucket says the build completed successfully, but Pinpoint can't find the isolate hash.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4fb9f5acb4a3b94258689fdae0a39eaa12c092fc

commit 4fb9f5acb4a3b94258689fdae0a39eaa12c092fc
Author: Jonathan Backer <backer@chromium.org>
Date: Thu May 10 22:19:33 2018

Lazy reset state tracking

Saves 0.4 ms out of 9.4 ms on thread_GPU_cpu_time_per_frame on
thread_times.key_silk_cases running on N5 with OOP-R enabled.

This CL does a couple of things.

(1) dirties GrContext when we make GL calls directly (via api() in
    RasterDecoderImpl or in helpers like TextureManager)

(2) does a hard RestoreState(nullptr) just before we service another
    decoder using virtual contexts.

The correctness/performance of this CL relies on two facts:

- RasterDecoder functions don't rely on ContextState persistence between
  calls (it uses Scoped*Binder and helpers call Restore* on exit). This
  means we can ignore how GrContext changes state during RasterDecoder
  execution.

- GetContextState is called if and only if we switch virtual contexts
  (https://cs.chromium.org/chromium/src/ui/gl/gl_context.cc?rcl=7665435ec16705f26cf15259d28b89d24ec80699&l=328)

This CL avoids most state resetting in the common case where we OOP-R a
bunch of tiles all at once (e.g. page load). Note our resetting is still
pessimistic. In particular, I would expect scenarios like this where we
still unnecessarily reset/restore state.

- client A receives GetContextState

- client B made current, but immediately descheduled to wait on a
SyncToken provided by A

- client A made current again

Bug: 836916
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I9eecef94a5429861ae4b70acdd3aba0450bded4b
Reviewed-on: https://chromium-review.googlesource.com/1047445
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Jonathan Backer <backer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557689}
[modify] https://crrev.com/4fb9f5acb4a3b94258689fdae0a39eaa12c092fc/gpu/command_buffer/service/raster_decoder.cc

I grabbed some low hanging fruit and saw a performance win. There is probably more that could be done to integrate the state trackers as mentioned by piman@ in the original bug report.

I don't intend to work on this further, so I'll mark this as available for now.

I don't think this is blocking anymore, in that case.