Changeset 230338 in webkit

Timestamp:

Apr 6, 2018 10:45:57 AM (6 years ago)

Author:

dbates@webkit.org

Message:

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/cache/DOMCache.cpp (modified) (1 diff)
• Modules/fetch/FetchResponse.cpp (modified) (1 diff)
• Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp (modified) (3 diffs)
• dom/Exception.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/Modules/cache/DOMCache.cpp

@@ -322 +322 @@
 
     if (response->loadingError()) {
-        promise.reject(Exception { TypeError, String { response->loadingError()->localizedDescription() } });
+        promise.reject(Exception { TypeError, response->loadingError()->localizedDescription() });
         return;
     }

trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp

@@ -242 +242 @@
 
     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)

trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp

@@ -915 +915 @@
             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) });
     });
 }
@@ -936 +936 @@
         if (protectedThis->isStopped())
             return;
-        protectedThis->m_peerConnectionBackend.setLocalDescriptionFailed(Exception { OperationError, String(error) });
+        protectedThis->m_peerConnectionBackend.setLocalDescriptionFailed(Exception { OperationError, WTFMove(error) });
     });
 }
@@ -955 +955 @@
         if (protectedThis->isStopped())
             return;
-        protectedThis->m_peerConnectionBackend.setRemoteDescriptionFailed(Exception { OperationError, String(error) });
+        protectedThis->m_peerConnectionBackend.setRemoteDescriptionFailed(Exception { OperationError, WTFMove(error) });
     });
 }

trunk/Source/WebCore/dom/Exception.h

@@ -34 +34 @@
 class Exception {
 public:
-    explicit Exception(ExceptionCode, String&& = { });
+    explicit Exception(ExceptionCode, String = { });
 
     ExceptionCode code() const { return m_code; }
@@ -52 +52 @@
 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) }
 {
 }