REGRESSION(r227594) [WinCairo] NULL pointer crash in GraphicsContext::getWindowsContext
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Feb 2018 00:16:50 +0000 (00:16 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Feb 2018 00:16:50 +0000 (00:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182282

Patch by Fujii Hironori <Hironori.Fujii@sony.com> on 2018-02-01
Reviewed by Žan Doberšek.

ImageBufferCairo has been changed to use GraphicsContextImplCairo
in r227594. But, GraphicsContext::getWindowsContext doesn't care
the case of using GraphicsContextImpl and crashes due to null
dereference of GraphicsContext::m_data.

GraphicsContext::getWindowsContext should create a HDC in that case.

Remove the argument mayCreateBitmap because it is always
true at the moment.

No new tests (Covered by the existing tests)

* platform/graphics/GraphicsContext.h:
Removed a argument mayCreateBitmap of getWindowsContext and releaseWindowsContext.
* platform/graphics/win/GraphicsContextCGWin.cpp:
(WebCore::GraphicsContext::releaseWindowsContext): Ditto.
* platform/graphics/win/GraphicsContextCairoWin.cpp:
(WebCore::GraphicsContext::releaseWindowsContext): Ditto.
* platform/graphics/win/GraphicsContextDirect2D.cpp:
(WebCore::GraphicsContext::releaseWindowsContext): Ditto.
* platform/graphics/win/GraphicsContextWin.cpp:
(WebCore::GraphicsContext::getWindowsContext):
Create a HDC if m_impl is null. Removed a argument mayCreateBitmap.
* platform/graphics/win/LocalWindowsContext.h:
(WebCore::LocalWindowsContext::LocalWindowsContext):
Removed m_mayCreateBitmap.
(WebCore::LocalWindowsContext::~LocalWindowsContext): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@227988 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/GraphicsContext.h
Source/WebCore/platform/graphics/win/GraphicsContextCGWin.cpp
Source/WebCore/platform/graphics/win/GraphicsContextCairoWin.cpp
Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp
Source/WebCore/platform/graphics/win/GraphicsContextWin.cpp
Source/WebCore/platform/graphics/win/LocalWindowsContext.h

index 6eba4c6..d3d11ac 100644 (file)
@@ -1,3 +1,38 @@
+2018-02-01  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        REGRESSION(r227594) [WinCairo] NULL pointer crash in GraphicsContext::getWindowsContext
+        https://bugs.webkit.org/show_bug.cgi?id=182282
+
+        Reviewed by Žan Doberšek.
+
+        ImageBufferCairo has been changed to use GraphicsContextImplCairo
+        in r227594. But, GraphicsContext::getWindowsContext doesn't care
+        the case of using GraphicsContextImpl and crashes due to null
+        dereference of GraphicsContext::m_data.
+
+        GraphicsContext::getWindowsContext should create a HDC in that case.
+
+        Remove the argument mayCreateBitmap because it is always
+        true at the moment.
+
+        No new tests (Covered by the existing tests)
+
+        * platform/graphics/GraphicsContext.h:
+        Removed a argument mayCreateBitmap of getWindowsContext and releaseWindowsContext.
+        * platform/graphics/win/GraphicsContextCGWin.cpp:
+        (WebCore::GraphicsContext::releaseWindowsContext): Ditto.
+        * platform/graphics/win/GraphicsContextCairoWin.cpp:
+        (WebCore::GraphicsContext::releaseWindowsContext): Ditto.
+        * platform/graphics/win/GraphicsContextDirect2D.cpp:
+        (WebCore::GraphicsContext::releaseWindowsContext): Ditto.
+        * platform/graphics/win/GraphicsContextWin.cpp:
+        (WebCore::GraphicsContext::getWindowsContext):
+        Create a HDC if m_impl is null. Removed a argument mayCreateBitmap.
+        * platform/graphics/win/LocalWindowsContext.h:
+        (WebCore::LocalWindowsContext::LocalWindowsContext):
+        Removed m_mayCreateBitmap.
+        (WebCore::LocalWindowsContext::~LocalWindowsContext): Ditto.
+
 2018-02-01  Christopher Reid  <chris.reid@sony.com>
 
         [Curl] Use SQLite database in cookie jar implementation for Curl port
index 579b0e2..9f0ca1e 100644 (file)
@@ -497,8 +497,8 @@ public:
     FloatSize scaleFactorForDrawing(const FloatRect& destRect, const FloatRect& srcRect) const;
 
 #if OS(WINDOWS)
-    HDC getWindowsContext(const IntRect&, bool supportAlphaBlend, bool mayCreateBitmap); // The passed in rect is used to create a bitmap for compositing inside transparency layers.
-    void releaseWindowsContext(HDC, const IntRect&, bool supportAlphaBlend, bool mayCreateBitmap); // The passed in HDC should be the one handed back by getWindowsContext.
+    HDC getWindowsContext(const IntRect&, bool supportAlphaBlend); // The passed in rect is used to create a bitmap for compositing inside transparency layers.
+    void releaseWindowsContext(HDC, const IntRect&, bool supportAlphaBlend); // The passed in HDC should be the one handed back by getWindowsContext.
     HDC hdc() const;
 #if PLATFORM(WIN)
 #if USE(WINGDI)
index ec4f7a9..c47884d 100644 (file)
@@ -92,9 +92,9 @@ void GraphicsContext::platformInit(HDC hdc, bool hasAlpha)
 
 // FIXME: Is it possible to merge getWindowsContext and createWindowsBitmap into a single API
 // suitable for all clients?
-void GraphicsContext::releaseWindowsContext(HDC hdc, const IntRect& dstRect, bool supportAlphaBlend, bool mayCreateBitmap)
+void GraphicsContext::releaseWindowsContext(HDC hdc, const IntRect& dstRect, bool supportAlphaBlend)
 {
-    bool createdBitmap = mayCreateBitmap && (!m_data->m_hdc || isInTransparencyLayer());
+    bool createdBitmap = m_impl || !m_data->m_hdc || isInTransparencyLayer();
     if (!createdBitmap) {
         m_data->restore();
         return;
index 5647d88..2cd5223 100644 (file)
@@ -126,9 +126,9 @@ static void drawBitmapToContext(PlatformContextCairo& platformContext, const DIB
     cairo_restore(cr);
 }
 
-void GraphicsContext::releaseWindowsContext(HDC hdc, const IntRect& dstRect, bool supportAlphaBlend, bool mayCreateBitmap)
+void GraphicsContext::releaseWindowsContext(HDC hdc, const IntRect& dstRect, bool supportAlphaBlend)
 {
-    bool createdBitmap = mayCreateBitmap && (!m_data->m_hdc || isInTransparencyLayer());
+    bool createdBitmap = m_impl || !m_data->m_hdc || isInTransparencyLayer();
     if (!hdc || !createdBitmap) {
         m_data->restore();
         return;
index 3aeb1db..010bfb3 100644 (file)
@@ -267,9 +267,9 @@ void GraphicsContext::drawNativeImage(const COMPtr<ID2D1Bitmap>& image, const Fl
         context->SetTransform(ctm);
 }
 
-void GraphicsContext::releaseWindowsContext(HDC hdc, const IntRect& dstRect, bool supportAlphaBlend, bool mayCreateBitmap)
+void GraphicsContext::releaseWindowsContext(HDC hdc, const IntRect& dstRect, bool supportAlphaBlend)
 {
-    bool createdBitmap = mayCreateBitmap && (!m_data->m_hdc || isInTransparencyLayer());
+    bool createdBitmap = m_impl || !m_data->m_hdc || isInTransparencyLayer();
     if (!createdBitmap) {
         m_data->restore();
         return;
index 49a2103..72b1347 100644 (file)
@@ -100,10 +100,13 @@ std::unique_ptr<GraphicsContext::WindowsBitmap> GraphicsContext::createWindowsBi
 }
 #endif
 
-HDC GraphicsContext::getWindowsContext(const IntRect& dstRect, bool supportAlphaBlend, bool mayCreateBitmap)
+HDC GraphicsContext::getWindowsContext(const IntRect& dstRect, bool supportAlphaBlend)
 {
+    HDC hdc = nullptr;
+    if (!m_impl)
+        hdc = m_data->m_hdc;
     // FIXME: Should a bitmap be created also when a shadow is set?
-    if (mayCreateBitmap && (!m_data->m_hdc || isInTransparencyLayer())) {
+    if (!hdc || isInTransparencyLayer()) {
         if (dstRect.isEmpty())
             return 0;
 
@@ -115,7 +118,7 @@ HDC GraphicsContext::getWindowsContext(const IntRect& dstRect, bool supportAlpha
         if (!bitmap)
             return 0;
 
-        auto bitmapDC = adoptGDIObject(::CreateCompatibleDC(m_data->m_hdc));
+        auto bitmapDC = adoptGDIObject(::CreateCompatibleDC(hdc));
         ::SelectObject(bitmapDC.get(), bitmap);
 
         // Fill our buffer with clear if we're going to alpha blend.
index 3addc14..9911b65 100644 (file)
@@ -34,18 +34,17 @@ namespace WebCore {
 class LocalWindowsContext {
     WTF_MAKE_NONCOPYABLE(LocalWindowsContext);
 public:
-    LocalWindowsContext(GraphicsContext& graphicsContext, const IntRect& rect, bool supportAlphaBlend = true, bool mayCreateBitmap = true)
+    LocalWindowsContext(GraphicsContext& graphicsContext, const IntRect& rect, bool supportAlphaBlend = true)
         : m_graphicsContext(graphicsContext)
         , m_rect(rect)
         , m_supportAlphaBlend(supportAlphaBlend)
-        , m_mayCreateBitmap(mayCreateBitmap)
     {
-        m_hdc = m_graphicsContext.getWindowsContext(m_rect, m_supportAlphaBlend, m_mayCreateBitmap);
+        m_hdc = m_graphicsContext.getWindowsContext(m_rect, m_supportAlphaBlend);
     }
 
     ~LocalWindowsContext()
     {
-        m_graphicsContext.releaseWindowsContext(m_hdc, m_rect, m_supportAlphaBlend, m_mayCreateBitmap);
+        m_graphicsContext.releaseWindowsContext(m_hdc, m_rect, m_supportAlphaBlend);
     }
 
     HDC hdc() const { return m_hdc; }
@@ -55,7 +54,6 @@ private:
     HDC m_hdc;
     IntRect m_rect;
     bool m_supportAlphaBlend;
-    bool m_mayCreateBitmap;
 };
 
 } // namespace WebCore