Reland: Prevent making invalid SkImageReps

Original CL: https://chromium-review.googlesource.com/c/chromium/src/+/2572896

Ensure that SkImageRep is always constructed with a valid bitmap.
Instead of constructing an invalid SkImageRep then throwing it away when
initializing an SkImage, we add an SkImage::CreateFromBitmap() that will
just return a null SkImage directly if the bitmap is null/uninitialized.

ImageSkia was creating a null ImageSkiaRep when searching for a
representation in order to force use of the nearest one, but it also
uses a boolean value to do the same, so stop making a null ImageSkiaRep.

Then disallow null/empty ImageSkiaRep and also disallow their transport
over mojo.

Remove a few tests that were creating invalid ImageSkiaReps to test that
they work - as they are no longer valid.

Have ImageSkia constructed from ImageSkiaOperations check for the
ImageSkiaRep that they get from their inner (non-null!) ImageSkia, as
that can still return a null ImageSkiaRep. In that case, the resulting
ImageSkiaRep must also be null. And we can do that without trying to
perform operations on the (invalid, null) bitmap inside the null
ImageSkiaRep.

This fixes the tests included in the CL. We no longer try to serialize
null or empty ImageSkiaRep since the class itself will not allow it. We
do test that we will not deserialize such invalid ImageSkiaReps. And a
test is also added to verify we do not allow non-n32 bitmaps to be
deserialized in ImageSkiaRep.

R=dcheng@chromium.org, sky@chromium.org

Bug: 1155258
Change-Id: Ia18360dcd2a59be8c49b7104f01dbdd20bbe6bbc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2596131
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838132}

PRESUBMIT.py

