Changeset 285866 in webkit

Timestamp:

Nov 16, 2021, 8:57:55 AM (5 years ago)

Author:

Chris Dumez

Message:

Decoder::unwrapForTesting() is unnecessarily inefficient
​https://bugs.webkit.org/show_bug.cgi?id=233145

Reviewed by Darin Adler.

Decoder::unwrapForTesting() is unnecessarily inefficient. It can take the whole
m_attachments data members from the decoder instead of calling removeAttachment()
repeatedly on the Decoder.

Also rename removeAttachment() to takeAttachment() since it returns the Attachment.
Update it to return a std::optional<Attachment> instead of using an out-parameter.

• Platform/IPC/Attachment.cpp:

(IPC::Attachment::decode):

• Platform/IPC/Attachment.h:
• Platform/IPC/Decoder.cpp:

(IPC::Decoder::unwrapForTesting):
(IPC::Decoder::takeAttachment):
(IPC::Decoder::removeAttachment): Deleted.

• Platform/IPC/Decoder.h:
• Platform/IPC/win/AttachmentWin.cpp:

(IPC::Attachment::decode):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Platform/IPC/Attachment.cpp (modified) (1 diff)
• Platform/IPC/Attachment.h (modified) (2 diffs)
• Platform/IPC/Decoder.cpp (modified) (3 diffs)
• Platform/IPC/Decoder.h (modified) (3 diffs)
• Platform/IPC/win/AttachmentWin.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-11-16  Chris Dumez  <cdumez@apple.com>
+
+        Decoder::unwrapForTesting() is unnecessarily inefficient
+        https://bugs.webkit.org/show_bug.cgi?id=233145
+
+        Reviewed by Darin Adler.
+
+        Decoder::unwrapForTesting() is unnecessarily inefficient. It can take the whole
+        m_attachments data members from the decoder instead of calling removeAttachment()
+        repeatedly on the Decoder.
+
+        Also rename removeAttachment() to takeAttachment() since it returns the Attachment.
+        Update it to return a std::optional<Attachment> instead of using an out-parameter.
+
+        * Platform/IPC/Attachment.cpp:
+        (IPC::Attachment::decode):
+        * Platform/IPC/Attachment.h:
+        * Platform/IPC/Decoder.cpp:
+        (IPC::Decoder::unwrapForTesting):
+        (IPC::Decoder::takeAttachment):
+        (IPC::Decoder::removeAttachment): Deleted.
+        * Platform/IPC/Decoder.h:
+        * Platform/IPC/win/AttachmentWin.cpp:
+        (IPC::Attachment::decode):
+
 2021-11-16  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebKit/Platform/IPC/Attachment.cpp

@@ -57 +57 @@
 }
 
-bool Attachment::decode(Decoder& decoder, Attachment& attachment)
+std::optional<Attachment> Attachment::decode(Decoder& decoder)
 {
-    if (!decoder.removeAttachment(attachment))
-        return false;
-    return true;
+    return decoder.takeLastAttachment();
 }
 #endif

trunk/Source/WebKit/Platform/IPC/Attachment.h

@@ -26 +26 @@
 
 #pragma once
+
+#include <optional>
 
 #if OS(DARWIN) && !USE(UNIX_DOMAIN_SOCKETS)
@@ -99 +101 @@
 
     void encode(Encoder&) const;
-    static WARN_UNUSED_RETURN bool decode(Decoder&, Attachment&);
+    static std::optional<Attachment> decode(Decoder&);
     
 private:

trunk/Source/WebKit/Platform/IPC/Decoder.cpp

@@ -34 +34 @@
 #include <wtf/StdLibExtras.h>
 
-#if PLATFORM(MAC)
-#include "ImportanceAssertion.h"
-#endif
-
 namespace IPC {
 
@@ -148 +144 @@
     ASSERT(decoder.isSyncMessage());
 
-    Vector<Attachment> attachments;
-    Attachment attachment;
-    while (decoder.removeAttachment(attachment))
-        attachments.append(WTFMove(attachment));
-    attachments.reverse();
+    auto attachments = std::exchange(decoder.m_attachments, { });
 
     DataReference wrappedMessage;
@@ -219 +211 @@
 }
 
-bool Decoder::removeAttachment(Attachment& attachment)
+std::optional<Attachment> Decoder::takeLastAttachment()
 {
     if (m_attachments.isEmpty())
-        return false;
-
-    attachment = m_attachments.takeLast();
-    return true;
+        return std::nullopt;
+    return m_attachments.takeLast();
 }
 

trunk/Source/WebKit/Platform/IPC/Decoder.h

@@ -28 +28 @@
 #include "Attachment.h"
 #include "MessageNames.h"
-#include "StringReference.h"
-#include <WebCore/SharedBuffer.h>
 #include <wtf/OptionSet.h>
 #include <wtf/Vector.h>
@@ -35 +33 @@
 #if PLATFORM(MAC)
 #include "ImportanceAssertion.h"
-#endif
-
-#if HAVE(QOS_CLASSES)
-#include <pthread/qos.h>
 #endif
 
@@ -141 +135 @@
     }
 
-    bool removeAttachment(Attachment&);
+    std::optional<Attachment> takeLastAttachment();
 
     static constexpr bool isIPCDecoder = true;

trunk/Source/WebKit/Platform/IPC/win/AttachmentWin.cpp

@@ -66 +66 @@
 }
 
-bool Attachment::decode(Decoder& decoder, Attachment& attachment)
+std::optional<Attachment> Attachment::decode(Decoder& decoder)
 {
-    ASSERT_ARG(attachment, attachment.m_handle == INVALID_HANDLE_VALUE);
-
     uint64_t sourceHandle;
     if (!decoder.decode(sourceHandle))
-        return false;
+        return std::nullopt;
 
     uint32_t sourcePID;
     if (!decoder.decode(sourcePID))
-        return false;
+        return std::nullopt;
 
     HANDLE duplicatedHandle;
     if (!getDuplicatedHandle(reinterpret_cast<HANDLE>(sourceHandle), sourcePID, duplicatedHandle))
-        return false;
+        return std::nullopt;
 
-    attachment.m_handle = duplicatedHandle;
-    return true;
+    return Attachment { duplicatedHandle };
 }