Changeset 181453 in webkit

Timestamp:

Mar 12, 2015, 1:37:13 PM (10 years ago)

Author:

ggaren@apple.com

Message:

REGRESSION: Crash under Heap::reportExtraMemoryAllocatedSlowCase for media element
​https://bugs.webkit.org/show_bug.cgi?id=142636

Reviewed by Mark Hahnenberg.

This was a pre-existing bug that I made a lot worse in
<​https://trac.webkit.org/changeset/181411>.

• html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::parseAttribute): Compare size before
subtracting rather than subtracting and then comparing to zero. The
latter technique is not valid for unsigned integers, which will happily
underflow into giant numbers.

• Modules/mediasource/SourceBuffer.cpp:

(WebCore::SourceBuffer::reportExtraMemoryAllocated): This code was

technically correct, but I took the opportunity to clean it up a bit.
There's no need to do two checks here, and it smells bad to check for
a negative unsigned integer.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/mediasource/SourceBuffer.cpp (modified) (2 diffs)
• html/HTMLMediaElement.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-03-12  Geoffrey Garen  <ggaren@apple.com>
+
+        REGRESSION: Crash under Heap::reportExtraMemoryAllocatedSlowCase for media element
+        https://bugs.webkit.org/show_bug.cgi?id=142636
+
+        Reviewed by Mark Hahnenberg.
+
+        This was a pre-existing bug that I made a lot worse in
+        <https://trac.webkit.org/changeset/181411>.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::parseAttribute): Compare size before
+        subtracting rather than subtracting and then comparing to zero. The
+        latter technique is not valid for unsigned integers, which will happily
+        underflow into giant numbers.
+
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::reportExtraMemoryAllocated): This code was
+         technically correct, but I took the opportunity to clean it up a bit.
+         There's no need to do two checks here, and it smells bad to check for
+         a negative unsigned integer.
+
 2015-03-12  Sebastian Dröge  <sebastian@centricular.com>
 

trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp

@@ -1992 +1992 @@
 {
     size_t extraMemoryCost = this->extraMemoryCost();
-    if (extraMemoryCost < m_reportedExtraMemoryCost)
+    if (extraMemoryCost <= m_reportedExtraMemoryCost)
         return;
 
@@ -1999 +1999 @@
 
     JSC::JSLockHolder lock(scriptExecutionContext()->vm());
-    if (extraMemoryCostDelta > 0) {
-        // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
-        // https://bugs.webkit.org/show_bug.cgi?id=142595
-        scriptExecutionContext()->vm().heap.deprecatedReportExtraMemory(extraMemoryCostDelta);
-    }
+    // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
+    // https://bugs.webkit.org/show_bug.cgi?id=142595
+    scriptExecutionContext()->vm().heap.deprecatedReportExtraMemory(extraMemoryCostDelta);
 }
 

trunk/Source/WebCore/html/HTMLMediaElement.cpp

@@ -634 +634 @@
 
         if (m_player) {
-            JSC::VM& vm = JSDOMWindowBase::commonVM();
-            JSC::JSLockHolder lock(vm);
-
             size_t extraMemoryCost = m_player->extraMemoryCost();
-            size_t extraMemoryCostDelta = extraMemoryCost - m_reportedExtraMemoryCost;
-            m_reportedExtraMemoryCost = extraMemoryCost;
-
-            if (extraMemoryCostDelta > 0) {
+            if (extraMemoryCost > m_reportedExtraMemoryCost) {
+                JSC::VM& vm = JSDOMWindowBase::commonVM();
+                JSC::JSLockHolder lock(vm);
+
+                size_t extraMemoryCostDelta = extraMemoryCost - m_reportedExtraMemoryCost;
+                m_reportedExtraMemoryCost = extraMemoryCost;
                 // FIXME: Adopt reportExtraMemoryVisited, and switch to reportExtraMemoryAllocated.
                 // https://bugs.webkit.org/show_bug.cgi?id=142595