Changeset 211360 in webkit

Timestamp:

Jan 30, 2017 2:14:36 AM (7 years ago)

Author:

Carlos Garcia Campos

Message:

Several web timing tests crash in GTK+ and AppleWin bots
​https://bugs.webkit.org/show_bug.cgi?id=167577

Reviewed by Ryosuke Niwa.

The problem is that entry is used in both the key, to get name, and in the value with WTFMove. So, the name is
invalidated by the move. It could be fixed by simply copying the name, instead of using entry->name, but I think
that code could be simplified using HashMap::ensure and then we don't need any string copy, nor even the static
insertPerformanceEntry().

Fix crashes in several imported/w3c/web-platform-tests/user-timing/ tests.

• page/PerformanceUserTiming.cpp:

(WebCore::UserTiming::mark):
(WebCore::UserTiming::measure):
(WebCore::insertPerformanceEntry): Deleted.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• page/PerformanceUserTiming.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-01-30  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        Several web timing tests crash in GTK+ and AppleWin bots
+        https://bugs.webkit.org/show_bug.cgi?id=167577
+
+        Reviewed by Ryosuke Niwa.
+
+        The problem is that entry is used in both the key, to get name, and in the value with WTFMove. So, the name is
+        invalidated by the move. It could be fixed by simply copying the name, instead of using entry->name, but I think
+        that code could be simplified using HashMap::ensure and then we don't need any string copy, nor even the static
+        insertPerformanceEntry().
+
+        Fix crashes in several imported/w3c/web-platform-tests/user-timing/ tests.
+
+        * page/PerformanceUserTiming.cpp:
+        (WebCore::UserTiming::mark):
+        (WebCore::UserTiming::measure):
+        (WebCore::insertPerformanceEntry): Deleted.
+
 2017-01-30  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Source/WebCore/page/PerformanceUserTiming.cpp

@@ -89 +89 @@
 }
 
-static void insertPerformanceEntry(PerformanceEntryMap& performanceEntryMap, Ref<PerformanceEntry>&& performanceEntry)
-{
-    RefPtr<PerformanceEntry> entry = WTFMove(performanceEntry);
-    auto it = performanceEntryMap.find(entry->name());
-    if (it != performanceEntryMap.end())
-        it->value.append(WTFMove(entry));
-    else
-        performanceEntryMap.set(entry->name(), Vector<RefPtr<PerformanceEntry>> { WTFMove(entry) });
-}
-
 static void clearPerformanceEntries(PerformanceEntryMap& performanceEntryMap, const String& name)
 {
@@ -114 +104 @@
         return Exception { SYNTAX_ERR };
 
-    insertPerformanceEntry(m_marksMap, PerformanceMark::create(markName, m_performance.now()));
+    auto& performanceEntryList = m_marksMap.ensure(markName, [] { return Vector<RefPtr<PerformanceEntry>>(); }).iterator->value;
+    performanceEntryList.append(PerformanceMark::create(markName, m_performance.now()));
+
     return { };
 }
@@ -162 +154 @@
     }
 
-    insertPerformanceEntry(m_measuresMap, PerformanceMeasure::create(measureName, startTime, endTime));
+    auto& performanceEntryList = m_measuresMap.ensure(measureName, [] { return Vector<RefPtr<PerformanceEntry>>(); }).iterator->value;
+    performanceEntryList.append(PerformanceMeasure::create(measureName, startTime, endTime));
+
     return { };
 }