1502799 - Implement origin-clean algorithm for ImageBitmap

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Andrea Marchesini [:baku]]

https://html.spec.whatwg.org/multipage/imagebitmap-and-animations.html

"An ImageBitmap object's bitmap has an origin-clean flag, which indicates whether the bitmap is tainted by content from a different origin. The flag is initially set to true and may be changed to false by the steps of createImageBitmap()."

Comment 1

[Andrea Marchesini [:baku]]

Attachment: write_only.patch

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9020695 [details] [diff] [review]
write_only.patch

Review of attachment 9020695 [details] [diff] [review]:
-----------------------------------------------------------------

WebGLRenderingContext/WebGL2RenderingContext expose tex(Sub)Image2D and tex(Sub)Image3D which accept an ImageBitmap. I think we need some extra checks around:

https://searchfox.org/mozilla-central/rev/7c848ac7630df5baf1314b0c03e015683599efb9/dom/canvas/WebGLTextureUpload.cpp#223

Comment 3

[Andrea Marchesini [:baku]]

Attachment: write_only.patch

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5e9d42be7b3
Implement origin-clean algorithm for ImageBitmap, r=aosmond

Comment 5

[Natalia Csoregi [:nataliaCs]]

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

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Can you request uplift for esr60? Thanks!

Comment 7

[Andrea Marchesini [:baku]]

Comment on attachment 9023032 [details] [diff] [review]
write_only.patch

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This patch is the entry point of a good security improvement related to how and when image data should be exposed to cross-origin content.
• User impact if declined:
• Fix Landed on Version: 65
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch has been, together with the others, in nightly for a while. Low risk.
• String or UUID changes made by this patch: none

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9023032 [details] [diff] [review]
write_only.patch

OK for ESR uplift for 60.7.0.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Looks like this needs rebasing :\

Comment 10

[Andrea Marchesini [:baku]]

Attachment: Bug 1502799 - Implement origin-clean algorithm for ImageBitmap, r=aosmond

Bug 1484980 - Add selective canvas tainting for content scripts r=bzbarsky
Bug 1526218 - transferFromImageBitmap() should propage the origin-clean state to the canvas element, r=aosmond
Bug 1528909 - cross-origin checks in CanvasRenderingContext2D::DrawImage, r=aosmond
Bug 1540221 - Setting fillStyle to a pattern of an unclean canvas makes the canvas origin-unclean, r=aosmond

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/ebd350549d83

Comment 12

[Cosmin Sabou [:CosminS]]

Backed out from esr60 for build bustages on HTMLCanvasElement.cpp.

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-esr60&resultStatus=testfailed%2Cbusted%2Cexception&revision=ebd350549d83bde2f5e12ec9ea1178d73ee1c25e&selectedJob=246339974

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=246339983&repo=mozilla-esr60&lineNumber=15380

Backout link: https://hg.mozilla.org/releases/mozilla-esr60/rev/9d3b9a48a91710e8bf4eb36c5181e391c7c14abe