@@ -1749,7 +1749,9 @@ def _GetMessageForMatchingType(input_api, affected_file, line_number, line,
   """
   result = []
 
-  if line.endswith(" nocheck"):
+  if input_api.re.search(r"^ *//", line):  # Ignore comments about banned types.
+    return result
+  if line.endswith(" nocheck"):  # A // nocheck comment will bypass this error.
     return result
 
   matched = False

PRESUBMIT_test.py

@@ -2320,6 +2320,10 @@ class BannedTypeCheckTest(unittest.TestCase):
                ['using std::string;']),
       MockFile('some/cpp/problematic/file2.cc',
                ['set_owned_by_client()']),
+      MockFile('some/cpp/nocheck/file.cc',
+               ['using namespace std;  // nocheck']),
+      MockFile('some/cpp/comment/file.cc',
+               ['  // A comment about `using namespace std;`']),
     ]
 
     results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
@@ -2331,6 +2335,10 @@ class BannedTypeCheckTest(unittest.TestCase):
         'third_party/blink/problematic/file.cc' in results[0].message)
     self.assertTrue('some/cpp/ok/file.cc' not in results[1].message)
     self.assertTrue('some/cpp/problematic/file2.cc' in results[0].message)
+    self.assertFalse('some/cpp/nocheck/file.cc' in results[0].message)
+    self.assertFalse('some/cpp/nocheck/file.cc' in results[1].message)
+    self.assertFalse('some/cpp/comment/file.cc' in results[0].message)
+    self.assertFalse('some/cpp/comment/file.cc' in results[1].message)
 
   def testBannedIosObjcFunctions(self):
     input_api = MockInputApi()

ash/accelerators/debug_commands.cc

@@ -48,7 +48,7 @@ gfx::ImageSkia CreateWallpaperImage(SkColor fill, SkColor rect) {
   paint.setBlendMode(SkBlendMode::kSrcOver);
   canvas.drawRoundRect(gfx::RectToSkRect(gfx::Rect(image_size)), 100.f, 100.f,
                        paint);
-  return gfx::ImageSkia(gfx::ImageSkiaRep(std::move(bitmap), 1.f));
+  return gfx::ImageSkia::CreateFromBitmap(std::move(bitmap), 1.f);
 }
 
 void HandleToggleWallpaperMode() {

ash/app_list/views/ghost_image_view.cc

@@ -188,7 +188,7 @@ gfx::ImageSkia GhostImageView::GetIconOutline(
                   bitmap.pixmap().height() + kGhostImagePadding * 2),
         rep.scale(), false /* is_opaque */);
     padded_canvas.DrawImageInt(
-        gfx::ImageSkia(gfx::ImageSkiaRep(bitmap, rep.scale())),
+        gfx::ImageSkia::CreateFromBitmap(bitmap, rep.scale()),
         kGhostImagePadding, kGhostImagePadding);
     bitmap = padded_canvas.GetBitmap();
 

ash/display/cursor_window_controller.cc

@@ -366,8 +366,8 @@ void CursorWindowController::UpdateCursorImage() {
 
   const gfx::ImageSkiaRep& image_rep = resized.GetRepresentation(cursor_scale);
   delegate_->SetCursorImage(resized.size(),
-                            gfx::ImageSkia(gfx::ImageSkiaRep(
-                                GetAdjustedBitmap(image_rep), cursor_scale)));
+                            gfx::ImageSkia::CreateFromBitmap(
+                                GetAdjustedBitmap(image_rep), cursor_scale));
   // TODO(danakj): Should this be rounded? Or kept as a floating point?
   hot_point_ = gfx::ToFlooredPoint(
       gfx::ConvertPointToDips(hot_point_in_physical_pixels, cursor_scale));

ash/drag_drop/drag_image_view.cc

@@ -127,7 +127,8 @@ void DragImageView::OnPaint(gfx::Canvas* canvas) {
     SkBitmap scaled = skia::ImageOperations::Resize(
         image_rep.GetBitmap(), skia::ImageOperations::RESIZE_LANCZOS3,
         drag_image_size_pixels.width(), drag_image_size_pixels.height());
-    gfx::ImageSkia image_skia(gfx::ImageSkiaRep(scaled, device_scale));
+    gfx::ImageSkia image_skia =
+        gfx::ImageSkia::CreateFromBitmap(scaled, device_scale);
     canvas->DrawImageInt(image_skia, 0, 0);
   }
 

ash/system/holding_space/holding_space_drag_util.cc

@@ -325,7 +325,7 @@ class DragImageView : public views::View {
                           /*clear_color=*/SK_ColorTRANSPARENT, is_pixel_canvas)
             .context(),
         size()));
-    return gfx::ImageSkia(gfx::ImageSkiaRep(bitmap, scale));
+    return gfx::ImageSkia::CreateFromBitmap(bitmap, scale);
   }
 
   // Returns the drag offset to use when rendering this view as a drag image.

chrome/browser/apps/app_service/app_icon_factory.cc

@@ -226,7 +226,7 @@ SkBitmap DecompressToSkBitmap(const unsigned char* data, size_t size) {
 }
 
 gfx::ImageSkia SkBitmapToImageSkia(SkBitmap bitmap, float icon_scale) {
-  return gfx::ImageSkia(gfx::ImageSkiaRep(bitmap, icon_scale));
+  return gfx::ImageSkia::CreateFromBitmap(bitmap, icon_scale);
 }
 
 // Returns a callback that converts a gfx::Image to an ImageSkia.

chrome/browser/apps/app_service/app_icon_factory_unittest.cc

@@ -195,7 +195,7 @@ class AppIconFactoryTest : public testing::Test {
     ASSERT_TRUE(gfx::PNGCodec::Decode(compressed_data.data(),
                                       compressed_data.size(), &decoded));
 
-    output_image_skia = gfx::ImageSkia(gfx::ImageSkiaRep(decoded, scale));
+    output_image_skia = gfx::ImageSkia::CreateFromBitmap(decoded, scale);
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
     if (base::FeatureList::IsEnabled(features::kAppServiceAdaptiveIcon)) {

chrome/browser/chromeos/crosapi/message_center_ash_unittest.cc

@@ -196,7 +196,8 @@ TEST_F(MessageCenterAshTest, HighDpiImage) {
 
   // Create a high DPI image.
   SkBitmap bitmap = gfx::test::CreateBitmap(2, 4);
-  gfx::ImageSkia high_dpi_image_skia(gfx::ImageSkiaRep(bitmap, 2.0f));
+  gfx::ImageSkia high_dpi_image_skia =
+      gfx::ImageSkia::CreateFromBitmap(bitmap, 2.0f);
   mojo_notification->image = high_dpi_image_skia;
 
   // Display the notification.

chrome/browser/chromeos/login/users/avatar/user_image_manager_test_util.cc

@@ -66,7 +66,7 @@ gfx::ImageSkia ImageLoader::Load() {
 }
 
 void ImageLoader::OnImageDecoded(const SkBitmap& decoded_image) {
-  decoded_image_ = gfx::ImageSkia(gfx::ImageSkiaRep(decoded_image, 1.0f));
+  decoded_image_ = gfx::ImageSkia::CreateFromBitmap(decoded_image, 1.0f);
   run_loop_.Quit();
 }
 

chrome/browser/chromeos/remote_apps/remote_apps_manager_browsertest.cc

@@ -126,7 +126,7 @@ gfx::ImageSkia CreateTestIcon(int size, SkColor color) {
   SkBitmap bitmap;
   bitmap.allocN32Pixels(size, size);
   bitmap.eraseColor(color);
-  return gfx::ImageSkia(gfx::ImageSkiaRep(bitmap, 1.0f));
+  return gfx::ImageSkia::CreateFromBitmap(bitmap, 1.0f);
 }
 
 void CheckIconsEqual(const gfx::ImageSkia& expected,

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

@@ -185,7 +185,7 @@ bool NotificationBitmapToGfxImage(
 
   // TODO(dewittj): Handle HiDPI images with more than one scale factor
   // representation.
-  gfx::ImageSkia skia(gfx::ImageSkiaRep(bitmap, 1.0f));
+  gfx::ImageSkia skia = gfx::ImageSkia::CreateFromBitmap(bitmap, 1.0f);
   *return_image = gfx::Image(skia);
   return true;
 }

chrome/browser/search/ntp_icon_source.cc

@@ -164,7 +164,7 @@ const ParsedNtpIconPath ParseNtpIconPath(const std::string& path) {
 void DrawFavicon(const SkBitmap& bitmap, gfx::Canvas* canvas, int size) {
   int x_origin = (size - bitmap.width()) / 2;
   int y_origin = (size - bitmap.height()) / 2;
-  canvas->DrawImageInt(gfx::ImageSkia(gfx::ImageSkiaRep(bitmap, /*scale=*/1.0)),
+  canvas->DrawImageInt(gfx::ImageSkia::CreateFromBitmap(bitmap, /*scale=*/1.f),
                        x_origin, y_origin);
 }
 

chrome/browser/ui/app_list/icon_standardizer_unittest.cc

@@ -87,7 +87,7 @@ TEST_F(CreateStandardIconTest, CircularIconToStandardIcon) {
 
   // Get the standard icon version of the red circle icon.
   gfx::ImageSkia generated_standard_icon = app_list::CreateStandardIconImage(
-      gfx::ImageSkia(gfx::ImageSkiaRep(circle_icon_bitmap, 2.0f)));
+      gfx::ImageSkia::CreateFromBitmap(circle_icon_bitmap, 2.0f));
 
   // Scale the bitmap to fit the size of a standardized circle icon.
   SkBitmap scaled_bitmap = skia::ImageOperations::Resize(
@@ -123,7 +123,7 @@ TEST_F(CreateStandardIconTest, StandardCircularIconToStandardIcon) {
 
   // Get the standard icon version of the red circle icon.
   gfx::ImageSkia standard_icon = app_list::CreateStandardIconImage(
-      gfx::ImageSkia(gfx::ImageSkiaRep(circle_icon_bitmap, 2.0f)));
+      gfx::ImageSkia::CreateFromBitmap(circle_icon_bitmap, 2.0f));
 
   EXPECT_TRUE(AreBitmapsEqual(*standard_icon.bitmap(), circle_icon_bitmap));
 }

chrome/browser/ui/app_list/md_icon_normalizer.cc

@@ -219,8 +219,8 @@ void MaybeResizeAndPad(const gfx::Size& required_size,
   }
 
   // Add padding.
-  gfx::Canvas canvas(required_size, 1, /* transparent */ false);
-  canvas.DrawImageInt(gfx::ImageSkia(gfx::ImageSkiaRep(resized, 1)),
+  gfx::Canvas canvas(required_size, 1, /*transparent=*/false);
+  canvas.DrawImageInt(gfx::ImageSkia::CreateFromBitmap(resized, 1),
                       padding.width(), padding.height());
   *bitmap_out = canvas.GetBitmap();
   return;

chrome/browser/ui/app_list/search/common/url_icon_source.cc

@@ -100,7 +100,7 @@ void UrlIconSource::OnSimpleLoaderComplete(
 
 void UrlIconSource::OnImageDecoded(const SkBitmap& decoded_image) {
   const float scale = decoded_image.width() / icon_size_;
-  icon_ = gfx::ImageSkia(gfx::ImageSkiaRep(decoded_image, scale));
+  icon_ = gfx::ImageSkia::CreateFromBitmap(decoded_image, scale);
   icon_loaded_callback_.Run();
 }
 

chrome/browser/ui/views/elevation_icon_setter.cc

@@ -77,7 +77,7 @@ void ElevationIconSetter::SetButtonIcon(base::OnceClosure callback,
 #endif
     button_->SetImage(
         views::Button::STATE_NORMAL,
-        gfx::ImageSkia(gfx::ImageSkiaRep(icon, device_scale_factor)));
+        gfx::ImageSkia::CreateFromBitmap(icon, device_scale_factor));
     button_->SizeToPreferredSize();
     if (button_->parent())
       button_->parent()->Layout();

chrome/browser/ui/views/global_media_controls/media_notification_container_impl_view.cc

@@ -264,7 +264,7 @@ void MediaNotificationContainerImplView::CreateDragImageWidget() {
                         true /* is_pixel_canvas */)
           .context(),
       GetPreferredSize()));
-  gfx::ImageSkia image(gfx::ImageSkiaRep(bitmap, 1.f));
+  gfx::ImageSkia image = gfx::ImageSkia::CreateFromBitmap(bitmap, 1.f);
   image_view->SetImage(image);
 
   drag_image_widget_->Show();

chrome/browser/ui/views/qrcode_generator/qrcode_generator_bubble.cc

@@ -71,7 +71,7 @@ gfx::ImageSkia GetPlaceholderImageSkia(const SkColor color) {
   bitmap.allocN32Pixels(kQRImageSizePx, kQRImageSizePx);
   bitmap.eraseARGB(0xFF, 0xFF, 0xFF, 0xFF);
   bitmap.eraseColor(color);
-  return gfx::ImageSkia(gfx::ImageSkiaRep(bitmap, 1.0f));
+  return gfx::ImageSkia::CreateFromBitmap(bitmap, 1.0f);
 }
 
 // Adds a new small vertical padding row to the current bottom of |layout|.

chrome/browser/ui/views/status_icons/status_icon_linux_wrapper.cc

@@ -30,7 +30,7 @@ gfx::ImageSkia GetBestImageRep(const gfx::ImageSkia& image) {
   }
   // All status icon implementations want the image in pixel coordinates, so use
   // a scale factor of 1.
-  return gfx::ImageSkia(gfx::ImageSkiaRep(best_rep, 1.0f));
+  return gfx::ImageSkia::CreateFromBitmap(best_rep, 1.0f);
 }
 
 }  // namespace

chrome/browser/ui/views/tab_icon_view.cc

@@ -36,8 +36,8 @@ gfx::ImageSkia CreateDefaultFavicon() {
 #if defined(OS_WIN)
   // The default window icon is the application icon, not the default favicon.
   HICON app_icon = GetAppIcon();
-  icon = gfx::ImageSkia(gfx::ImageSkiaRep(
-      IconUtil::CreateSkBitmapFromHICON(app_icon, gfx::Size(16, 16)), 1.0f));
+  icon = gfx::ImageSkia::CreateFromBitmap(
+      IconUtil::CreateSkBitmapFromHICON(app_icon, gfx::Size(16, 16)), 1.0f);
   DestroyIcon(app_icon);
 #else
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();

chrome/test/base/perf/performance_test.cc

@@ -63,7 +63,8 @@ void CreateAndSetWallpaper() {
                         /*isOpaque=*/true);
   SkCanvas canvas(bitmap);
   canvas.drawColor(SK_ColorGREEN);
-  gfx::ImageSkia image(gfx::ImageSkiaRep(std::move(bitmap), 1.f));
+  gfx::ImageSkia image =
+      gfx::ImageSkia::CreateFromBitmap(std::move(bitmap), 1.f);
 
   base::RunLoop run_loop;
   TestWallpaperObserver observer(run_loop.QuitClosure());

components/exo/drag_drop_operation.cc

@@ -276,7 +276,8 @@ void DragDropOperation::OnDragIconCaptured(const SkBitmap& icon_bitmap) {
   DCHECK(icon_);
 
   float scale_factor = origin_->get()->window()->layer()->device_scale_factor();
-  gfx::ImageSkia icon_skia(gfx::ImageSkiaRep(icon_bitmap, scale_factor));
+  gfx::ImageSkia icon_skia =
+      gfx::ImageSkia::CreateFromBitmap(icon_bitmap, scale_factor);
   gfx::Vector2d icon_offset = -icon_->get()->GetBufferOffset();
 
   if (os_exchange_data_) {

components/favicon_base/select_favicon_frames.cc

@@ -240,7 +240,7 @@ gfx::ImageSkia CreateFaviconImageSkia(
 
   if (desired_size_in_dip == 0) {
     size_t index = results[0].index;
-    return gfx::ImageSkia(gfx::ImageSkiaRep(bitmaps[index], 1.0f));
+    return gfx::ImageSkia::CreateFromBitmap(bitmaps[index], 1.0f);
   }
 
   auto image_source = std::make_unique<FaviconImageSource>();

content/browser/renderer_host/render_widget_host_impl.cc

@@ -2615,7 +2615,7 @@ void RenderWidgetHostImpl::StartDragging(
   }
 
   float scale = GetScaleFactorForView(GetView());
-  gfx::ImageSkia image(gfx::ImageSkiaRep(bitmap, scale));
+  gfx::ImageSkia image = gfx::ImageSkia::CreateFromBitmap(bitmap, scale);
   view->StartDragging(filtered_data, drag_operations_mask, image,
                       bitmap_offset_in_dip, *event_info, this);
 }

ui/gfx/icon_util.cc

@@ -98,7 +98,7 @@ bool BuildResizedImageFamily(const gfx::ImageFamily& image_family,
 // |bitmaps| must be an empty vector, and not NULL.
 // Returns true on success, false on failure. This fails if any image in
 // |image_family| is not a 32-bit ARGB image, or is otherwise invalid.
-bool ConvertImageFamilyToBitmaps(
+void ConvertImageFamilyToBitmaps(
     const gfx::ImageFamily& image_family,
     std::vector<SkBitmap>* bitmaps,
     scoped_refptr<base::RefCountedMemory>* png_bytes) {
@@ -117,8 +117,7 @@ bool ConvertImageFamilyToBitmaps(
 
     SkBitmap bitmap = image.AsBitmap();
     CHECK_EQ(bitmap.colorType(), kN32_SkColorType);
-    if (bitmap.isNull())
-      return false;
+    CHECK(!bitmap.isNull());
 
     // Special case: Icons exactly 256x256 are stored in PNG format.
     if (image.Width() == IconUtil::kLargeIconSize &&
@@ -128,8 +127,6 @@ bool ConvertImageFamilyToBitmaps(
       bitmaps->push_back(bitmap);
     }
   }
-
-  return true;
 }
 
 }  // namespace
@@ -461,8 +458,7 @@ bool IconUtil::CreateIconFileFromImageFamily(
 
   std::vector<SkBitmap> bitmaps;
   scoped_refptr<base::RefCountedMemory> png_bytes;
-  if (!ConvertImageFamilyToBitmaps(resized_image_family, &bitmaps, &png_bytes))
-    return false;
+  ConvertImageFamilyToBitmaps(resized_image_family, &bitmaps, &png_bytes);
 
   // Guaranteed true because BuildResizedImageFamily will provide at least one
   // image < 256x256.

ui/gfx/icon_util_unittest.cc

@@ -207,14 +207,8 @@ TEST_F(IconUtilTest, TestCreateIconFileInvalidParameters) {
   base::FilePath invalid_icon_filename =
       temp_directory_.GetPath().AppendASCII("<>?.ico");
 
-  // Null bitmap.
-  SkBitmap bitmap;
-  image_family.Add(gfx::Image::CreateFrom1xBitmap(bitmap));
-  EXPECT_FALSE(IconUtil::CreateIconFileFromImageFamily(image_family,
-                                                       valid_icon_filename));
-  EXPECT_FALSE(base::PathExists(valid_icon_filename));
-
   // Invalid file name.
+  SkBitmap bitmap;
   image_family.clear();
   bitmap.allocN32Pixels(1, 1);
   image_family.Add(gfx::Image::CreateFrom1xBitmap(bitmap));

ui/gfx/image/image_skia.cc

@@ -75,10 +75,13 @@ ImageSkiaRep ScaleImageSkiaRep(const ImageSkiaRep& rep, float target_scale) {
 
 // A helper class such that ImageSkia can be cheaply copied. ImageSkia holds a
 // refptr instance of ImageSkiaStorage, which in turn holds all of ImageSkia's
-// information. Using a |base::SequenceChecker| on a
-// |base::RefCountedThreadSafe| subclass may sound strange but is necessary to
-// turn the 'thread-non-safe modifiable ImageSkiaStorage' into the 'thread-safe
-// read-only ImageSkiaStorage'.
+// information.
+// The ImageSkia, and this class, are designed to be thread-safe in their const
+// methods, but also are bound to a single sequence for mutating methods.
+// NOTE: The FindRepresentation() method const and thread-safe *iff* it is
+// called with `fetch_new_image` set to true. Otherwise it may mutate the
+// class, which is not thread-safe. Internally, mutation is bound to a single
+// sequence with a `base::SequenceChecker`.
 class ImageSkiaStorage : public base::RefCountedThreadSafe<ImageSkiaStorage> {
  public:
   ImageSkiaStorage(std::unique_ptr<ImageSkiaSource> source,
@@ -122,7 +125,7 @@ class ImageSkiaStorage : public base::RefCountedThreadSafe<ImageSkiaStorage> {
   //   one and rescale the image.
   // Right now only Windows uses 2 and other platforms use 1 by default.
   // TODO(mukai, oshima): abandon 1 code path and use 2 for every platforms.
-  std::vector<ImageSkiaRep>::iterator FindRepresentation(
+  std::vector<const ImageSkiaRep>::iterator FindRepresentation(
       float scale,
       bool fetch_new_image) const;
 
@@ -131,9 +134,12 @@ class ImageSkiaStorage : public base::RefCountedThreadSafe<ImageSkiaStorage> {
 
   virtual ~ImageSkiaStorage();
 
-  // Vector of bitmaps and their associated scale.
+  // Each entry in here has a different scale and is returned when looking for
+  // an ImageSkiaRep of that scale.
   std::vector<gfx::ImageSkiaRep> image_reps_;
 
+  // If no ImageSkiaRep exists in `image_reps_` for a given scale, the `source_`
+  // is queried to produce an ImageSkiaRep at that scale.
   std::unique_ptr<ImageSkiaSource> source_;
 
   // Size of the image in DIP.
@@ -141,7 +147,9 @@ class ImageSkiaStorage : public base::RefCountedThreadSafe<ImageSkiaStorage> {
 
   bool read_only_;
 
-  base::SequenceChecker sequence_checker_;
+  // This isn't using SEQUENCE_CHECKER() macros because we use the sequence
+  // checker outside of DCHECKs to make branching decisions.
+  base::SequenceChecker sequence_checker_;  // nocheck
 
   DISALLOW_COPY_AND_ASSIGN(ImageSkiaStorage);
 };
@@ -203,16 +211,13 @@ bool ImageSkiaStorage::HasRepresentationAtAllScales() const {
   return source_ && source_->HasRepresentationAtAllScales();
 }
 
-std::vector<ImageSkiaRep>::iterator ImageSkiaStorage::FindRepresentation(
+std::vector<const ImageSkiaRep>::iterator ImageSkiaStorage::FindRepresentation(
     float scale,
     bool fetch_new_image) const {
-  ImageSkiaStorage* non_const = const_cast<ImageSkiaStorage*>(this);
-
-  auto closest_iter = non_const->image_reps().end();
-  auto exact_iter = non_const->image_reps().end();
+  auto closest_iter = image_reps_.end();
+  auto exact_iter = image_reps_.end();
   float smallest_diff = std::numeric_limits<float>::max();
-  for (auto it = non_const->image_reps().begin(); it < image_reps_.end();
-       ++it) {
+  for (auto it = image_reps_.begin(); it < image_reps_.end(); ++it) {
     if (it->scale() == scale) {
       // found exact match
       fetch_new_image = false;
@@ -228,10 +233,15 @@ std::vector<ImageSkiaRep>::iterator ImageSkiaStorage::FindRepresentation(
     }
   }
 
-  if (fetch_new_image && source_.get()) {
+  if (fetch_new_image && source_) {
     DCHECK(sequence_checker_.CalledOnValidSequence())
         << "An ImageSkia with the source must be accessed by the same "
            "sequence.";
+    // This method is const and thread-safe, unless `fetch_new_image` is true,
+    // in which case the method is no longer considered const and we ensure
+    // that it is used in this way on a single sequence at a time with the above
+    // `sequence_checker_`.
+    auto* mutable_this = const_cast<ImageSkiaStorage*>(this);
 
     ImageSkiaRep image;
     float resource_scale = scale;
@@ -256,17 +266,12 @@ std::vector<ImageSkiaRep>::iterator ImageSkiaStorage::FindRepresentation(
                      [&image](const ImageSkiaRep& rep) {
                        return rep.scale() == image.scale();
                      }) == image_reps_.end()) {
-      non_const->image_reps().push_back(image);
+      mutable_this->image_reps_.push_back(image);
     }
 
-    // If the result image's scale isn't same as the expected scale, create a
-    // null ImageSkiaRep with the |scale| so that the next lookup will fall back
-    // to the closest scale.
-    if (image.is_null() || image.scale() != scale) {
-      non_const->image_reps().push_back(ImageSkiaRep(SkBitmap(), scale));
-    }
-
-    // image_reps_ must have the exact much now, so find again.
+    // image_reps_ should have the exact much now, or we will fallback
+    // to the new closest value. We pass false to prevent the generation step
+    // from running again and repeating the recursion.
     return FindRepresentation(scale, false);
   }
   return exact_iter != image_reps_.end() ? exact_iter : closest_iter;
@@ -299,6 +304,7 @@ ImageSkia::ImageSkia(std::unique_ptr<ImageSkiaSource> source, float scale)
 }
 
 ImageSkia::ImageSkia(const ImageSkiaRep& image_rep) {
+  DCHECK(!image_rep.is_null());
   Init(image_rep);
   // No other thread has reference to this, so it's safe to detach the sequence.
   DetachStorageFromSequence();
@@ -334,8 +340,19 @@ float ImageSkia::GetMaxSupportedScale() {
   return g_supported_scales->back();
 }
 
+// static
+ImageSkia ImageSkia::CreateFromBitmap(const SkBitmap& bitmap, float scale) {
+  // An uninitialized/empty/null bitmap makes a null ImageSkia.
+  if (bitmap.drawsNothing())
+    return ImageSkia();
+  return ImageSkia(ImageSkiaRep(bitmap, scale));
+}
+
 // static
 ImageSkia ImageSkia::CreateFrom1xBitmap(const SkBitmap& bitmap) {
+  // An uninitialized/empty/null bitmap makes a null ImageSkia.
+  if (bitmap.drawsNothing())
+    return ImageSkia();
   return ImageSkia(ImageSkiaRep(bitmap, 0.0f));
 }
 
@@ -367,7 +384,6 @@ const void* ImageSkia::GetBackingObject() const {
 
 void ImageSkia::AddRepresentation(const ImageSkiaRep& image_rep) {
   DCHECK(!image_rep.is_null());
-
   // TODO(oshima): This method should be called |SetRepresentation|
   // and replace the existing rep if there is already one with the
   // same scale so that we can guarantee that a ImageSkia instance contains only
@@ -496,10 +512,7 @@ void ImageSkia::RemoveUnsupportedRepresentationsForScale(float scale) {
 }
 
 void ImageSkia::Init(const ImageSkiaRep& image_rep) {
-  if (image_rep.GetBitmap().drawsNothing()) {
-    storage_.reset();
-    return;
-  }
+  DCHECK(!image_rep.is_null());
   storage_ = new internal::ImageSkiaStorage(
       NULL, gfx::Size(image_rep.GetWidth(), image_rep.GetHeight()));
   storage_->image_reps().push_back(image_rep);

ui/gfx/image/image_skia.h

@@ -73,9 +73,21 @@ class GFX_EXPORT ImageSkia {
   // Returns the maximum scale supported by this platform.
   static float GetMaxSupportedScale();
 
-  // Creates an image from the passed in bitmap.
-  // DIP width and height are based on scale factor of 1x.
-  // Adds ref to passed in bitmap.
+  // Creates an image from the passed in bitmap, which is designed for display
+  // at the device scale factor given in `scale`. The DIP width and height will
+  // be based on that scale factor. A scale factor of 0 is equivalent to
+  // calling CreateFrom1xBitmap(), which indicates the bitmap is not scaled.
+  // The `bitmap`, if present, will be made immutable. If the `bitmap` is
+  // uninitialized, empty, or null then the returned ImageSkia will be
+  // default-constructed and empty.
+  // WARNING: If the device scale factory differs from the scale given here,
+  // the resulting image will be pixelated when displayed.
+  static ImageSkia CreateFromBitmap(const SkBitmap& bitmap, float scale);
+
+  // Creates an image from the passed in bitmap. The DIP width and height will
+  // be based on scale factor of 1x. The `bitmap`, if present, will be made
+  // immutable. If the bitmap is uninitialized, empty, or null then the
+  // returned ImageSkia will be default-constructed and empty.
   // WARNING: The resulting image will be pixelated when painted on a high
   // density display.
   static ImageSkia CreateFrom1xBitmap(const SkBitmap& bitmap);

ui/gfx/image/image_skia_operations.cc

@@ -69,7 +69,12 @@ class BinaryImageSource : public gfx::ImageSkiaSource {
   // gfx::ImageSkiaSource overrides:
   ImageSkiaRep GetImageForScale(float scale) override {
     ImageSkiaRep first_rep = first_.GetRepresentation(scale);
+    if (first_rep.is_null())
+      return first_rep;
     ImageSkiaRep second_rep = second_.GetRepresentation(scale);
+    if (second_rep.is_null())
+      return second_rep;
+
     if (first_rep.pixel_size() != second_rep.pixel_size()) {
       DCHECK_NE(first_rep.scale(), second_rep.scale());
       if (first_rep.scale() == second_rep.scale()) {
@@ -167,6 +172,9 @@ class TransparentImageSource : public gfx::ImageSkiaSource {
   // gfx::ImageSkiaSource overrides:
   ImageSkiaRep GetImageForScale(float scale) override {
     ImageSkiaRep image_rep = image_.GetRepresentation(scale);
+    if (image_rep.is_null())
+      return image_rep;
+
     SkBitmap alpha;
     alpha.allocN32Pixels(image_rep.pixel_width(),
                          image_rep.pixel_height());
@@ -220,6 +228,9 @@ class TiledImageSource : public gfx::ImageSkiaSource {
   // gfx::ImageSkiaSource overrides:
   ImageSkiaRep GetImageForScale(float scale) override {
     ImageSkiaRep source_rep = source_.GetRepresentation(scale);
+    if (source_rep.is_null())
+      return source_rep;
+
     gfx::Rect bounds = DIPToPixelBounds(gfx::Rect(src_x_, src_y_, dst_w_,
                                                   dst_h_), source_rep.scale());
     return ImageSkiaRep(SkBitmapOperations::CreateTiledBitmap(
@@ -251,6 +262,9 @@ class HSLImageSource : public gfx::ImageSkiaSource {
   // gfx::ImageSkiaSource overrides:
   ImageSkiaRep GetImageForScale(float scale) override {
     ImageSkiaRep image_rep = image_.GetRepresentation(scale);
+    if (image_rep.is_null())
+      return image_rep;
+
     return gfx::ImageSkiaRep(SkBitmapOperations::CreateHSLShiftedBitmap(
                                  image_rep.GetBitmap(), hsl_shift_),
                              image_rep.scale());
@@ -280,7 +294,12 @@ class ButtonImageSource: public gfx::ImageSkiaSource {
   // gfx::ImageSkiaSource overrides:
   ImageSkiaRep GetImageForScale(float scale) override {
     ImageSkiaRep image_rep = image_.GetRepresentation(scale);
+    if (image_rep.is_null())
+      return image_rep;
     ImageSkiaRep mask_rep = mask_.GetRepresentation(scale);
+    if (mask_rep.is_null())
+      return image_rep;
+
     if (image_rep.scale() != mask_rep.scale()) {
       image_rep = image_.GetRepresentation(1.0f);
       mask_rep = mask_.GetRepresentation(1.0f);
@@ -314,6 +333,9 @@ class ExtractSubsetImageSource: public gfx::ImageSkiaSource {
   // gfx::ImageSkiaSource overrides:
   ImageSkiaRep GetImageForScale(float scale) override {
     ImageSkiaRep image_rep = image_.GetRepresentation(scale);
+    if (image_rep.is_null())
+      return image_rep;
+
     SkIRect subset_bounds_in_pixel = RectToSkIRect(
         DIPToPixelBounds(subset_bounds_, image_rep.scale()));
     SkBitmap dst;
@@ -346,6 +368,8 @@ class ResizeSource : public ImageSkiaSource {
   // gfx::ImageSkiaSource overrides:
   ImageSkiaRep GetImageForScale(float scale) override {
     const ImageSkiaRep& image_rep = source_.GetRepresentation(scale);
+    if (image_rep.is_null())
+      return image_rep;
     if (image_rep.GetWidth() == target_dip_size_.width() &&
         image_rep.GetHeight() == target_dip_size_.height())
       return image_rep;
@@ -376,6 +400,8 @@ class DropShadowSource : public ImageSkiaSource {
   // gfx::ImageSkiaSource overrides:
   ImageSkiaRep GetImageForScale(float scale) override {
     const ImageSkiaRep& image_rep = source_.GetRepresentation(scale);
+    if (image_rep.is_null())
+      return image_rep;
 
     ShadowValues shadows_in_pixel;
     for (size_t i = 0; i < shadows_in_dip_.size(); ++i)
@@ -442,6 +468,9 @@ class RotatedSource : public ImageSkiaSource {
   // gfx::ImageSkiaSource overrides:
   ImageSkiaRep GetImageForScale(float scale) override {
     const ImageSkiaRep& image_rep = source_.GetRepresentation(scale);
+    if (image_rep.is_null())
+      return image_rep;
+
     const SkBitmap rotated_bitmap =
         SkBitmapOperations::Rotate(image_rep.GetBitmap(), rotation_);
     return ImageSkiaRep(rotated_bitmap, image_rep.scale());
@@ -487,6 +516,9 @@ class ColorMaskSource : public gfx::ImageSkiaSource {
   // gfx::ImageSkiaSource overrides:
   ImageSkiaRep GetImageForScale(float scale) override {
     ImageSkiaRep image_rep = image_.GetRepresentation(scale);
+    if (image_rep.is_null())
+      return image_rep;
+
     return ImageSkiaRep(
         SkBitmapOperations::CreateColorMask(image_rep.GetBitmap(), color_),
         image_rep.scale());

ui/gfx/image/image_skia_rep_default.cc

@@ -4,7 +4,7 @@
 
 #include "ui/gfx/image/image_skia_rep_default.h"
 
-#include "base/check.h"
+#include "base/check_op.h"
 #include "base/notreached.h"
 #include "cc/paint/display_item_list.h"
 #include "cc/paint/record_paint_canvas.h"
@@ -33,10 +33,8 @@ ImageSkiaRep::ImageSkiaRep(const SkBitmap& src, float scale)
       pixel_size_(gfx::Size(src.width(), src.height())),
       bitmap_(src),
       scale_(scale) {
-  // If the bitmap has been initialized then it must be in N32 format.
-  if (!(bitmap_.isNull() && bitmap_.colorType() == kUnknown_SkColorType &&
-        bitmap_.alphaType() == kUnknown_SkAlphaType))
-    CHECK_EQ(bitmap_.colorType(), kN32_SkColorType);
+  CHECK_EQ(bitmap_.colorType(), kN32_SkColorType);
+  DCHECK(!bitmap_.drawsNothing());
   bitmap_.setImmutable();
   paint_image_ = cc::PaintImage::CreateFromBitmap(src);
 }
@@ -47,7 +45,9 @@ ImageSkiaRep::ImageSkiaRep(sk_sp<cc::PaintRecord> paint_record,
     : paint_record_(std::move(paint_record)),
       type_(ImageRepType::kImageTypeDrawable),
       pixel_size_(pixel_size),
-      scale_(scale) {}
+      scale_(scale) {
+  DCHECK(!pixel_size.IsEmpty());
+}
 
 ImageSkiaRep::ImageSkiaRep(const ImageSkiaRep& other)
     : paint_image_(other.paint_image_),

ui/gfx/image/image_skia_rep_default.h

@@ -28,17 +28,21 @@ class GFX_EXPORT ImageSkiaRep {
   // Creates a bitmap with kARGB_8888_Config config with given |size| in DIP.
   // This allocates pixels in the bitmap. It is guaranteed that the data in the
   // bitmap are initialized but the actual values are undefined.
-  // Specifying 0 scale means the image is for unscaled image. (unscaled()
-  // returns truen, and scale() returns 1.0f;)
+  // Specifying 0 scale means the image is for unscaled image (unscaled()
+  // returns true, and scale() returns 1.0f).
   ImageSkiaRep(const gfx::Size& size, float scale);
 
-  // Creates a bitmap with given scale.
-  // Adds ref to |src|.
+  // Creates an ImageSkiaRep that holds the `src` bitmap, which is created for
+  // display at the given device scale factor. Takes ownership of a reference to
+  // the SkBitmap's backing store. The `src` bitmap may not be uninitialized,
+  // null or empty; in that case the default constructor should be used
+  // instead.
   ImageSkiaRep(const SkBitmap& src, float scale);
 
   // Creates an image rep backed by a paint record of given size and scale. This
-  // is used when the image representation is sourced from a drawable sunch as
-  // CanvasImageSource.
+  // is used when the image representation is sourced from a drawable such as
+  // CanvasImageSource. The `size` must not be empty; in that case the default
+  // constructor should be used instead.
   ImageSkiaRep(sk_sp<cc::PaintRecord> paint_record,
                const gfx::Size& size,
                float scale);

ui/gfx/image/image_skia_rep_ios.cc

@@ -4,6 +4,7 @@
 
 #include "ui/gfx/image/image_skia_rep_ios.h"
 
+#include "base/check_op.h"
 #include "ui/gfx/color_palette.h"
 
 namespace gfx {
@@ -22,6 +23,8 @@ ImageSkiaRep::ImageSkiaRep(const SkBitmap& src, float scale)
     : pixel_size_(gfx::Size(src.width(), src.height())),
       bitmap_(src),
       scale_(scale) {
+  CHECK_EQ(src.colorType(), kN32_SkColorType);
+  DCHECK(!bitmap_.drawsNothing());
   bitmap_.setImmutable();
 }
 

ui/gfx/image/mojom/BUILD.gn

@@ -20,7 +20,6 @@ mojom("mojom") {
         {
           mojom = "gfx.mojom.ImageSkiaRep"
           cpp = "::gfx::ImageSkiaRep"
-          nullable_is_same_type = true
         },
       ]
       traits_headers = [ "image_skia_mojom_traits.h" ]
@@ -61,6 +60,7 @@ source_set("unit_test") {
     "//base",
     "//base/test:test_support",
     "//mojo/public/cpp/bindings",
+    "//mojo/public/cpp/test_support:test_utils",
     "//testing/gtest",
     "//ui/gfx:test_support",
   ]

ui/gfx/image/mojom/image.mojom

@@ -8,7 +8,10 @@ import "skia/public/mojom/bitmap.mojom";
 
 [Stable]
 struct ImageSkiaRep {
-  // Transport of the bitmap in this representation.
+  // Transport of the bitmap in this representation. The type here is
+  // BitmapWithArbitraryBpp because this structure is marked Stable, however
+  // only N32-format bitmaps are allowed, similar to the skia.mojom.BitmapN32
+  // type.
   skia.mojom.BitmapWithArbitraryBpp bitmap;
 
   // Corresponding scale of the bitmap or 0 if unscaled.

ui/gfx/image/mojom/image_skia_mojom_traits.cc

@@ -10,12 +10,21 @@
 
 namespace mojo {
 
+// static
+SkBitmap
+StructTraits<gfx::mojom::ImageSkiaRepDataView, gfx::ImageSkiaRep>::bitmap(
+    const gfx::ImageSkiaRep& input) {
+  SkBitmap bitmap = input.GetBitmap();
+  DCHECK(!bitmap.drawsNothing());
+  CHECK_EQ(bitmap.colorType(), kN32_SkColorType);
+  return bitmap;
+}
+
 // static
 float StructTraits<gfx::mojom::ImageSkiaRepDataView, gfx::ImageSkiaRep>::scale(
     const gfx::ImageSkiaRep& input) {
   const float scale = input.unscaled() ? 0.0f : input.scale();
   DCHECK_GE(scale, 0.0f);
-
   return scale;
 }
 
@@ -30,6 +39,14 @@ bool StructTraits<gfx::mojom::ImageSkiaRepDataView, gfx::ImageSkiaRep>::Read(
   SkBitmap bitmap;
   if (!data.ReadBitmap(&bitmap))
     return false;
+  // Null/uninitialized bitmaps are not allowed, and an ImageSkiaRep is never
+  // empty-sized either.
+  if (bitmap.drawsNothing())
+    return false;
+  // Similar to BitmapN32, ImageSkiaReps are expected to have an N32 bitmap
+  // type.
+  if (bitmap.colorType() != kN32_SkColorType)
+    return false;
 
   *out = gfx::ImageSkiaRep(bitmap, data.scale());
   return true;

ui/gfx/image/mojom/image_skia_mojom_traits.h

@@ -9,6 +9,7 @@
 
 #include <vector>
 
+#include "base/check_op.h"
 #include "skia/public/mojom/bitmap_skbitmap_mojom_traits.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/image/image_skia.h"
@@ -19,14 +20,9 @@ namespace mojo {
 
 template <>
 struct StructTraits<gfx::mojom::ImageSkiaRepDataView, gfx::ImageSkiaRep> {
-  static SkBitmap bitmap(const gfx::ImageSkiaRep& input) {
-    return input.GetBitmap();
-  }
+  static SkBitmap bitmap(const gfx::ImageSkiaRep& input);
   static float scale(const gfx::ImageSkiaRep& input);
 
-  static bool IsNull(const gfx::ImageSkiaRep& input) { return input.is_null(); }
-  static void SetToNull(gfx::ImageSkiaRep* out) { *out = gfx::ImageSkiaRep(); }
-
   static bool Read(gfx::mojom::ImageSkiaRepDataView data,
                    gfx::ImageSkiaRep* out);
 };

ui/gfx/image/mojom/image_traits_unittest.cc

@@ -9,14 +9,15 @@
 #include "base/test/task_environment.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "mojo/public/cpp/bindings/remote.h"
+#include "mojo/public/cpp/test_support/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/image/image_skia_operations.h"
 #include "ui/gfx/image/image_skia_source.h"
 #include "ui/gfx/image/image_unittest_util.h"
+#include "ui/gfx/image/mojom/image.mojom.h"
 #include "ui/gfx/image/mojom/image_skia_mojom_traits.h"
-#include "ui/gfx/image/mojom/image_traits_test_service.mojom.h"
 
 namespace gfx {
 
@@ -25,8 +26,7 @@ namespace {
 // A test ImageSkiaSource that creates an ImageSkiaRep for any scale.
 class TestImageSkiaSource : public ImageSkiaSource {
  public:
-  explicit TestImageSkiaSource(const gfx::Size& dip_size)
-      : dip_size_(dip_size) {}
+  explicit TestImageSkiaSource(const Size& dip_size) : dip_size_(dip_size) {}
   ~TestImageSkiaSource() override = default;
 
   // ImageSkiaSource:
@@ -35,156 +35,132 @@ class TestImageSkiaSource : public ImageSkiaSource {
   }
 
  private:
-  const gfx::Size dip_size_;
+  const Size dip_size_;
 
   DISALLOW_COPY_AND_ASSIGN(TestImageSkiaSource);
 };
 
-// Revisit this after Deserialize(Serialize()) API works with handles.
-class ImageTraitsTest : public testing::Test,
-                        public mojom::ImageTraitsTestService {
- public:
-  ImageTraitsTest() = default;
-
-  // testing::Test:
-  void SetUp() override {
-    receivers_.Add(this, service_.BindNewPipeAndPassReceiver());
-  }
-
-  mojo::Remote<mojom::ImageTraitsTestService>& service() { return service_; }
-
- private:
-  // mojom::ImageTraitsTestService:
-  void EchoImageSkiaRep(const ImageSkiaRep& in,
-                        EchoImageSkiaRepCallback callback) override {
-    std::move(callback).Run(in);
-  }
-  void EchoImageSkia(const ImageSkia& in,
-                     EchoImageSkiaCallback callback) override {
-    std::move(callback).Run(in);
-  }
-
-  base::test::TaskEnvironment task_environment_;
-  mojo::ReceiverSet<ImageTraitsTestService> receivers_;
-  mojo::Remote<mojom::ImageTraitsTestService> service_;
-
-  DISALLOW_COPY_AND_ASSIGN(ImageTraitsTest);
-};
+// A helper to construct a skia.mojom.BitmapN32 without using StructTraits
+// to bypass checks on the sending/serialization side.
+mojo::StructPtr<mojom::ImageSkiaRep> ConstructImageSkiaRep(
+    const SkBitmap& bitmap,
+    float scale) {
+  auto mojom_rep = mojom::ImageSkiaRep::New();
+  mojom_rep->bitmap = bitmap;
+  mojom_rep->scale = scale;
+  return mojom_rep;
+}
 
 }  // namespace
 
-TEST_F(ImageTraitsTest, NullImageSkiaRep) {
-  ImageSkiaRep null_rep;
-  ASSERT_TRUE(null_rep.is_null());
-
-  ImageSkiaRep output(gfx::Size(1, 1), 1.0f);
-  ASSERT_FALSE(output.is_null());
-  service()->EchoImageSkiaRep(null_rep, &output);
-  EXPECT_TRUE(output.is_null());
+TEST(ImageTraitsTest, VerifyMojomConstruction) {
+  SkBitmap bitmap;
+  bitmap.allocN32Pixels(1, 1);
+  mojo::StructPtr<mojom::ImageSkiaRep> input = ConstructImageSkiaRep(bitmap, 1);
+  ImageSkiaRep output;
+  EXPECT_TRUE(
+      mojo::test::SerializeAndDeserialize<mojom::ImageSkiaRep>(input, output));
 }
 
-TEST_F(ImageTraitsTest, EmptyImageSkiaRep) {
+TEST(ImageTraitsTest, BadColorTypeImageSkiaRep_Deserialize) {
+  SkBitmap a8_bitmap;
+  a8_bitmap.allocPixels(SkImageInfo::MakeA8(1, 1));
+
+  mojo::StructPtr<mojom::ImageSkiaRep> input =
+      ConstructImageSkiaRep(a8_bitmap, 1);
+  ImageSkiaRep output;
+  EXPECT_FALSE(
+      mojo::test::SerializeAndDeserialize<mojom::ImageSkiaRep>(input, output));
+}
+
+TEST(ImageTraitsTest, EmptyImageSkiaRep_Deserialize) {
   SkBitmap empty_bitmap;
   empty_bitmap.allocN32Pixels(0, 0);
   // Empty SkBitmap is not null.
   EXPECT_FALSE(empty_bitmap.isNull());
   EXPECT_TRUE(empty_bitmap.drawsNothing());
 
-  ImageSkiaRep empty_rep(empty_bitmap, 1.0f);
-  // ImageSkiaRep with empty bitmap is not null.
-  ASSERT_FALSE(empty_rep.is_null());
-
-  ImageSkiaRep output(gfx::Size(1, 1), 1.0f);
-  ASSERT_FALSE(output.is_null());
-  service()->EchoImageSkiaRep(empty_rep, &output);
-  EXPECT_TRUE(empty_rep.GetBitmap().drawsNothing());
-  EXPECT_TRUE(test::AreBitmapsEqual(empty_rep.GetBitmap(), output.GetBitmap()));
+  mojo::StructPtr<mojom::ImageSkiaRep> input =
+      ConstructImageSkiaRep(empty_bitmap, 1);
+  ImageSkiaRep output;
+  EXPECT_FALSE(
+      mojo::test::SerializeAndDeserialize<mojom::ImageSkiaRep>(input, output));
 }
 
-TEST_F(ImageTraitsTest, ImageSkiaRep) {
-  ImageSkiaRep image_rep(gfx::Size(2, 4), 2.0f);
+TEST(ImageTraitsTest, ValidImageSkiaRep) {
+  ImageSkiaRep image_rep(Size(2, 4), 2.0f);
 
   ImageSkiaRep output;
-  service()->EchoImageSkiaRep(image_rep, &output);
+  ASSERT_TRUE(mojo::test::SerializeAndDeserialize<mojom::ImageSkiaRep>(
+      image_rep, output));
 
   EXPECT_FALSE(output.is_null());
   EXPECT_EQ(image_rep.scale(), output.scale());
   EXPECT_TRUE(test::AreBitmapsEqual(image_rep.GetBitmap(), output.GetBitmap()));
 }
 
-TEST_F(ImageTraitsTest, UnscaledImageSkiaRep) {
-  ImageSkiaRep image_rep(gfx::Size(2, 4), 0.0f);
+TEST(ImageTraitsTest, UnscaledImageSkiaRep) {
+  ImageSkiaRep image_rep(Size(2, 4), 0.0f);
   ASSERT_TRUE(image_rep.unscaled());
 
-  ImageSkiaRep output(gfx::Size(1, 1), 1.0f);
-  EXPECT_FALSE(output.unscaled());
-  service()->EchoImageSkiaRep(image_rep, &output);
+  ImageSkiaRep output;
+  ASSERT_TRUE(mojo::test::SerializeAndDeserialize<mojom::ImageSkiaRep>(
+      image_rep, output));
   EXPECT_TRUE(output.unscaled());
   EXPECT_TRUE(test::AreBitmapsEqual(image_rep.GetBitmap(), output.GetBitmap()));
 }
 
-TEST_F(ImageTraitsTest, NullImageSkia) {
+TEST(ImageTraitsTest, NullImageSkia) {
   ImageSkia null_image;
   ASSERT_TRUE(null_image.isNull());
 
-  ImageSkia output(ImageSkiaRep(gfx::Size(1, 1), 1.0f));
+  ImageSkia output(ImageSkiaRep(Size(1, 1), 1.0f));
   ASSERT_FALSE(output.isNull());
-  service()->EchoImageSkia(null_image, &output);
+  ASSERT_TRUE(mojo::test::SerializeAndDeserialize<mojom::ImageSkia>(null_image,
+                                                                    output));
   EXPECT_TRUE(output.isNull());
 }
 
-TEST_F(ImageTraitsTest, ImageSkiaRepsAreCreatedAsNeeded) {
-  const gfx::Size kSize(1, 2);
+TEST(ImageTraitsTest, ImageSkiaRepsAreCreatedAsNeeded) {
+  const Size kSize(1, 2);
   ImageSkia image(std::make_unique<TestImageSkiaSource>(kSize), kSize);
   EXPECT_FALSE(image.isNull());
   EXPECT_TRUE(image.image_reps().empty());
 
   ImageSkia output;
   EXPECT_TRUE(output.isNull());
-  service()->EchoImageSkia(image, &output);
+  ASSERT_TRUE(
+      mojo::test::SerializeAndDeserialize<mojom::ImageSkia>(image, output));
   EXPECT_FALSE(image.image_reps().empty());
   EXPECT_FALSE(output.isNull());
 }
 
-TEST_F(ImageTraitsTest, ImageSkia) {
-  const gfx::Size kSize(1, 2);
+TEST(ImageTraitsTest, ImageSkia) {
+  const Size kSize(1, 2);
   ImageSkia image(std::make_unique<TestImageSkiaSource>(kSize), kSize);
   image.GetRepresentation(1.0f);
   image.GetRepresentation(2.0f);
 
   ImageSkia output;
-  service()->EchoImageSkia(image, &output);
+  ASSERT_TRUE(
+      mojo::test::SerializeAndDeserialize<mojom::ImageSkia>(image, output));
 
   EXPECT_TRUE(test::AreImagesEqual(Image(output), Image(image)));
 }
 
-TEST_F(ImageTraitsTest, EmptyRepPreserved) {
-  const gfx::Size kSize(1, 2);
-  ImageSkia image(std::make_unique<TestImageSkiaSource>(kSize), kSize);
-  image.GetRepresentation(1.0f);
-
-  SkBitmap empty_bitmap;
-  empty_bitmap.allocN32Pixels(0, 0);
-  image.AddRepresentation(ImageSkiaRep(empty_bitmap, 2.0f));
-
-  ImageSkia output;
-  service()->EchoImageSkia(image, &output);
-
-  EXPECT_TRUE(test::AreImagesEqual(Image(output), Image(image)));
-}
-
-TEST_F(ImageTraitsTest, ImageSkiaWithOperations) {
-  const gfx::Size kSize(32, 32);
+TEST(ImageTraitsTest, ImageSkiaWithOperations) {
+  const Size kSize(32, 32);
   ImageSkia image(std::make_unique<TestImageSkiaSource>(kSize), kSize);
 
-  const gfx::Size kNewSize(16, 16);
+  const Size kNewSize(16, 16);
   ImageSkia resized = ImageSkiaOperations::CreateResizedImage(
       image, skia::ImageOperations::RESIZE_BEST, kNewSize);
   resized.GetRepresentation(1.0f);
   resized.GetRepresentation(2.0f);
 
   ImageSkia output;
-  service()->EchoImageSkia(resized, &output);
+  ASSERT_TRUE(
+      mojo::test::SerializeAndDeserialize<mojom::ImageSkia>(resized, output));
 
   EXPECT_TRUE(test::AreImagesEqual(Image(output), Image(resized)));
 }

ui/gfx/nine_image_painter_unittest.cc

@@ -72,7 +72,7 @@ TEST(NineImagePainterTest, PaintHighDPI) {
 
   float image_scale = 2.f;
 
-  gfx::ImageSkia image(gfx::ImageSkiaRep(src, image_scale));
+  gfx::ImageSkia image = gfx::ImageSkia::CreateFromBitmap(src, image_scale);
   gfx::Insets insets(10, 10, 10, 10);
   gfx::NineImagePainter painter(image, insets);
 
@@ -104,7 +104,7 @@ TEST(NineImagePainterTest, PaintStaysInBounds) {
   src.eraseColor(SK_ColorGREEN);
   src.erase(SK_ColorRED, SkIRect::MakeXYWH(2, 2, 2, 2));
 
-  gfx::ImageSkia image(gfx::ImageSkiaRep(src, 0.0f));
+  gfx::ImageSkia image = gfx::ImageSkia::CreateFrom1xBitmap(src);
   gfx::Insets insets(2, 2, 2, 2);
   gfx::NineImagePainter painter(image, insets);
 
@@ -136,7 +136,7 @@ TEST(NineImagePainterTest, PaintWithBoundOffset) {
   src.eraseColor(SK_ColorRED);
   src.eraseArea(SkIRect::MakeXYWH(1, 1, 8, 8), SK_ColorGREEN);
 
-  gfx::ImageSkia image(gfx::ImageSkiaRep(src, 0.0f));
+  gfx::ImageSkia image = gfx::ImageSkia::CreateFrom1xBitmap(src);
   gfx::Insets insets(1, 1, 1, 1);
   gfx::NineImagePainter painter(image, insets);
 
@@ -168,7 +168,7 @@ TEST(NineImagePainterTest, PaintWithScale) {
 
   float image_scale = 2.f;
 
-  gfx::ImageSkia image(gfx::ImageSkiaRep(src, image_scale));
+  gfx::ImageSkia image = gfx::ImageSkia::CreateFromBitmap(src, image_scale);
   gfx::Insets insets(10, 10, 10, 10);
   gfx::NineImagePainter painter(image, insets);
 
@@ -199,7 +199,7 @@ TEST(NineImagePainterTest, PaintWithNegativeScale) {
 
   float image_scale = 2.f;
 
-  gfx::ImageSkia image(gfx::ImageSkiaRep(src, image_scale));
+  gfx::ImageSkia image = gfx::ImageSkia::CreateFromBitmap(src, image_scale);
   gfx::Insets insets(10, 10, 10, 10);
   gfx::NineImagePainter painter(image, insets);
 

ui/views/button_drag_utils.cc

@@ -91,7 +91,7 @@ void SetDragImage(const GURL& url,
                         widget.GetCompositor()->is_pixel_canvas())
           .context(),
       size));
-  gfx::ImageSkia image(gfx::ImageSkiaRep(bitmap, raster_scale));
+  gfx::ImageSkia image = gfx::ImageSkia::CreateFromBitmap(bitmap, raster_scale);
   data->provider().SetDragImage(image, press_point);
 }
 

ui/views/controls/menu/menu_controller.cc

@@ -1506,7 +1506,8 @@ void MenuController::StartDrag(SubmenuView* source,
   float raster_scale = ScaleFactorForDragFromWidget(source->GetWidget());
   gfx::Canvas canvas(item->size(), raster_scale, false /* opaque */);
   item->PaintButton(&canvas, MenuItemView::PaintButtonMode::kForDrag);
-  gfx::ImageSkia image(gfx::ImageSkiaRep(canvas.GetBitmap(), raster_scale));
+  gfx::ImageSkia image =
+      gfx::ImageSkia::CreateFromBitmap(canvas.GetBitmap(), raster_scale);
 
   std::unique_ptr<OSExchangeData> data(std::make_unique<OSExchangeData>());
   item->GetDelegate()->WriteDragData(item, data.get());

ui/views/controls/textfield/textfield.cc

@@ -1328,7 +1328,7 @@ void Textfield::WriteDragDataForView(View* sender,
           .context(),
       label.size()));
   constexpr gfx::Vector2d kOffset(-15, 0);
-  gfx::ImageSkia image(gfx::ImageSkiaRep(bitmap, raster_scale));
+  gfx::ImageSkia image = gfx::ImageSkia::CreateFromBitmap(bitmap, raster_scale);
   data->provider().SetDragImage(image, kOffset);
   if (controller_)
     controller_->OnWriteDragData(data);