571074 - nsTransferable::GetTransferData callers depend on valid out params also when it returns an error

Status: RESOLVED FIXED

(Core :: DOM: Serializers, defect)

Description

[Mats Palmgren (inactive)]

(Follow-up from bug 565392 comment 10)

nsTransferable::GetTransferData callers depend on valid out params also when it returns an error.

Comment 1

[Tom S [:evilpie]]

Attachment: Bug 571074 - Mark nsITransferable.getTransferData as [must_use]. r?mats

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9900ac6071c
Mark nsITransferable.getTransferData as [must_use]. r=mats

Comment 3

[Andreea Pavel [:apavel]]

Backed out for failing clipboard at browser/base/content/test/general/browser_clipboard.js

Push that failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f9900ac6071c17ff7af3ccfaf2a8ce02ea66cffc

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=214722805&repo=autoland&lineNumber=6278

Backout: https://hg.mozilla.org/integration/autoland/rev/1e64f8c3f949bb16b4981f255033021b42118586

Comment 4

[Tom S [:evilpie]]

In HTMLEditor::PasteInternal, the kHTMLContext and/or kHTMLInfo MIME types on the clipboard should not be required to paste.

Comment 5

[Tom S [:evilpie]]

nsContentUtils::TransferableToIPCTransferable changed in behavior, because we don't add an empty item anymore, but instead skip it. I think for that we should first clean this up.

Comment 6

[Tom S [:evilpie]]

Attachment: Bug 571074 - Refactor nsContentUtils::TransferableToIPCTransferable to make error handling more obvious. r?smaug

While trying to handle a failed GetTransferData call I made some mistake, mostly because this function has incredibily
confusing control flow. I would actually assume some of its current behavior is not intentional.

For this we assume that data == nullptr <-> NS_FAILED(rv), this is probably not 100% correct, but doesn't seem to
matter in practice. (This is a change in behavior that is intentional, because we want to check return value)

If we trace the data == nullptr case through the old code, we realize that for aInSyncMessage we will just continue.
(This is because nullptr doesn't queryInterface to anything)

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b90c87fbab03
Refactor nsContentUtils::TransferableToIPCTransferable to make error handling more obvious. r=smaug

Comment 8

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/b90c87fbab03

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e4c5ef41c73
Mark nsITransferable.getTransferData as [must_use]. r=mats

Comment 10

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/2e4c5ef41c73