>Fortunately, this particular call sequence does not happen in practice.

Note: a malicious client is not bound by chrome's calling patterns, and we must handle the case safely.

Agreed. What I meant by "does not happen in practice" is that that we can take a perf hit on handling this case properly for the safe of security, but we can probably avoid the perf hit on the common case that actually does happen.

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

commit 5ac08a7287ff6b40881af8a8209db27f5ec4fb80
Author: Jonathan Backer <backer@chromium.org>
Date: Wed Jun 27 16:15:35 2018

Make GL state and ContextState consistent

ContextState is meant to track GL state. It is used for virtual context
switching and temporary changes to GL state (e.g. temporarily change GL
state and restore back to ContextState).

Whenever we call Ganesh we must assume that GL state has diverged from
ContextSate. This CL does 2 things within RasterDecoder:

(1) Whitelists certain commands as not caring if GL state has diverged
    from ContextState. This would be if the command doesn't access GL
    state (e.g. InsertSyncPoint) or is using Ganesh (RasterCHROMIUM).

(2) Does a hard reset back to ContextState whenever the GL state could
    have diverged from ContextState and a command is not whitelisted.

The purpose of the whitelist is to avoid unnecessary state resets on the
common OOP-R codepaths.

Correctness:

- GLES2Decoder: The GL state matches the ContextState before and after a
  command is executed here. The interesting case is making a
  GLES2Decoder current. If using a virtual context, we will restore
  state appropriately when the GLES2Decoder is made current because of
  RasterDecoderImpl::GetContextState.

- RasterDecoder: There are two cases to consider

  Case 1: Making a RasterDecoder current. If we are using virtual
  contexts, we will restore to |state_| and GrContext::resetContext
  because of RasterDecoderImpl::{GetContextState,RestoreState}. If not,
  we will restore to the previous GL state (either |state_| or GrContext
  consistent with previous GL state).

  Case 2a: Executing a whitelisted command: Either the command doesn't
    inspect/modify GL state (InsertSyncPoint, CreateAndConsumeTexture)
    or it requires and maintains that GrContext state tracking matches
    GL context state (e.g. *RasterCHROMIUM).

  Case 2b: Executing a command that is not whitelisted: We now force GL
    state to match |state_| with as necessary RestoreState(nullptr).
    This will GrContext::resetContext.

Bug:  856219 
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;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I85d9391f833760b37cb70d216e9e1e44ffe1c23f
Reviewed-on: https://chromium-review.googlesource.com/1114136
Commit-Queue: Jonathan Backer <backer@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570794}
[modify] https://crrev.com/5ac08a7287ff6b40881af8a8209db27f5ec4fb80/gpu/command_buffer/service/raster_decoder.cc

As noted on one of the CL comments, we took a slight perf hit when we create a new tile with OOP-R. There should be no regression when reusing a tile. Howeover, the RestoreState(nullptr) that was invoked on this path was 0.3 ms as opposed to the 1.2 ms for the glTexImage2D call on a Nexus 5. So this probably isn't noticeable.