[task 2019-05-14T08:48:26.175Z] 08:48:26 INFO - make[5]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/html'
[task 2019-05-14T08:48:26.179Z] 08:48:26 INFO - /builds/worker/workspace/build/src/gcc/bin/g++ -o Unified_cpp_dom_html0.i_o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/dom/html -I/builds/worker/workspace/build/src/obj-firefox/dom/html -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/caps -I/builds/worker/workspace/build/src/docshell/base -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/dom/canvas -I/builds/worker/workspace/build/src/dom/html/input -I/builds/worker/workspace/build/src/dom/media -I/builds/worker/workspace/build/src/dom/security -I/builds/worker/workspace/build/src/dom/xbl -I/builds/worker/workspace/build/src/dom/xul -I/builds/worker/workspace/build/src/editor/txmgr -I/builds/worker/workspace/build/src/image -I/builds/worker/workspace/build/src/layout/forms -I/builds/worker/workspace/build/src/layout/generic -I/builds/worker/workspace/build/src/layout/style -I/builds/worker/workspace/build/src/layout/tables -I/builds/worker/workspace/build/src/layout/xul -I/builds/worker/workspace/build/src/netwerk/base -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 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wformat -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -O3 -fomit-frame-pointer -Werror -Wno-error=shadow -fprofile-generate -MD -MP -MF .deps/Unified_cpp_dom_html0.i_o.pp /builds/worker/workspace/build/src/obj-firefox/dom/html/Unified_cpp_dom_html0.cpp
[task 2019-05-14T08:48:26.180Z] 08:48:26 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/dom/html/Unified_cpp_dom_html0.cpp:65:0:
[task 2019-05-14T08:48:26.181Z] 08:48:26 INFO - /builds/worker/workspace/build/src/dom/html/HTMLCanvasElement.cpp:940:6: error: prototype for 'bool mozilla::dom::HTMLCanvasElement::IsWriteOnly()' does not match any in class 'mozilla::dom::HTMLCanvasElement'
[task 2019-05-14T08:48:26.182Z] 08:48:26 INFO - bool HTMLCanvasElement::IsWriteOnly() { return mWriteOnly; }
[task 2019-05-14T08:48:26.183Z] 08:48:26 INFO - ^~~~~~~~~~~~~~~~~
[task 2019-05-14T08:48:26.183Z] 08:48:26 INFO - In file included from /builds/worker/workspace/build/src/dom/html/HTMLCanvasElement.cpp:7:0,
[task 2019-05-14T08:48:26.184Z] 08:48:26 INFO - from /builds/worker/workspace/build/src/obj-firefox/dom/html/Unified_cpp_dom_html0.cpp:65:
[task 2019-05-14T08:48:26.185Z] 08:48:26 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/HTMLCanvasElement.h:204:8: error: candidate is: bool mozilla::dom::HTMLCanvasElement::IsWriteOnly() const
[task 2019-05-14T08:48:26.185Z] 08:48:26 INFO - bool IsWriteOnly() const;
[task 2019-05-14T08:48:26.186Z] 08:48:26 INFO - ^~~~~~~~~~~
[task 2019-05-14T08:48:26.187Z] 08:48:26 INFO - /builds/worker/workspace/build/src/config/rules.mk:1054: recipe for target 'Unified_cpp_dom_html0.i_o' failed
[task 2019-05-14T08:48:26.188Z] 08:48:26 INFO - make[5]: *** [Unified_cpp_dom_html0.i_o] Error 1
[task 2019-05-14T08:48:26.188Z] 08:48:26 INFO - make[5]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/html'
[task 2019-05-14T08:48:26.189Z] 08:48:26 INFO - /builds/worker/workspace/build/src/config/recurse.mk:73: recipe for target 'dom/html/target' failed
[task 2019-05-14T08:48:26.190Z] 08:48:26 INFO - make[4]: *** [dom/html/target] Error 2
[task 2019-05-14T08:48:26.190Z] 08:48:26 INFO - make[4]: *** Waiting for unfinished jobs....

Comment 13

[Andrea Marchesini [:baku]]

New patch submitted

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/64ee63facd4f

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/e24ec6070d7b

Comment 16

[Jan Beich]

Why workarounds for bug 1519629 and bug 1519729 were bootlegged?
https://hg.mozilla.org/releases/mozilla-esr60/diff/64ee63facd4f/servo/components/style/gecko_string_cache/namespace.rs
https://hg.mozilla.org/releases/mozilla-esr60/diff/64ee63facd4f/servo/components/style_traits/values.rs

Comment 17

[Andrea Marchesini [:baku]]

Attachment: Bug 1502799 - Reintroduce serialize_function, disabled by mistake by previous patches, r?emilio

Comment 18

[Andrea Marchesini [:baku]]

Sent a patch to enabled them again. My mistake.

Comment 19

[Jan Beich]

Comment on attachment 9065302 [details]
Bug 1502799 - Reintroduce serialize_function, disabled by mistake by previous patches, r?emilio

Why do you leave docstring define_css_keyword_enum which is part of bug 1519729? Either backport all fixes for later Rust versions properly[1] or backout all unrelated changes.

[1] For example:
https://github.com/freebsd/freebsd-ports/blob/1e2726401874/www/firefox-esr/files/patch-bug1519629
https://github.com/freebsd/freebsd-ports/blob/1e2726401874/www/firefox-esr/files/patch-bug1519729

Comment 20

[Andrea Marchesini [:baku]]

This patch is meant to land in esr-60. It's not going to be in m-i/m-c. I just want to remove the rust changes in my previous patch. Bug 1519729 landed in 66 and it's not going to be ported to esr-60.

Comment 21

[Andrea Marchesini [:baku]]

Comment on attachment 9065302 [details]
Bug 1502799 - Reintroduce serialize_function, disabled by mistake by previous patches, r?emilio

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: In the previous patch I removed and added comments to rust code. Those changes were meant to be just for testing. Would be nice if we can remove them.
• User impact if declined: none
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch introduces what I removed/added in the previous code and it's completely unrelated to imagebitmap
• String or UUID changes made by this patch: none

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

We're already built 60.7.0, But, if we end up having a 60.7.1 dot release, we can take this as a ridealong. I'll leave the uplift request open for now.

Comment 23

[Andrea Marchesini [:baku]]

Comment on attachment 9065302 [details]
Bug 1502799 - Reintroduce serialize_function, disabled by mistake by previous patches, r?emilio

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: I didn't want to cancel this esr-60 request.
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch:

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9065302 [details]
Bug 1502799 - Reintroduce serialize_function, disabled by mistake by previous patches, r?emilio

Fixes a mistake in the rebased ESR60 patch. Approved for 60.8esr.

Comment 25

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr60/rev/4ac1f2b385f2