Factor out cc::OutputSurface::InitializeAndSetContext3D

cc/output/output_surface.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/output/output_surface.h"
 
 #include <set>
 #include <string>
 #include <vector>
 
@@ skipping 12 matching lines @@
 using std::vector;
 
 namespace cc {
 
 class OutputSurfaceCallbacks
     : public WebKit::WebGraphicsContext3D::
         WebGraphicsSwapBuffersCompleteCallbackCHROMIUM,
       public WebKit::WebGraphicsContext3D::WebGraphicsContextLostCallback {
  public:
   explicit OutputSurfaceCallbacks(OutputSurfaceClient* client)
-      : client_(client) {}
+      : client_(client) {
+    DCHECK(client_);
+  }
 
   // WK:WGC3D::WGSwapBuffersCompleteCallbackCHROMIUM implementation.
   virtual void onSwapBuffersComplete() { client_->OnSwapBuffersComplete(); }
 
   // WK:WGC3D::WGContextLostCallback implementation.
   virtual void onContextLost() { client_->DidLoseOutputSurface(); }
 
  private:
   OutputSurfaceClient* client_;
 };
@@ skipping 24 matching lines @@
 OutputSurface::~OutputSurface() {
 }
 
 bool OutputSurface::ForcedDrawToSoftwareDevice() const {
   return false;
 }
 
 bool OutputSurface::BindToClient(
     cc::OutputSurfaceClient* client) {
   DCHECK(client);
-  if (context3d_ && !context3d_->makeContextCurrent())
+  client_ = client;
+  bool success = true;
+
+  if (context3d_)
+    success = InitializeAndSetContext3D(context3d_.Pass());
+
+  if (!success)

[danakj, 2013/06/05 18:50:22]

if (success) client_ = client; and drop the above assignment

[boliu, 2013/06/05 19:06:31]

Won't work because client_ is used in InitializeAndSetContext3D to create new
OutputSurfaceCallbacks.

Also context3d_.Pass() and then gets assigned back is a weird pattern as well,
but this is all to not duplicate code.

I'll take any suggestions to make this less weird.

[danakj, 2013/06/05 19:10:12]

Ah okay, I see. Well we have a test for exactly this, so I think it's all good.

+    client_ = NULL;
+
+  return success;
+}
+
+bool OutputSurface::InitializeAndSetContext3D(
+    scoped_ptr<WebKit::WebGraphicsContext3D> context3d) {
+  DCHECK(context3d);
+  DCHECK(client_);
+
+  if (!context3d->makeContextCurrent())
     return false;
-  client_ = client;
-  if (!context3d_)
-    return true;
-  string extensions_string = UTF16ToASCII(context3d_->getString(GL_EXTENSIONS));
+
+  string extensions_string = UTF16ToASCII(context3d->getString(GL_EXTENSIONS));
   vector<string> extensions_list;
   base::SplitString(extensions_string, ' ', &extensions_list);
   set<string> extensions(extensions_list.begin(), extensions_list.end());
-
   has_gl_discard_backbuffer_ =
       extensions.count("GL_CHROMIUM_discard_backbuffer") > 0;
 
+  context3d_ = context3d.Pass();
   callbacks_.reset(new OutputSurfaceCallbacks(client_));
   context3d_->setSwapBuffersCompleteCallbackCHROMIUM(callbacks_.get());
   context3d_->setContextLostCallback(callbacks_.get());
 
   return true;
 }
 
 void OutputSurface::SendFrameToParentCompositor(CompositorFrame* frame) {
   NOTIMPLEMENTED();
 }
@@ skipping 28 matching lines @@
 }
 
 void OutputSurface::PostSubBuffer(gfx::Rect rect,
                                   const ui::LatencyInfo& latency_info) {
   DCHECK(context3d_);
   context3d_->postSubBufferCHROMIUM(
       rect.x(), rect.y(), rect.width(), rect.height());
 }
 
 }  // namespace cc