Avoid null-pointer dereference for PPAPI Instance BindGraphics....

webkit/plugins/ppapi/ppapi_plugin_instance.cc

 // Copyright (c) 2010 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 "webkit/plugins/ppapi/ppapi_plugin_instance.h"
 
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/scoped_ptr.h"
 #include "base/utf_string_conversions.h"
@@ skipping 441 matching lines @@
   if (graphics_2d) {
     if (!graphics_2d->BindToInstance(this))
       return false;  // Can't bind to more than one instance.
 
     // See http://crbug.com/49403: this can be further optimized by keeping the
     // old device around and painting from it.
     if (bound_graphics_2d()) {
       // Start the new image with the content of the old image until the plugin
       // repaints.
       const SkBitmap* old_backing_bitmap =
           bound_graphics_2d()->image_data()->GetMappedBitmap();

[David Springer, 2011/01/24 20:19:29]

Is there any chance that these accessors could return a NULL ptr?  E.g. I can
imagine image_data() returning a NULL ptr while initialization is going on in
the NaCl process.  It seems to me that the chance of a NULL ptr is greater when
using a NaCl process than when using a trusted plugin.

-      SkRect old_size = SkRect::MakeWH(
-          SkScalar(static_cast<float>(old_backing_bitmap->width())),
-          SkScalar(static_cast<float>(old_backing_bitmap->height())));
+      if (old_backing_bitmap != NULL) {

[brettw, 2011/01/24 21:00:07]

Instead of checking to see if there's no mapped bitmap, this should just map the
data. Can you use an ImageDataAutoMapper on the image data? Then GetMappedBitmap
will always return success.

+        SkRect old_size = SkRect::MakeWH(
+            SkScalar(static_cast<float>(old_backing_bitmap->width())),
+            SkScalar(static_cast<float>(old_backing_bitmap->height())));
 
-      SkCanvas canvas(*graphics_2d->image_data()->GetMappedBitmap());
-      canvas.drawBitmap(*old_backing_bitmap, 0, 0);
+        SkCanvas canvas(*graphics_2d->image_data()->GetMappedBitmap());
+        canvas.drawBitmap(*old_backing_bitmap, 0, 0);
 
-      // Fill in any extra space with white.
-      canvas.clipRect(old_size, SkRegion::kDifference_Op);
-      canvas.drawARGB(255, 255, 255, 255);
+        // Fill in any extra space with white.
+        canvas.clipRect(old_size, SkRegion::kDifference_Op);
+        canvas.drawARGB(255, 255, 255, 255);
+      }
     }
 
     bound_graphics_ = graphics_2d;
     // BindToInstance will have invalidated the plugin if necessary.
   } else if (graphics_3d) {
     // Make sure graphics can only be bound to the instance it is
     // associated with.
     if (graphics_3d->instance() != this)
       return false;
     if (!graphics_3d->BindToInstance(true))
@@ skipping 747 matching lines @@
   return found->second;
 }
 
 bool PluginInstance::IsFullPagePlugin() const {
   WebFrame* frame = container()->element().document().frame();
   return frame->view()->mainFrame()->document().isPluginDocument();
 }
 
 }  // namespace ppapi
 }  // namespace webkit