The tricky part about logging to the console is that many of the errors are generated by invalid resources, from which we have no context. This means we don't know the instance to send the error message of.

Plumbing this through in a way that works proved much harder than I expected. I added log functions to the PpapiGlobals so that we can call it easily from all places. It can either go off an instance (like PPB_Console does) or a module, or nothing. In the module case, all consoles associated with all instances in the module get the log message, in the no context case, all consoles associated with any pepper plugin get the message.

In the IPC proxy, we know the module from whence the error came since there's a unique process for it. So proxied errors with no context when run out of process get translated into errors with PP_Module context. In the common case, there's only one instance for a module, so this works out nicely.

I added some logging ability to resources and added some errors to Graphics2D and URLLoader. We can add messages to more classes as the need arises. I added some advice on writing them to the chromium pepper page.

Review URL: https://chromiumcodereview.appspot.com/9160017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@119853 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 25 insertions, 6 deletions

diff --git a/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc b/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc
index ddfea5f..f76f622 100644
--- a/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc
+++ b/webkit/plugins/ppapi/ppb_graphics_2d_impl.cc
@@ -223,8 +223,11 @@ void PPB_Graphics2D_Impl::PaintImageData(PP_Resource image_data,
     return;
 
   EnterResourceNoLock<PPB_ImageData_API> enter(image_data, true);
-  if (enter.failed())
+  if (enter.failed()) {
+    Log(PP_LOGLEVEL_ERROR,
+        "PPB_Graphics2D.PaintImageData: Bad image resource.");
     return;
+  }
   PPB_ImageData_Impl* image_resource =
       static_cast<PPB_ImageData_Impl*>(enter.object());
 
@@ -232,8 +235,11 @@ void PPB_Graphics2D_Impl::PaintImageData(PP_Resource image_data,
   operation.paint_image = image_resource;
   if (!ValidateAndConvertRect(src_rect, image_resource->width(),
                               image_resource->height(),
-                              &operation.paint_src_rect))
+                              &operation.paint_src_rect)) {
+    Log(PP_LOGLEVEL_ERROR,
+        "PPB_Graphics2D.PaintImageData: Rectangle is outside bounds.");
     return;
+  }
 
   // Validate the bitmap position using the previously-validated rect, there
   // should be no painted area outside of the image.
@@ -259,16 +265,22 @@ void PPB_Graphics2D_Impl::Scroll(const PP_Rect* clip_rect,
   if (!ValidateAndConvertRect(clip_rect,
                               image_data_->width(),
                               image_data_->height(),
-                              &operation.scroll_clip_rect))
+                              &operation.scroll_clip_rect)) {
+    Log(PP_LOGLEVEL_ERROR,
+        "PPB_Graphics2D.Scroll: Rectangle is outside bounds.");
     return;
+  }
 
   // If we're being asked to scroll by more than the clip rect size, just
   // ignore this scroll command and say it worked.
   int32 dx = amount->x;
   int32 dy = amount->y;
   if (dx <= -image_data_->width() || dx >= image_data_->width() ||
-      dy <= -image_data_->height() || dy >= image_data_->height())
+      dy <= -image_data_->height() || dy >= image_data_->height()) {
+    Log(PP_LOGLEVEL_ERROR,
+        "PPB_Graphics2D.Scroll: Scroll amount is larger than image size.");
     return;
+  }
 
   operation.scroll_dx = dx;
   operation.scroll_dy = dy;
@@ -284,12 +296,19 @@ void PPB_Graphics2D_Impl::ReplaceContents(PP_Resource image_data) {
       static_cast<PPB_ImageData_Impl*>(enter.object());
 
   if (!PPB_ImageData_Impl::IsImageDataFormatSupported(
-          image_resource->format()))
+          image_resource->format())) {
+    Log(PP_LOGLEVEL_ERROR,
+        "PPB_Graphics2D.ReplaceContents: Image data format is not supported.");
     return;
+  }
 
   if (image_resource->width() != image_data_->width() ||
-      image_resource->height() != image_data_->height())
+      image_resource->height() != image_data_->height()) {
+    Log(PP_LOGLEVEL_ERROR,
+        "PPB_Graphics2D.ReplaceContents: Image size doesn't match "
+        "Graphics2D size.");
     return;
+  }
 
   QueuedOperation operation(QueuedOperation::REPLACE);
   operation.replace_image = image_resource;