Use MOZ_MUST_USE in netwerk/protocol/wyciwyg

NEW
Assigned to

Status

()

Core
Networking
P5
normal
a year ago
3 months ago

People

(Reporter: wcpan, Assigned: wcpan)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [necko-would-take])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Subtask of bug 1303701.
Assignee: nobody → wpan
Blocks: 1303701
Whiteboard: [necko-would-take]
Comment hidden (mozreview-request)

Comment 2

11 months ago
mozreview-review
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
Attachment #8824850 - Flags: review?(valentin.gosu) → review+
Comment hidden (mozreview-request)
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.