1323950 - Use MOZ_MUST_USE in netwerk/protocol/wyciwyg

Status: RESOLVED WONTFIX

(Core :: Networking, defect, P5)

Description

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Subtask of bug 1303701.

Comment 1

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Attachment: Bug 1323950 - Use MOZ_MUST_USE in netwerk/protocol/wyciwyg

Review commit: https://reviewboard.mozilla.org/r/103146/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/103146/

Comment 2

[Valentin Gosu [:valentin] (he/him) {{ PTO until Nov 5 }}]

Comment on attachment 8824850 [details]
Bug 1323950 - Use MOZ_MUST_USE in netwerk/protocol/wyciwyg

https://reviewboard.mozilla.org/r/103146/#review106268

::: dom/html/nsHTMLDocument.cpp:1893
(Diff revision 1)
>  
>    // Save the data in cache if the write isn't from within the doc
>    if (mWyciwygChannel && !key) {
>      if (!aText.IsEmpty()) {
> -      mWyciwygChannel->WriteToCacheEntry(aText);
> +      rv = mWyciwygChannel->WriteToCacheEntry(aText);
> +      MOZ_ASSERT(NS_SUCCEEDED(rv));

May fail. LOG or NS_WARNING instead.

::: dom/html/nsHTMLDocument.cpp:1898
(Diff revision 1)
> +      MOZ_ASSERT(NS_SUCCEEDED(rv));
>      }
>  
>      if (aNewlineTerminate) {
> -      mWyciwygChannel->WriteToCacheEntry(new_line);
> +      rv = mWyciwygChannel->WriteToCacheEntry(new_line);
> +      MOZ_ASSERT(NS_SUCCEEDED(rv));

May fail. LOG or NS_WARNING instead.

::: dom/html/nsHTMLDocument.cpp:2326
(Diff revision 1)
>    loadInfo->SetPrincipalToInherit(NodePrincipal());
>  
>    mWyciwygChannel = do_QueryInterface(channel);
>  
> -  mWyciwygChannel->SetSecurityInfo(mSecurityInfo);
> +  rv = mWyciwygChannel->SetSecurityInfo(mSecurityInfo);
> +  MOZ_ASSERT(NS_SUCCEEDED(rv));

May fail. LOG or NS_WARNING instead.

::: dom/html/nsHTMLDocument.cpp:2369
(Diff revision 1)
>    // note there can be a write request without a load group if
>    // this is a synchronously constructed about:blank document
>    if (loadGroup && mWyciwygChannel) {
> -    mWyciwygChannel->CloseCacheEntry(NS_OK);
> +    nsresult rv = mWyciwygChannel->CloseCacheEntry(NS_OK);
> +    if (NS_FAILED(rv)) {
> +      printf("nsHTMLDocument::RemoveWyciwygChannel "

write to stderr maybe? Printfs are lost in non-debug builds, and no one is looking anyway.

::: netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp:263
(Diff revision 1)
>      return IPC_FAIL_NO_REASON(this);
>    }
>  
> -  if (mChannel)
> -    mChannel->WriteToCacheEntry(data);
> +  if (mChannel) {
> +    DebugOnly<nsresult> rv = mChannel->WriteToCacheEntry(data);
> +    MOZ_ASSERT(NS_SUCCEEDED(rv));

May fail. LOG or NS_WARNING instead.

::: netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp:274
(Diff revision 1)
>  mozilla::ipc::IPCResult
>  WyciwygChannelParent::RecvCloseCacheEntry(const nsresult& reason)
>  {
>    if (mChannel) {
> -    mChannel->CloseCacheEntry(reason);
> +    DebugOnly<nsresult> rv = mChannel->CloseCacheEntry(reason);
> +    MOZ_ASSERT(NS_SUCCEEDED(rv));

May fail. LOG or NS_WARNING instead.

::: netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp:299
(Diff revision 1)
>  {
>    if (mChannel) {
>      nsCOMPtr<nsISupports> securityInfo;
>      NS_DeserializeObject(aSecurityInfo, getter_AddRefs(securityInfo));
> -    mChannel->SetSecurityInfo(securityInfo);
> +    DebugOnly<nsresult> rv = mChannel->SetSecurityInfo(securityInfo);
> +    MOZ_ASSERT(NS_SUCCEEDED(rv));

May fail. LOG or NS_WARNING instead.

::: netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp:336
(Diff revision 1)
>    chan->GetContentLength(&contentLength);
>  
>    int32_t charsetSource = kCharsetUninitialized;
>    nsAutoCString charset;
> -  chan->GetCharsetAndSource(&charsetSource, charset);
> +  rv = chan->GetCharsetAndSource(&charsetSource, charset);
> +  MOZ_ASSERT(NS_SUCCEEDED(rv));

May fail. LOG/NS_WARNING

::: netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp:69
(Diff revision 1)
>      : nsWyciwygAsyncEvent(aChannel), mData(aData) {}
>  
>    NS_IMETHOD Run() override
>    {
> -    mChannel->WriteToCacheEntryInternal(mData);
> +    mozilla::DebugOnly<nsresult> rv = mChannel->WriteToCacheEntryInternal(mData);
> +    MOZ_ASSERT(NS_SUCCEEDED(rv));

May fail. LOG/NS_WARNING

::: netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp:84
(Diff revision 1)
>      : nsWyciwygAsyncEvent(aChannel), mReason(aReason) {}
>  
>    NS_IMETHOD Run() override
>    {
> -    mChannel->CloseCacheEntryInternal(mReason);
> +    mozilla::DebugOnly<nsresult> rv = mChannel->CloseCacheEntryInternal(mReason);
> +    MOZ_ASSERT(NS_SUCCEEDED(rv));

May fail. LOG/NS_WARNING

::: netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp:701
(Diff revision 1)
>    }
>  
>    // a failure from Connect means that we have to abort the channel.
>    if (NS_FAILED(rv)) {
> -    CloseCacheEntry(rv);
> +    rv = CloseCacheEntry(rv);
> +    MOZ_ASSERT(NS_SUCCEEDED(rv));

May fail. LOG/NS_WARNING

::: netwerk/protocol/wyciwyg/nsWyciwygChannel.cpp:769
(Diff revision 1)
>  
>    if (mLoadGroup)
>      mLoadGroup->RemoveRequest(this, nullptr, mStatus);
>  
> -  CloseCacheEntry(mStatus);
> +  mozilla::DebugOnly<nsresult> rv = CloseCacheEntry(mStatus);
> +  MOZ_ASSERT(NS_SUCCEEDED(rv));

May fail. LOG/NS_WARNING

Comment 3

[Wei-Cheng Pan [:wcpan] [:wcp] [:legnaleurc] (left Mozilla)]

Comment on attachment 8824850 [details]
Bug 1323950 - Use MOZ_MUST_USE in netwerk/protocol/wyciwyg

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/103146/diff/1-2/

Comment 4

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 5

[Andrew McCreight [:mccr8]]

This code is being deleted soon, in bug 1489308.