CacheQueryOptions::isolatedCopy() copies the cache name twice
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Aug 2017 18:08:54 +0000 (18:08 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 Aug 2017 18:08:54 +0000 (18:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175974

Reviewed by Youenn Fablet.

Currently CacheQueryOptions has a user-defined constructor that calls String.isolatedCopy()
on the passed cache name. CacheQueryOptions::isolatedCopy() also calls String.isolatedCopy()
on the cache name before passing the result to the user-defined constructor; => we malloc
and copy the cache name twice. Ideally we would remove the user-defined constructors and
have callers use aggregate initializer syntax to instantiate a CacheQueryOptions. Unfortunately
we cannot do this until we upgrade from Visual Studio 2015 to Visual Studio 2017 as the former
does not support non-static data member initializers (NSDMI) for aggregates and CacheQueryOptions
has some. Therefore we modify the user-defined, non-default, constructor to take a String&&
and conditionally compile the the constructors when building with compilers that do not
support NSDMI for aggregates.

* Modules/cache/CacheQueryOptions.h:
(WebCore::CacheQueryOptions::CacheQueryOptions):

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

Source/WebCore/ChangeLog
Source/WebCore/Modules/cache/CacheQueryOptions.h

index 1f88083..8180b5e 100644 (file)
@@ -1,3 +1,24 @@
+2017-08-29  Daniel Bates  <dabates@apple.com>
+
+        CacheQueryOptions::isolatedCopy() copies the cache name twice
+        https://bugs.webkit.org/show_bug.cgi?id=175974
+
+        Reviewed by Youenn Fablet.
+
+        Currently CacheQueryOptions has a user-defined constructor that calls String.isolatedCopy()
+        on the passed cache name. CacheQueryOptions::isolatedCopy() also calls String.isolatedCopy()
+        on the cache name before passing the result to the user-defined constructor; => we malloc
+        and copy the cache name twice. Ideally we would remove the user-defined constructors and
+        have callers use aggregate initializer syntax to instantiate a CacheQueryOptions. Unfortunately
+        we cannot do this until we upgrade from Visual Studio 2015 to Visual Studio 2017 as the former
+        does not support non-static data member initializers (NSDMI) for aggregates and CacheQueryOptions
+        has some. Therefore we modify the user-defined, non-default, constructor to take a String&&
+        and conditionally compile the the constructors when building with compilers that do not
+        support NSDMI for aggregates.
+
+        * Modules/cache/CacheQueryOptions.h:
+        (WebCore::CacheQueryOptions::CacheQueryOptions):
+
 2017-08-29  Youenn Fablet  <youenn@apple.com>
 
         CanvasCaptureMediaStreamTrack clone is not a CanvasCaptureMediaStreamTrack
index 7427b8d..35c4775 100644 (file)
 namespace WebCore {
 
 struct CacheQueryOptions {
+#if !COMPILER_SUPPORTS(NSDMI_FOR_AGGREGATES)
     CacheQueryOptions() = default;
-    CacheQueryOptions(bool ignoreSearch, bool ignoreMethod, bool ignoreVary, String cacheName);
+    CacheQueryOptions(bool ignoreSearch, bool ignoreMethod, bool ignoreVary, String&& cacheName)
+        : ignoreSearch { ignoreSearch }
+        , ignoreMethod { ignoreMethod }
+        , ignoreVary { ignoreVary }
+        , cacheName { WTFMove(cacheName) }
+    {
+    }
+#endif
     CacheQueryOptions isolatedCopy() const { return { ignoreSearch, ignoreMethod, ignoreVary, cacheName.isolatedCopy() }; }
 
     bool ignoreSearch { false };
@@ -40,12 +48,4 @@ struct CacheQueryOptions {
     String cacheName;
 };
 
-inline CacheQueryOptions::CacheQueryOptions(bool ignoreSearch, bool ignoreMethod, bool ignoreVary, String cacheName)
-    : ignoreSearch(ignoreSearch)
-    , ignoreMethod(ignoreMethod)
-    , ignoreVary(ignoreVary)
-    , cacheName(cacheName.isolatedCopy())
-{
-}
-
 } // namespace WebCore