The Shader class now support deferred shader compiling.

gpu/command_buffer/service/shader_manager.cc

Index: gpu/command_buffer/service/shader_manager.cc
diff --git a/gpu/command_buffer/service/shader_manager.cc b/gpu/command_buffer/service/shader_manager.cc
index 2707b90b92de4105d6b8c9f1dad28a0663158f0e..dabc7b492f68b072aba5a0b56ecbbc27a91d3c12 100644
--- a/gpu/command_buffer/service/shader_manager.cc
+++ b/gpu/command_buffer/service/shader_manager.cc
@@ -26,6 +26,7 @@ std::string GetTopVariableName(const std::string& fullname) {
 
 Shader::Shader(GLuint service_id, GLenum shader_type)
       : use_count_(0),
+        shader_state_(kShaderStateWaiting),
         service_id_(service_id),
         shader_type_(shader_type),
         valid_(false) {
@@ -34,13 +35,28 @@ Shader::Shader(GLuint service_id, GLenum shader_type)
 Shader::~Shader() {
 }
 
+void Shader::RequestCompile() {
+  shader_state_ = kShaderStateCompileRequested;
+  last_compiled_source_ = source_;
+}
+
 void Shader::DoCompile(ShaderTranslatorInterface* translator,
                        TranslatedShaderSourceType type) {
+  // We require that RequestCompile() must be called before DoCompile(),
+  // so we can return early if the shader state is not what we expect.
+  if (shader_state_ != kShaderStateCompileRequested) {
+    return;

[Ken Russell (switch to Gerrit), 2015/02/04 22:59:59]

The lack of error checking in this new code path seems problematic. It seems
shaders can transition from the "compiled" state back to the "compile requested"
state and back? Are there any assertions that can be made about the shader's
state? What happens if a new compilation request comes in while one is underway?
Etc.

Have you thought about how guarantees like those in section 2.10.3 "Program
Objects" of the OpenGL ES 3.0.3 spec are going to be maintained with this coming
asynchronous compilation and linking? For example, if a program's in use (via
UseProgram), and it's recompiled/relinked and the relink fails, then the
currently active program does not change. I'm not sure how the command buffer
handles this today.

[David Yen, 2015/02/05 00:09:02]

That functionality will still work, it is handled inside of
GLES2DecoderImpl::DoLinkProgram, it sets the current program after a successful
link if it is the currently used program.

Regarding the functionality of the compiler I believe returning here does the
right thing. Here are the relevant points from the spec:

  - If CompileShader failed, any information about a previous compile is lost.
Thus a failed compile does not restore the old state of shader.
  - Changing the source code of a shader object with ShaderSource does not
change its compile status or the compiled shader code

The way the current code is written, a valid shader must have both the state be
"compiled" and have the "valid" variable to be true. There are a couple of
situations that arise here:

1. User does not set the source and calls glGetShaderiv() or glLinkProgram(),
this will in turn calls DoCompile() which will immediately return since the
state is "waiting". The shader will be invalid which is correct.

2. User sets the source, calls glCompileShader(), then glGetShaderiv() or
glLinkProgram(). glCompileShader() will set the state to "CompileRequested" and
DoCompile() will set the state to "Compiled" while valid will be set to whether
or not the compile succeeded. This is the normal case and works as intended.

3. User finishes compiling something (so the state is "compiled"), then sets the
source using glSetSource, but then calls glGetShaderiv() or glLinkProgram()
without calling glCompileShader(). In that case the compiled shader should be
the originally compiled state and DoCompile() should not do anything (until
glCompileShader() is called and thus changes the state back to
"RequestCompile"). This also exhibits the correct behavior if you read the 2nd
bullet point I included from the spec.

4. User finished compiling something, then continually calls either
glGetShaderiv() or glLinkProgram(). This will not recompile the shader and just
continually return and not change the compile state or the valid boolean. This
is also the correct behavior.

I believe these 4 cases cover all the different cases that can happen. Let me
know if you think I have missed anything.

Perhaps I should document them in the comments?

+  }
+
+  // Signify the shader has been compiled, whether or not it is valid
+  // is dependent on the |valid_| member variable.
+  shader_state_ = kShaderStateCompiled;
+
   // Translate GL ES 2.0 shader to Desktop GL shader and pass that to
   // glShaderSource and then glCompileShader.
-  const char* source_for_driver = source_.c_str();
+  const char* source_for_driver = last_compiled_source_.c_str();
   if (translator) {
-    valid_ = translator->Translate(source_,
+    valid_ = translator->Translate(last_compiled_source_,
                                    &log_info_,
                                    &translated_source_,
                                    &attrib_map_,
@@ -50,7 +66,6 @@ void Shader::DoCompile(ShaderTranslatorInterface* translator,
     if (!valid_) {
       return;
     }
-    signature_source_ = source_;
     source_for_driver = translated_source_.c_str();
   }
 
@@ -88,7 +103,7 @@ void Shader::DoCompile(ShaderTranslatorInterface* translator,
     LOG_IF(ERROR, translator)
         << "Shader translator allowed/produced an invalid shader "
         << "unless the driver is buggy:"
-        << "\n--original-shader--\n" << source_
+        << "\n--original-shader--\n" << last_compiled_source_
         << "\n--translated-shader--\n" << source_for_driver
         << "\n--info-log--\n" << log_info_;
   }