Returning error when NotificationConversionHelper::NotificationBitmapToGfxImage() get failed in not…

chrome/browser/extensions/api/notifications/notifications_api.cc

Index: chrome/browser/extensions/api/notifications/notifications_api.cc
diff --git a/chrome/browser/extensions/api/notifications/notifications_api.cc b/chrome/browser/extensions/api/notifications/notifications_api.cc
index 71fb9122a375d0082b8a09c53efa7e975eab4f38..28406a0abc96b622440ddf0105a233bd7356f7c3 100644
--- a/chrome/browser/extensions/api/notifications/notifications_api.cc
+++ b/chrome/browser/extensions/api/notifications/notifications_api.cc
@@ -318,17 +318,23 @@ bool NotificationsApiFunction::UpdateNotification(
   // TODO(dewittj): Return error if this fails.

[Peter Beverloo, 2015/05/13 10:52:42]

This TODO is now redundant.

[Deepak, 2015/05/13 13:21:11]

Done.

   if (options->icon_bitmap) {
     gfx::Image icon;
-    NotificationConversionHelper::NotificationBitmapToGfxImage(
-        image_scale, bitmap_sizes.icon_size, options->icon_bitmap.get(), &icon);
+    if (!NotificationConversionHelper::NotificationBitmapToGfxImage(
+            image_scale, bitmap_sizes.icon_size, options->icon_bitmap.get(),
+            &icon)) {
+      SetError(kUnableToDecodeIconError);
+      return false;
+    }
     notification->set_icon(icon);
   }
 
-  gfx::Image app_icon_mask;
-  if (NotificationConversionHelper::NotificationBitmapToGfxImage(
-          image_scale,
-          bitmap_sizes.app_icon_mask_size,
-          options->app_icon_mask_bitmap.get(),
-          &app_icon_mask)) {
+  if (options->app_icon_mask_url.get()) {
+    gfx::Image app_icon_mask;
+    if (!NotificationConversionHelper::NotificationBitmapToGfxImage(

[Peter Beverloo, 2015/05/13 10:54:04]

On a slightly higher-level note, now that most calls to
NotificationBitmapToGfxImage() are guarded to make sure that the
|notification_bitmap| is non-NULL, it'd be cool if we can change the early
bail-out for NULL values to a DCHECK.

[Deepak, 2015/05/13 13:21:11]

Done.

+            image_scale, bitmap_sizes.app_icon_mask_size,
+            options->app_icon_mask_bitmap.get(), &app_icon_mask)) {
+      SetError(kUnableToDecodeIconError);
+      return false;
+    }
     notification->set_small_image(app_icon_mask);
   }
 
@@ -363,12 +369,16 @@ bool NotificationsApiFunction::UpdateNotification(
   }
 
   gfx::Image image;

[Peter Beverloo, 2015/05/13 10:52:42]

You can now move this to within the body of the new if-clause.

[Deepak, 2015/05/13 13:21:11]

Done.

-  bool has_image = NotificationConversionHelper::NotificationBitmapToGfxImage(
-      image_scale,
-      bitmap_sizes.image_size,
-      options->image_bitmap.get(),
-      &image);
-  if (has_image) {
+  if (options->image_url.get()) {
+    bool has_image = NotificationConversionHelper::NotificationBitmapToGfxImage(
+        image_scale,
+        bitmap_sizes.image_size,
+        options->image_bitmap.get(),
+        &image);
+    if (!has_image) {
+      SetError(kUnableToDecodeIconError);
+      return false;
+    }
     // We should have an image if and only if the type is an image type.
     if (notification->type() != message_center::NOTIFICATION_TYPE_IMAGE) {
       SetError(kExtraImageProvided);