Have class Exception take String by value instead of a String&&
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Apr 2018 17:45:57 +0000 (17:45 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Apr 2018 17:45:57 +0000 (17:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184360

Reviewed by Alexey Proskuryakov.

For convenience support instantiating an Exception with either an lvalue String or
rvalue String.

Although it can be argued that having Exception take a String by value instead of String&&
can lead to missed opportunities to WTFMove() a String object into Exception such mistakes
are just that, missed opportunities. That is, correctness is not affected and we may perform
an unnecessary ref/deref of the underlying StringImpl when instantiating an Exception. If
such missed opportunities show up in profiles and such mistakes happen often then we can
re-evaluate the decision to have Exception take a String by value.

* Modules/cache/DOMCache.cpp:
(WebCore::DOMCache::put): Simplify code now that Exception takes a String by value.
* Modules/fetch/FetchResponse.cpp:
(WebCore::FetchResponse::BodyLoader::didFail): Ditto.
* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
(WebCore::LibWebRTCMediaEndpoint::createSessionDescriptionFailed): Move String into Exception to avoid an
unnecessary ref/de-ref.
(WebCore::LibWebRTCMediaEndpoint::setLocalSessionDescriptionFailed): Ditto.
(WebCore::LibWebRTCMediaEndpoint::setRemoteSessionDescriptionFailed): Ditto.
* dom/Exception.h:
(WebCore::Exception::Exception): Take String by value. Also use uniform initializer syntax.

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

Source/WebCore/ChangeLog
Source/WebCore/Modules/cache/DOMCache.cpp
Source/WebCore/Modules/fetch/FetchResponse.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
Source/WebCore/dom/Exception.h

index d3718a1..55c8a64 100644 (file)
@@ -1,3 +1,32 @@
+2018-04-06  Daniel Bates  <dabates@apple.com>
+
+        Have class Exception take String by value instead of a String&&
+        https://bugs.webkit.org/show_bug.cgi?id=184360
+
+        Reviewed by Alexey Proskuryakov.
+
+        For convenience support instantiating an Exception with either an lvalue String or
+        rvalue String.
+
+        Although it can be argued that having Exception take a String by value instead of String&&
+        can lead to missed opportunities to WTFMove() a String object into Exception such mistakes
+        are just that, missed opportunities. That is, correctness is not affected and we may perform
+        an unnecessary ref/deref of the underlying StringImpl when instantiating an Exception. If
+        such missed opportunities show up in profiles and such mistakes happen often then we can
+        re-evaluate the decision to have Exception take a String by value.
+
+        * Modules/cache/DOMCache.cpp:
+        (WebCore::DOMCache::put): Simplify code now that Exception takes a String by value.
+        * Modules/fetch/FetchResponse.cpp:
+        (WebCore::FetchResponse::BodyLoader::didFail): Ditto.
+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
+        (WebCore::LibWebRTCMediaEndpoint::createSessionDescriptionFailed): Move String into Exception to avoid an
+        unnecessary ref/de-ref.
+        (WebCore::LibWebRTCMediaEndpoint::setLocalSessionDescriptionFailed): Ditto.
+        (WebCore::LibWebRTCMediaEndpoint::setRemoteSessionDescriptionFailed): Ditto.
+        * dom/Exception.h:
+        (WebCore::Exception::Exception): Take String by value. Also use uniform initializer syntax.
+
 2018-04-06  Antti Koivisto  <antti@apple.com>
 
         Tighten ImageSource to have BitmapImage pointer instead of Image
index 45c5ee6..98d62ad 100644 (file)
@@ -321,7 +321,7 @@ void DOMCache::put(RequestInfo&& info, Ref<FetchResponse>&& response, DOMPromise
     auto request = requestOrException.releaseReturnValue();
 
     if (response->loadingError()) {
-        promise.reject(Exception { TypeError, String { response->loadingError()->localizedDescription() } });
+        promise.reject(Exception { TypeError, response->loadingError()->localizedDescription() });
         return;
     }
 
index 7de58da..d96c7bb 100644 (file)
@@ -241,10 +241,10 @@ void FetchResponse::BodyLoader::didFail(const ResourceError& error)
     m_response.m_loadingError = error;
 
     if (auto responseCallback = WTFMove(m_responseCallback))
-        responseCallback(Exception { TypeError, String(error.localizedDescription()) });
+        responseCallback(Exception { TypeError, error.localizedDescription() });
 
     if (auto consumeDataCallback = WTFMove(m_consumeDataCallback))
-        consumeDataCallback(Exception { TypeError, String(error.localizedDescription()) });
+        consumeDataCallback(Exception { TypeError, error.localizedDescription() });
 
 #if ENABLE(STREAMS_API)
     if (m_response.m_readableStreamSource) {
index 9f645ed..b015789 100644 (file)
@@ -914,9 +914,9 @@ void LibWebRTCMediaEndpoint::createSessionDescriptionFailed(const std::string& e
         if (protectedThis->isStopped())
             return;
         if (protectedThis->m_isInitiator)
-            protectedThis->m_peerConnectionBackend.createOfferFailed(Exception { OperationError, String(error) });
+            protectedThis->m_peerConnectionBackend.createOfferFailed(Exception { OperationError, WTFMove(error) });
         else
-            protectedThis->m_peerConnectionBackend.createAnswerFailed(Exception { OperationError, String(error) });
+            protectedThis->m_peerConnectionBackend.createAnswerFailed(Exception { OperationError, WTFMove(error) });
     });
 }
 
@@ -935,7 +935,7 @@ void LibWebRTCMediaEndpoint::setLocalSessionDescriptionFailed(const std::string&
     callOnMainThread([protectedThis = makeRef(*this), error = WTFMove(error)] {
         if (protectedThis->isStopped())
             return;
-        protectedThis->m_peerConnectionBackend.setLocalDescriptionFailed(Exception { OperationError, String(error) });
+        protectedThis->m_peerConnectionBackend.setLocalDescriptionFailed(Exception { OperationError, WTFMove(error) });
     });
 }
 
@@ -954,7 +954,7 @@ void LibWebRTCMediaEndpoint::setRemoteSessionDescriptionFailed(const std::string
     callOnMainThread([protectedThis = makeRef(*this), error = WTFMove(error)] {
         if (protectedThis->isStopped())
             return;
-        protectedThis->m_peerConnectionBackend.setRemoteDescriptionFailed(Exception { OperationError, String(error) });
+        protectedThis->m_peerConnectionBackend.setRemoteDescriptionFailed(Exception { OperationError, WTFMove(error) });
     });
 }
 
index 097b917..e1e7adb 100644 (file)
@@ -33,7 +33,7 @@ namespace WebCore {
 
 class Exception {
 public:
-    explicit Exception(ExceptionCode, String&& = { });
+    explicit Exception(ExceptionCode, String = { });
 
     ExceptionCode code() const { return m_code; }
     const String& message() const { return m_message; }
@@ -51,9 +51,9 @@ private:
 
 Exception isolatedCopy(Exception&&);
 
-inline Exception::Exception(ExceptionCode code, String&& message)
-    : m_code(code)
-    , m_message(WTFMove(message))
+inline Exception::Exception(ExceptionCode code, String message)
+    : m_code { code }
+    , m_message { WTFMove(message) }
 {
 }