1493292 - Remove aDataLen parameters from nsITransferable

Status: RESOLVED FIXED

(Core :: Widget, enhancement)

Description

[Tom S [:evilpie]]

As far as I can tell the dataLen is actually not needed for anything.

Comment 1

[Edgar Chen [:edgar] (OOO until 4/10)]

It looks like this bug should be under Widget component to me. Feel free to switch back if I am wrong.

Comment 2

[Tom S [:evilpie]]

I thought I could start with just removing the aDataLen parameter GetTransferData, but there are very few cases where we pass that into SetTransferData again.

Comment 3

[Tom S [:evilpie]]

There is also the special meaning of length = 0.

Comment 5

[Tom S [:evilpie]]

Attachment: WIP

Comment 7

[Tom S [:evilpie]]

Looking pretty good on Linux, not sure yet if any of the orange is related to my patches: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e394d7597935c52d4745bcf92b95007f39b60f24

Comment 8

[Tom S [:evilpie]]

At least one failure is related to nsIFormatConverter.

Comment 9

[Tom S [:evilpie]]

Attachment: Bug 1493292 - GetStringSize instead of aDataLen. r?mats

Comment 10

[Tom S [:evilpie]]

Attachment: Bug 1493292 - Remove aDataLen parameters from nsITransferable.getTransferData. r?smaug

Depends on D11200

Comment 11

[Tom S [:evilpie]]

Attachment: Bug 1493292 - Remove aDataLen parameters from nsITransferable.setTransferData. r?smaug

Depends on D11201

Comment 12

[Tom S [:evilpie]]

Attachment: Bug 1493292 - Remove len from nsTransferable. r?smaug

Depends on D11202

Comment 13

[Tom S [:evilpie]]

Attachment: Bug 1493292 - Remove len from nsIFlavorDataProvider r?smaug

Depends on D11203

Comment 14

[Tom S [:evilpie]]

Attachment: Bug 1493292 - Remove len from nsIFormatConverter. r?smaug

Depends on D11204

Comment 15

[Tom S [:evilpie]]

I feel a bit guilty about dumping this all on smaug, feel free to redirect this to other people who could review this.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/55769869037c
GetStringSize instead of aDataLen. r=mats
https://hg.mozilla.org/integration/autoland/rev/3a9b6d65d8c7
Remove aDataLen parameters from nsITransferable.getTransferData. r=smaug
https://hg.mozilla.org/integration/autoland/rev/71430fceb4a3
Remove aDataLen parameters from nsITransferable.setTransferData. r=smaug
https://hg.mozilla.org/integration/autoland/rev/f2cabd69c568
Remove len from nsTransferable. r=smaug
https://hg.mozilla.org/integration/autoland/rev/6487aa307123
Remove len from nsIFlavorDataProvider r=smaug
https://hg.mozilla.org/integration/autoland/rev/f198bf91320b
Remove len from nsIFormatConverter. r=smaug

Comment 17

[Natalia Csoregi [:nataliaCs]]

Backed out for bustage on /nsTransferable.cpp

backout: https://hg.mozilla.org/integration/autoland/rev/4b0554a10847e9a879ab193bf16a3bbdaa8e69d4

push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f198bf91320bf774f2c9f204c426ac0c2da95aa2&selectedJob=212900743

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=212900743&repo=autoland&lineNumber=22930

