Oilpan: simplify ImageBitmapTest heap clearing.

Source/core/frame/ImageBitmapTest.cpp

 /*
  * Copyright (c) 2013, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 128 matching lines @@
     memoryCache()->updateDecodedResource(cachedImageInteriorCrop.get(), UpdateForPropertyChange);
     memoryCache()->updateDecodedResource(cachedImageExteriorCrop.get(), UpdateForPropertyChange);
     memoryCache()->updateDecodedResource(cachedImageOutsideCrop.get(), UpdateForPropertyChange);
 
     // HTMLImageElements should default to CacheLiveResourcePriorityLow.
     ASSERT_EQ(memoryCache()->priority(imageNoCrop->cachedImage()), MemoryCacheLiveResourcePriorityLow);
     ASSERT_EQ(memoryCache()->priority(imageInteriorCrop->cachedImage()), MemoryCacheLiveResourcePriorityLow);
     ASSERT_EQ(memoryCache()->priority(imageExteriorCrop->cachedImage()), MemoryCacheLiveResourcePriorityLow);
     ASSERT_EQ(memoryCache()->priority(imageOutsideCrop->cachedImage()), MemoryCacheLiveResourcePriorityLow);
 
-    RefPtrWillBeRawPtr<ImageBitmap> imageBitmapInteriorCrop = ImageBitmap::create(imageInteriorCrop.get(), IntRect(m_bitmap.width() / 2, m_bitmap.height() / 2, m_bitmap.width(), m_bitmap.height()));
+    RefPtrWillBePersistent<ImageBitmap> imageBitmapInteriorCrop = ImageBitmap::create(imageInteriorCrop.get(), IntRect(m_bitmap.width() / 2, m_bitmap.height() / 2, m_bitmap.width(), m_bitmap.height()));
     {
         RefPtrWillBeRawPtr<ImageBitmap> imageBitmapNoCrop = ImageBitmap::create(imageNoCrop.get(), IntRect(0, 0, m_bitmap.width(), m_bitmap.height()));
         RefPtrWillBeRawPtr<ImageBitmap> imageBitmapInteriorCrop2 = ImageBitmap::create(imageInteriorCrop.get(), IntRect(m_bitmap.width() / 2, m_bitmap.height() / 2, m_bitmap.width(), m_bitmap.height()));
         RefPtrWillBeRawPtr<ImageBitmap> imageBitmapExteriorCrop = ImageBitmap::create(imageExteriorCrop.get(), IntRect(-m_bitmap.width() / 2, -m_bitmap.height() / 2, m_bitmap.width(), m_bitmap.height()));
         RefPtrWillBeRawPtr<ImageBitmap> imageBitmapOutsideCrop = ImageBitmap::create(imageOutsideCrop.get(), IntRect(-m_bitmap.width(), -m_bitmap.height(), m_bitmap.width(), m_bitmap.height()));
 
         // Images that are referenced by ImageBitmaps have CacheLiveResourcePriorityHigh.
         ASSERT_EQ(memoryCache()->priority(imageNoCrop->cachedImage()), MemoryCacheLiveResourcePriorityHigh);
         ASSERT_EQ(memoryCache()->priority(imageInteriorCrop->cachedImage()), MemoryCacheLiveResourcePriorityHigh);
         ASSERT_EQ(memoryCache()->priority(imageExteriorCrop->cachedImage()), MemoryCacheLiveResourcePriorityHigh);
 
         // ImageBitmaps that do not contain any of the source image do not elevate CacheLiveResourcePriority.
         ASSERT_EQ(memoryCache()->priority(imageOutsideCrop->cachedImage()), MemoryCacheLiveResourcePriorityLow);
 
-        // Stub out references to the ImageBitmaps created and force a
-        // garbage collection to have the ImageBitmaps be collected and
-        // destructed.
-        imageBitmapNoCrop = nullptr;
-        imageBitmapInteriorCrop2 = nullptr;
-        imageBitmapExteriorCrop = nullptr;
-        imageBitmapOutsideCrop = nullptr;
-        Heap::collectGarbage(ThreadState::HeapPointersOnStack);
+        // Force a garbage collection to sweep out the local ImageBitmaps.
+        Heap::collectGarbage(ThreadState::NoHeapPointersOnStack);

[haraken, 2014/05/19 13:28:02]

hmm, I'm not quite sure if this change addresses the test failure.

Currently there is a raw pointer on stack, but we are using
Heap::collectGarbage(HeapPointersOnStack) to prevent the on-stack raw pointer
from getting collected. In other words, the on-stack raw pointer shouldn't be
collected by the Heap::collectGarbage. If this observation is correct, this CL
won't solve the problem...

[sof, 2014/05/19 13:31:30]

The only object reference we care about is a Persistent outside this block, so
why would it not work out?

[wibling-chromium, 2014/05/19 13:34:36]

As I understand it the assumption was that doing a conservative GC could keep
the a dead imageBitmap alive. By not doing the conservative GC this should not
happen.

[haraken, 2014/05/19 13:39:29]

Sorry, you're right. It might work.

I thought that the issue is that before this CL, the on-stack raw pointer is
prematurely collected by Heap::collectGarbage. However, it cannot happen because
the Heap::collectGarbage is called with the HeapPointersOnStack option, which
does conservative scanning and keeps the on-stack raw pointer alive.

However, I understand that the issue can exist somewhere else. For example, the
issue can be that the conservative scanning of the Heap::collectGarbage
conservatively protects things that doesn't need to be protected and it can lead
to the failure. If so, it's possible that this change fixes the issue.

Nit: You can use Heap::collectAllGarbage() instead.

[Mads Ager (chromium), 2014/05/19 13:52:46]

Consider moving this to right after the block instead. That way it is impossible
to start operating on the raw pointers inside of the block after they have been
collected.

[sof, 2014/05/19 14:12:57]

ok, done that (also had to do some ALLOW_UNUSED decorations to get it to compile
across platforms.)

haraken: I did consider using collectAllGarbage(), but it seemed a bit too
expensive when wanting to flush just 4/5 objects. Hope that's ok.

     }
 
     // CacheLiveResourcePriroity should return to CacheLiveResourcePriorityLow when no ImageBitmaps reference the image.
     ASSERT_EQ(memoryCache()->priority(imageNoCrop->cachedImage()), MemoryCacheLiveResourcePriorityLow);
     ASSERT_EQ(memoryCache()->priority(imageExteriorCrop->cachedImage()), MemoryCacheLiveResourcePriorityLow);
     ASSERT_EQ(memoryCache()->priority(imageOutsideCrop->cachedImage()), MemoryCacheLiveResourcePriorityLow);
 
     // There is still an ImageBitmap that references this image.
     ASSERT_EQ(memoryCache()->priority(imageInteriorCrop->cachedImage()), MemoryCacheLiveResourcePriorityHigh);
     imageBitmapInteriorCrop = nullptr;
@@ skipping 32 matching lines @@
     {
         RefPtrWillBeRawPtr<ImageBitmap> imageBitmapFromCanvas = ImageBitmap::create(canvasElement.get(), IntRect(0, 0, canvasElement->width(), canvasElement->height()));
         imageBitmapDerived = ImageBitmap::create(imageBitmapFromCanvas.get(), IntRect(0, 0, 20, 20));
     }
     CanvasRenderingContext* context = canvasElement->getContext("2d");
     TrackExceptionState exceptionState;
     toCanvasRenderingContext2D(context)->drawImage(imageBitmapDerived.get(), 0, 0, exceptionState);
 }
 
 } // namespace