Bug 1624726: Part 6 - Eliminate pointers from QueueParamTraits::Read r=jgilbert

Since we are no longer peeking or removing without copying, Read always gets a valid object.  This makes its parameter a reference and removes extraneous null checks.

Differential Revision: https://phabricator.services.mozilla.com/D78544
This commit is contained in:
David Parks 2020-06-08 23:42:37 +00:00
parent 8de1bbb2c8
commit a037a1dfed
4 changed files with 61 additions and 110 deletions

View File

@ -502,7 +502,7 @@ class IpdlConsumer final : public SupportsWeakPtr<IpdlConsumer<_Actor>> {
size_t write = mBuf.Length();
mozilla::webgl::ConsumerView<SelfType> view(this, &read, write);
QueueStatus status = DeserializeArgs(view, &aArgs...);
QueueStatus status = DeserializeArgs(view, aArgs...);
if (IsSuccess(status) && (read > 0)) {
mBuf.RemoveElementsAt(0, read);
}
@ -515,7 +515,7 @@ class IpdlConsumer final : public SupportsWeakPtr<IpdlConsumer<_Actor>> {
template <typename Arg, typename... Args>
QueueStatus DeserializeArgs(mozilla::webgl::ConsumerView<SelfType>& aView,
Arg* aArg, Args*... aArgs) {
Arg& aArg, Args&... aArgs) {
QueueStatus status = DeserializeArg(aView, aArg);
if (!IsSuccess(status)) {
return status;
@ -525,10 +525,10 @@ class IpdlConsumer final : public SupportsWeakPtr<IpdlConsumer<_Actor>> {
template <typename Arg>
QueueStatus DeserializeArg(mozilla::webgl::ConsumerView<SelfType>& aView,
Arg* aArg) {
Arg& aArg) {
return mozilla::webgl::
QueueParamTraits<typename mozilla::webgl::RemoveCVR<Arg>::Type>::Read(
aView, const_cast<typename std::remove_cv<Arg>::type*>(aArg));
aView, const_cast<std::remove_cv_t<Arg>*>(&aArg));
}
public:

View File

@ -673,7 +673,7 @@ class PcqConsumer : public detail::PcqBase {
}
// Only update the queue if the operation was successful.
QueueStatus status = TryRemoveArgs(view, &aArgs...);
QueueStatus status = TryRemoveArgs(view, aArgs...);
if (!status) {
return status;
}
@ -748,13 +748,13 @@ class PcqConsumer : public detail::PcqBase {
// Version of the helper for copying values out of the queue.
template <typename... Args>
QueueStatus TryRemoveArgs(ConsumerView<PcqConsumer>& aView, Args*... aArgs);
QueueStatus TryRemoveArgs(ConsumerView<PcqConsumer>& aView, Args&... aArgs);
template <typename Arg, typename... Args>
QueueStatus TryRemoveArgs(ConsumerView<PcqConsumer>& aView, Arg* aArg,
Args*... aArgs) {
QueueStatus status = TryCopyItem<Arg>(aView, aArg);
return IsSuccess(status) ? TryRemoveArgs<Args...>(aView, aArgs...) : status;
QueueStatus TryRemoveArgs(ConsumerView<PcqConsumer>& aView, Arg& aArg,
Args&... aArgs) {
QueueStatus status = TryCopyItem(aView, aArg);
return IsSuccess(status) ? TryRemoveArgs(aView, aArgs...) : status;
}
QueueStatus TryRemoveArgs(ConsumerView<PcqConsumer>&) {
@ -764,10 +764,10 @@ class PcqConsumer : public detail::PcqBase {
// If an item is available then it is copied into aArg. The item is skipped
// over if aArg is null.
template <typename Arg>
QueueStatus TryCopyItem(ConsumerView<PcqConsumer>& aView, Arg* aArg) {
QueueStatus TryCopyItem(ConsumerView<PcqConsumer>& aView, Arg& aArg) {
MOZ_ASSERT(aArg);
return QueueParamTraits<typename RemoveCVR<Arg>::Type>::Read(
aView, const_cast<std::remove_cv_t<Arg>*>(aArg));
aView, const_cast<std::remove_cv_t<Arg>*>(&aArg));
}
template <typename Arg>

View File

@ -260,8 +260,8 @@ class ConsumerView {
template <typename Arg>
QueueStatus ReadParam(Arg* aArg) {
MOZ_ASSERT(aArg);
return mozilla::webgl::QueueParamTraits<
typename RemoveCVR<Arg>::Type>::Read(*this, aArg);
return mozilla::webgl::QueueParamTraits<std::remove_cv_t<Arg>*>::Read(*this,
aArg);
}
/**
@ -270,8 +270,8 @@ class ConsumerView {
template <typename Arg>
size_t MinSizeParam(Arg& aArg) {
MOZ_ASSERT(aArg);
return mozilla::webgl::QueueParamTraits<
typename RemoveCVR<Arg>::Type>::MinSize(*this, aArg);
return mozilla::webgl::QueueParamTraits<std::remove_cv_t<Arg>&>::MinSize(
*this, aArg);
}
inline size_t MinSizeBytes(size_t aNBytes);
@ -393,8 +393,7 @@ struct EnumSerializer {
}
template <typename U>
static bool Read(ConsumerView<U>& aConsumerView, PickleIterator* aIter,
ParamType* aResult) {
static bool Read(ConsumerView<U>& aConsumerView, ParamType* aResult) {
DataType value;
if (!aConsumerView.ReadParam(&value)) {
CrashReporter::AnnotateCrashReport(
@ -452,7 +451,7 @@ struct QueueParamTraits<QueueStatus> {
template <typename U>
static QueueStatus Read(ConsumerView<U>& aConsumerView, ParamType* aArg) {
return aConsumerView.ReadParam(aArg ? &aArg->mValue : nullptr);
return aConsumerView.ReadParam(&aArg->mValue);
}
template <typename View>
@ -488,9 +487,7 @@ struct QueueParamTraits<nsACString> {
if (!aConsumerView.ReadParam(&isVoid)) {
return aConsumerView.GetStatus();
}
if (aArg) {
aArg->SetIsVoid(isVoid);
}
aArg->SetIsVoid(isVoid);
if (isVoid) {
return QueueStatus::kSuccess;
}
@ -501,23 +498,19 @@ struct QueueParamTraits<nsACString> {
}
if (len == 0) {
if (aArg) {
*aArg = "";
}
*aArg = "";
return QueueStatus::kSuccess;
}
char* buf = aArg ? new char[len + 1] : nullptr;
if (aArg && (!buf)) {
char* buf = new char[len + 1];
if (!buf) {
return QueueStatus::kOOMError;
}
if (!aConsumerView.Read(buf, len)) {
return aConsumerView.GetStatus();
}
buf[len] = '\0';
if (aArg) {
aArg->Adopt(buf, len);
}
aArg->Adopt(buf, len);
return QueueStatus::kSuccess;
}
@ -558,9 +551,7 @@ struct QueueParamTraits<nsAString> {
if (!aConsumerView.ReadParam(&isVoid)) {
return aConsumerView.GetStatus();
}
if (aArg) {
aArg->SetIsVoid(isVoid);
}
aArg->SetIsVoid(isVoid);
if (isVoid) {
return QueueStatus::kSuccess;
}
@ -572,20 +563,16 @@ struct QueueParamTraits<nsAString> {
}
if (len == 0) {
if (aArg) {
*aArg = nsString();
}
*aArg = nsString();
return QueueStatus::kSuccess;
}
uint32_t sizeofchar = sizeof(typename ParamType::char_type);
typename ParamType::char_type* buf = nullptr;
if (aArg) {
buf = static_cast<typename ParamType::char_type*>(
malloc((len + 1) * sizeofchar));
if (!buf) {
return QueueStatus::kOOMError;
}
buf = static_cast<typename ParamType::char_type*>(
malloc((len + 1) * sizeofchar));
if (!buf) {
return QueueStatus::kOOMError;
}
if (!aConsumerView.Read(buf, len * sizeofchar)) {
@ -593,10 +580,7 @@ struct QueueParamTraits<nsAString> {
}
buf[len] = L'\0';
if (aArg) {
aArg->Adopt(buf, len);
}
aArg->Adopt(buf, len);
return QueueStatus::kSuccess;
}
@ -653,12 +637,12 @@ struct NSArrayQueueParamTraits<nsTArray<_ElementType>, false> {
return aConsumerView.GetStatus();
}
if (aArg && !aArg->AppendElements(arrayLen, fallible)) {
if (!aArg->AppendElements(arrayLen, fallible)) {
return QueueStatus::kOOMError;
}
for (auto i : IntegerRange(arrayLen)) {
ElementType* elt = aArg ? (&aArg->ElementAt(i)) : nullptr;
ElementType& elt = aArg->ElementAt(i);
aConsumerView.ReadParam(elt);
}
return aConsumerView.GetStatus();
@ -696,7 +680,7 @@ struct NSArrayQueueParamTraits<nsTArray<_ElementType>, true> {
return aConsumerView.GetStatus();
}
if (aArg && !aArg->AppendElements(arrayLen, fallible)) {
if (!aArg->AppendElements(arrayLen, fallible)) {
return QueueStatus::kOOMError;
}
@ -733,16 +717,15 @@ struct ArrayQueueParamTraits<Array<_ElementType, Length>, false> {
template <typename U>
static QueueStatus Write(ProducerView<U>& aProducerView,
const ParamType& aArg) {
for (size_t i = 0; i < Length; ++i) {
aProducerView.WriteParam(aArg[i]);
for (const auto& elt : aArg) {
aProducerView.WriteParam(elt);
}
return aProducerView.GetStatus();
}
template <typename U>
static QueueStatus Read(ConsumerView<U>& aConsumerView, ParamType* aArg) {
for (size_t i = 0; i < Length; ++i) {
ElementType* elt = aArg ? (&((*aArg)[i])) : nullptr;
for (auto& elt : *aArg) {
aConsumerView.ReadParam(elt);
}
return aConsumerView.GetStatus();
@ -751,8 +734,8 @@ struct ArrayQueueParamTraits<Array<_ElementType, Length>, false> {
template <typename View>
static size_t MinSize(View& aView, const ParamType& aArg) {
size_t ret = 0;
for (size_t i = 0; i < Length; ++i) {
ret += aView.MinSizeParam(aArg[i]);
for (const auto& elt : aArg) {
ret += aView.MinSizeParam(elt);
}
return ret;
}
@ -809,16 +792,10 @@ struct QueueParamTraits<Maybe<ElementType>> {
}
if (!isSome) {
if (aArg) {
aArg->reset();
}
aArg->reset();
return QueueStatus::kSuccess;
}
if (!aArg) {
return aConsumerView.template ReadParam<ElementType>(nullptr);
}
aArg->emplace();
return aConsumerView.ReadParam(aArg->ptr());
}
@ -854,16 +831,10 @@ struct QueueParamTraits<Maybe<Variant<T, Ts...>>> {
}
if (!isSome) {
if (aArg) {
aArg->reset();
}
aArg->reset();
return QueueStatus::kSuccess;
}
if (!aArg) {
return aConsumerView.template ReadParam<Variant<T, Ts...>>(nullptr);
}
aArg->emplace(VariantType<T>());
return aConsumerView.ReadParam(aArg->ptr());
}
@ -890,8 +861,8 @@ struct QueueParamTraits<std::pair<TypeA, TypeB>> {
template <typename U>
static QueueStatus Read(ConsumerView<U>& aConsumerView, ParamType* aArg) {
aConsumerView.ReadParam(aArg ? (&aArg->first()) : nullptr);
return aConsumerView.ReadParam(aArg ? (&aArg->second()) : nullptr);
aConsumerView.ReadParam(aArg->first());
return aConsumerView.ReadParam(aArg->second());
}
template <typename View>
@ -924,29 +895,22 @@ struct QueueParamTraits<UniquePtr<T>> {
return aConsumerView.GetStatus();
}
if (isNull) {
if (aArg) {
aArg->reset(nullptr);
}
aArg->reset(nullptr);
return QueueStatus::kSuccess;
}
T* obj = nullptr;
if (aArg) {
obj = new T();
if (!obj) {
return QueueStatus::kOOMError;
}
aArg->reset(obj);
obj = new T();
if (!obj) {
return QueueStatus::kOOMError;
}
aArg->reset(obj);
return aConsumerView.ReadParam(obj);
}
template <typename View>
static size_t MinSize(View& aView, const ParamType& aArg) {
size_t ret = aView.template MinSizeParam<bool>(aArg);
if (!aArg) {
return ret;
}
return ret + aView.MinSizeParam(*aArg);
}
};
@ -984,10 +948,7 @@ struct QueueParamTraits<Variant<Types...>> {
static QueueStatus Read(ConsumerView<U>& aView, Tag aTag, ParamType* aArg) {
if (aTag == N - 1) {
using EntryType = typename mozilla::detail::Nth<N - 1, Types...>::Type;
if (aArg) {
return aView.ReadParam(static_cast<EntryType*>(aArg->ptr()));
}
return aView.template ReadParam<EntryType>();
return aView.ReadParam(*static_cast<EntryType*>(aArg->ptr()));
}
return Next::Read(aView, aTag, aArg);
}
@ -1008,9 +969,7 @@ struct QueueParamTraits<Variant<Types...>> {
if (!aConsumerView.ReadParam(&tag)) {
return aConsumerView.GetStatus();
}
if (aArg) {
aArg->tag = tag;
}
aArg->tag = tag;
return VariantReader<sizeof...(Types)>::Read(aConsumerView, tag, aArg);
}

View File

@ -88,17 +88,11 @@ struct QueueParamTraits<RawBuffer<T>> {
}
if (len == 0) {
if (aArg) {
aArg->mLength = 0;
aArg->mData = nullptr;
}
aArg->mLength = 0;
aArg->mData = nullptr;
return QueueStatus::kSuccess;
}
if (!aArg) {
return aConsumerView.Read(nullptr, len * sizeof(T));
}
struct RawBufferReadMatcher {
QueueStatus operator()(RefPtr<mozilla::ipc::SharedMemoryBasic>& smem) {
if (!smem) {
@ -159,7 +153,7 @@ struct QueueParamTraits<Result<V, E>> {
}
template <typename U>
static QueueStatus Read(ConsumerView<U>& aConsumerView, T* const aArg) {
static QueueStatus Read(ConsumerView<U>& aConsumerView, T* aArg) {
bool ok;
auto status = aConsumerView.ReadParam(&ok);
if (!status) return status;
@ -202,7 +196,7 @@ struct QueueParamTraits<std::string> {
}
template <typename U>
static QueueStatus Read(ConsumerView<U>& aConsumerView, T* const aArg) {
static QueueStatus Read(ConsumerView<U>& aConsumerView, T* aArg) {
size_t size;
auto status = aConsumerView.ReadParam(&size);
if (!status) return status;
@ -212,9 +206,7 @@ struct QueueParamTraits<std::string> {
if (!dest) return QueueStatus::kFatalError;
status = aConsumerView.Read(dest, size);
if (aArg) {
aArg->assign(dest, size);
}
aArg->assign(dest, size);
return status;
}
@ -239,14 +231,14 @@ struct QueueParamTraits<std::vector<U>> {
}
template <typename V>
static QueueStatus Read(ConsumerView<V>& aConsumerView, T* const aArg) {
static QueueStatus Read(ConsumerView<V>& aConsumerView, T* aArg) {
MOZ_CRASH("no way to fallibly resize vectors without exceptions");
size_t size;
auto status = aConsumerView.ReadParam(&size);
if (!status) return status;
aArg->resize(size);
status = aConsumerView.Read(aArg->data(), aArg->size());
status = aConsumerView.Read(aArg->data(), size);
return status;
}
@ -277,11 +269,11 @@ struct QueueParamTraits<CompileResult> {
}
template <typename U>
static QueueStatus Read(ConsumerView<U>& aConsumerView, T* const aArg) {
aConsumerView.ReadParam(aArg ? &aArg->pending : nullptr);
aConsumerView.ReadParam(aArg ? &aArg->log : nullptr);
aConsumerView.ReadParam(aArg ? &aArg->translatedSource : nullptr);
return aConsumerView.ReadParam(aArg ? &aArg->success : nullptr);
static QueueStatus Read(ConsumerView<U>& aConsumerView, T* aArg) {
aConsumerView.ReadParam(&aArg->pending);
aConsumerView.ReadParam(&aArg->log);
aConsumerView.ReadParam(&aArg->translatedSource);
return aConsumerView.ReadParam(&aArg->success);
}
template <typename View>