[task 2018-11-20T15:04:54.516Z] 15:04:54     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/widget'
[task 2018-11-20T15:04:54.517Z] 15:04:54     INFO -  /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ -target x86_64-apple-darwin11 -B /builds/worker/workspace/build/src/cctools/bin -isysroot /builds/worker/workspace/build/src/MacOSX10.11.sdk -o Unified_cpp_widget2.o -c -flto=thin -fvisibility=hidden -fvisibility-inlines-hidden -DNDEBUG=1 -DTRIMMED=1 -DMOZ_CROSS_PROCESS_IME -DOS_POSIX=1 -DOS_MACOSX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/widget -I/builds/worker/workspace/build/src/obj-firefox/widget -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/dom/ipc -I/builds/worker/workspace/build/src/layout/base -I/builds/worker/workspace/build/src/layout/forms -I/builds/worker/workspace/build/src/layout/generic -I/builds/worker/workspace/build/src/layout/painting -I/builds/worker/workspace/build/src/layout/xul -I/builds/worker/workspace/build/src/layout/xul/tree -I/builds/worker/workspace/build/src/view -I/builds/worker/workspace/build/src/widget -I/builds/worker/workspace/build/src/widget/headless -I/builds/worker/workspace/build/src/widget/cocoa -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Qunused-arguments -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-exceptions -fno-strict-aliasing -stdlib=libc++ -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O3 -fno-omit-frame-pointer -funwind-tables -Werror  -MD -MP -MF .deps/Unified_cpp_widget2.o.pp   /builds/worker/workspace/build/src/obj-firefox/widget/Unified_cpp_widget2.cpp
[task 2018-11-20T15:04:54.517Z] 15:04:54     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/widget/Unified_cpp_widget2.cpp:47:
[task 2018-11-20T15:04:54.521Z] 15:04:54    ERROR -  /builds/worker/workspace/build/src/widget/nsTransferable.cpp:140:15: error: comparison of integers of different signs: 'int32_t' (aka 'int') and 'uint32_t' (aka 'unsigned int') [-Werror,-Wsign-compare]
[task 2018-11-20T15:04:54.521Z] 15:04:54     INFO -    if (written == aDataLen) {
[task 2018-11-20T15:04:54.521Z] 15:04:54     INFO -        ~~~~~~~ ^  ~~~~~~~~
[task 2018-11-20T15:04:54.522Z] 15:04:54     INFO -  1 error generated.
[task 2018-11-20T15:04:54.522Z] 15:04:54     INFO -  /builds/worker/workspace/build/src/config/rules.mk:1131: recipe for target 'Unified_cpp_widget2.o' failed
[task 2018-11-20T15:04:54.522Z] 15:04:54    ERROR -  make[4]: *** [Unified_cpp_widget2.o] Error 1
[task 2018-11-20T15:04:54.522Z] 15:04:54     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/widget'
[task 2018-11-20T15:04:54.522Z] 15:04:54     INFO -  /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'widget/target' failed
[task 2018-11-20T15:04:54.522Z] 15:04:54    ERROR -  make[3]: *** [widget/target] Error 2
[task 2018-11-20T15:04:54.523Z] 15:04:54     INFO -  make[3]: *** Waiting for unfinished jobs....
[task 2018-11-20T15:04:54.523Z] 15:04:54     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/layout/xul'

Comment 18

[Tom S [:evilpie]]

Buh

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29b3f0dfc6bb
GetStringSize instead of aDataLen. r=mats
https://hg.mozilla.org/integration/autoland/rev/a58e50deefdc
Remove aDataLen parameters from nsITransferable.getTransferData. r=smaug
https://hg.mozilla.org/integration/autoland/rev/7e2c0177115a
Remove aDataLen parameters from nsITransferable.setTransferData. r=smaug
https://hg.mozilla.org/integration/autoland/rev/f02e1aba84a5
Remove len from nsTransferable. r=smaug
https://hg.mozilla.org/integration/autoland/rev/8c6a2b354422
Remove len from nsIFlavorDataProvider r=smaug
https://hg.mozilla.org/integration/autoland/rev/bd64f10454cc
Remove len from nsIFormatConverter. r=smaug

Comment 20

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/29b3f0dfc6bb
https://hg.mozilla.org/mozilla-central/rev/a58e50deefdc
https://hg.mozilla.org/mozilla-central/rev/7e2c0177115a
https://hg.mozilla.org/mozilla-central/rev/f02e1aba84a5
https://hg.mozilla.org/mozilla-central/rev/8c6a2b354422
https://hg.mozilla.org/mozilla-central/rev/bd64f10454cc