Oilpan: simplify ImageBitmapTest heap clearing.

Source/core/frame/ImageBitmapTest.cpp

Index: Source/core/frame/ImageBitmapTest.cpp
diff --git a/Source/core/frame/ImageBitmapTest.cpp b/Source/core/frame/ImageBitmapTest.cpp
index fecdbca702074224c7fe378703f0d61e91d0742c..73bea955f84ebbbf01818bc7e36e5ace10125cee 100644
--- a/Source/core/frame/ImageBitmapTest.cpp
+++ b/Source/core/frame/ImageBitmapTest.cpp
@@ -146,7 +146,7 @@ TEST_F(ImageBitmapTest, ImageBitmapLiveResourcePriority)
     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()));
@@ -161,14 +161,8 @@ TEST_F(ImageBitmapTest, ImageBitmapLiveResourcePriority)
         // 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.