Title: [230338] trunk/Source/WebCore
Revision
230338
Author
dba...@webkit.org
Date
2018-04-06 10:45:57 -0700 (Fri, 06 Apr 2018)

Log 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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (230337 => 230338)


--- trunk/Source/WebCore/ChangeLog	2018-04-06 17:44:40 UTC (rev 230337)
+++ trunk/Source/WebCore/ChangeLog	2018-04-06 17:45:57 UTC (rev 230338)
@@ -1,3 +1,32 @@
+2018-04-06  Daniel Bates  <daba...@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  <an...@apple.com>
 
         Tighten ImageSource to have BitmapImage pointer instead of Image

Modified: trunk/Source/WebCore/Modules/cache/DOMCache.cpp (230337 => 230338)


--- trunk/Source/WebCore/Modules/cache/DOMCache.cpp	2018-04-06 17:44:40 UTC (rev 230337)
+++ trunk/Source/WebCore/Modules/cache/DOMCache.cpp	2018-04-06 17:45:57 UTC (rev 230338)
@@ -321,7 +321,7 @@
     auto request = requestOrException.releaseReturnValue();
 
     if (response->loadingError()) {
-        promise.reject(Exception { TypeError, String { response->loadingError()->localizedDescription() } });
+        promise.reject(Exception { TypeError, response->loadingError()->localizedDescription() });
         return;
     }
 

Modified: trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp (230337 => 230338)


--- trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp	2018-04-06 17:44:40 UTC (rev 230337)
+++ trunk/Source/WebCore/Modules/fetch/FetchResponse.cpp	2018-04-06 17:45:57 UTC (rev 230338)
@@ -241,10 +241,10 @@
     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) {

Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp (230337 => 230338)


--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp	2018-04-06 17:44:40 UTC (rev 230337)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp	2018-04-06 17:45:57 UTC (rev 230338)
@@ -914,9 +914,9 @@
         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 @@
     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 @@
     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) });
     });
 }
 

Modified: trunk/Source/WebCore/dom/Exception.h (230337 => 230338)


--- trunk/Source/WebCore/dom/Exception.h	2018-04-06 17:44:40 UTC (rev 230337)
+++ trunk/Source/WebCore/dom/Exception.h	2018-04-06 17:45:57 UTC (rev 230338)
@@ -33,7 +33,7 @@
 
 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 @@
 
 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) }
 {
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to