CSSStyleSheet members should clear their owner node when destroyed
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Jul 2016 21:01:23 +0000 (21:01 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Jul 2016 21:01:23 +0000 (21:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=117470

Reviewed by Chris Dumez.

Make sure that CSSStyleSheet members are detached from their owner node when
the owning object is destroyed.

I audited other CSSStyleSheet uses, and found one other place where the owner node was not
being cleared during destruction. The Inspector also uses CSSStyleSheet, but seems to
handle the node ownership properly.

Fix based on a Blink change (patch by <haraken@chromium.org>):
<https://chromium.googlesource.com/chromium/blink/+/c4949bfdeb2a613701afa1410bdae70531b8f6bf>

Also includes a follow-up fix (patch by <haraken@chromium.org>):
<https://chromium.googlesource.com/chromium/blink/+/9c3932dc80b33429db3a5873cb266b726c8a19bf>

No test case. Was found by the Chromium team through review of their crash traces under minor DOM GC.

* contentextensions/ContentExtensionStyleSheet.cpp:
(WebCore::ContentExtensions::ContentExtensionStyleSheet::~ContentExtensionStyleSheet):
* contentextensions/ContentExtensionStyleSheet.h:
* dom/InlineStyleSheetOwner.cpp:
(WebCore::InlineStyleSheetOwner::~InlineStyleSheetOwner):
(WebCore::authorStyleSheetsForElement):

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

Source/WebCore/ChangeLog
Source/WebCore/contentextensions/ContentExtensionStyleSheet.cpp
Source/WebCore/contentextensions/ContentExtensionStyleSheet.h
Source/WebCore/dom/InlineStyleSheetOwner.cpp

index 8505ca5..124aac0 100644 (file)
@@ -1,3 +1,32 @@
+2016-07-13  Brent Fulgham  <bfulgham@apple.com>
+
+        CSSStyleSheet members should clear their owner node when destroyed
+        https://bugs.webkit.org/show_bug.cgi?id=117470
+
+        Reviewed by Chris Dumez.
+
+        Make sure that CSSStyleSheet members are detached from their owner node when
+        the owning object is destroyed.
+
+        I audited other CSSStyleSheet uses, and found one other place where the owner node was not
+        being cleared during destruction. The Inspector also uses CSSStyleSheet, but seems to
+        handle the node ownership properly.
+
+        Fix based on a Blink change (patch by <haraken@chromium.org>):
+        <https://chromium.googlesource.com/chromium/blink/+/c4949bfdeb2a613701afa1410bdae70531b8f6bf>
+
+        Also includes a follow-up fix (patch by <haraken@chromium.org>):
+        <https://chromium.googlesource.com/chromium/blink/+/9c3932dc80b33429db3a5873cb266b726c8a19bf>
+
+        No test case. Was found by the Chromium team through review of their crash traces under minor DOM GC.
+
+        * contentextensions/ContentExtensionStyleSheet.cpp:
+        (WebCore::ContentExtensions::ContentExtensionStyleSheet::~ContentExtensionStyleSheet):
+        * contentextensions/ContentExtensionStyleSheet.h:
+        * dom/InlineStyleSheetOwner.cpp:
+        (WebCore::InlineStyleSheetOwner::~InlineStyleSheetOwner):
+        (WebCore::authorStyleSheetsForElement):
+
 2016-07-14  Csaba Osztrogon√°c  <ossy@webkit.org>
 
         Fix the !ENABLE(WEB_SOCKETS) build after r202930
index d312677..44d7c57 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -43,6 +43,11 @@ ContentExtensionStyleSheet::ContentExtensionStyleSheet(Document& document)
     m_styleSheet->contents().setIsUserStyleSheet(true);
 }
 
+ContentExtensionStyleSheet::~ContentExtensionStyleSheet()
+{
+    m_styleSheet->clearOwnerNode();
+}
+
 bool ContentExtensionStyleSheet::addDisplayNoneSelector(const String& selector, uint32_t selectorID)
 {
     ASSERT(selectorID != std::numeric_limits<uint32_t>::max());
index a503b58..5144076 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -45,6 +45,7 @@ public:
     {
         return adoptRef(*new ContentExtensionStyleSheet(document));
     }
+    virtual ~ContentExtensionStyleSheet();
 
     bool addDisplayNoneSelector(const String& selector, uint32_t selectorID);
 
index e006b81..6ba9388 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2006, 2007 Rob Buis
- * Copyright (C) 2008, 2013 Apple, Inc. All rights reserved.
+ * Copyright (C) 2008-2016 Apple, Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -46,6 +46,8 @@ InlineStyleSheetOwner::InlineStyleSheetOwner(Document& document, bool createdByP
 
 InlineStyleSheetOwner::~InlineStyleSheetOwner()
 {
+    if (m_sheet)
+        clearSheet();
 }
 
 static AuthorStyleSheets& authorStyleSheetsForElement(Element& element)