Closed Bug 1908205 Opened 2 years ago Closed 1 year ago

Use OffscreenCanvas for screenshots so we can use blob instead of data urls

Categories

(Firefox :: Screenshots, task, P1)

Product:
Firefox
Component:
Screenshots
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
140 Branch
Tracking Flags:
Tracking Status
firefox140 --- fixed

People

(Reporter: niklas, Assigned: niklas)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Bug 1908205 - Switch to OffscreenCanvas and blobs for screenshots. r=sfoster,dimi
48 bytes, text/x-phabricator-request
Details | Review

In bug 1733639 we switched to using a blob instead of DataURLs. We eventually went away from that in a refactor. We should go back to using blobs and switch to OffscreenCanvas because we never actually show the canvas.

Pushed by nbaumgardner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/873bbe70aa8c Switch to OffscreenCanvas and blobs for screenshots. r=sfoster

Backed out for causing mochitests failures.

  • Push with failures - another mochitests failures
  • Failure Log
  • Failure line: TEST-UNEXPECTED-FAIL | browser/components/screenshots/tests/browser/browser_screenshots_test_visible.js | A promise chain failed to handle a rejection: can't access property "methodsUsed", this[#overlay] is null - stack: getMethodsUsed@resource:///actors/ScreenshotsComponentChild.sys.mjs:208:23
Flags: needinfo?(nbaumgardner)
Pushed by nbaumgardner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/859b4c639883 Switch to OffscreenCanvas and blobs for screenshots. r=sfoster

Backed out 2 changesets (bug 1908205, bug 1896232) for causing bc assertion failures on nsUrlClassifierDBService.cpp.

[task 2024-08-22T19:10:35.213Z] 19:10:35     INFO - TEST-START | browser/components/screenshots/tests/browser/browser_keyboard_shortcuts.js
[task 2024-08-22T19:10:35.272Z] 19:10:35     INFO - GECKO(2589) | [Child 2751: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 7fc78c06d800 == 1 [pid = 2751] [id = 0]
[task 2024-08-22T19:10:35.272Z] 19:10:35     INFO - GECKO(2589) | [Child 2751: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 1 (7fc78b75b600) [pid = 2751] [serial = 1] [outer = 0]
[task 2024-08-22T19:10:35.278Z] 19:10:35     INFO - GECKO(2589) | [Child 2751: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 2 (7fc78c06dc00) [pid = 2751] [serial = 2] [outer = 7fc78b75b600]
[task 2024-08-22T19:10:35.486Z] 19:10:35     INFO - GECKO(2589) | [Parent 2589, Main Thread] WARNING: BounceTrackingState::OnDocumentStartRequest failed.: 'NS_SUCCEEDED(rv)', file /builds/worker/checkouts/gecko/netwerk/ipc/DocumentLoadListener.cpp:2612
[task 2024-08-22T19:10:35.503Z] 19:10:35     INFO - GECKO(2589) | [Child 2751: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 3 (7fc78c06f800) [pid = 2751] [serial = 3] [outer = 7fc78b75b600]
[task 2024-08-22T19:10:35.709Z] 19:10:35     INFO - GECKO(2589) | [Child 2807, Main Thread] WARNING: 'presShell->NeedLayoutFlush()', file /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2207
[task 2024-08-22T19:10:36.038Z] 19:10:36     INFO - GECKO(2589) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to /tmp/tmp8d_vv_ou.mozrunner/runtests_leaks_tab_pid2861.log
[task 2024-08-22T19:10:36.145Z] 19:10:36     INFO - GECKO(2589) | [Child 2861, Main Thread] WARNING: could not set real-time limit in CubebUtils::InitLibrary: file /builds/worker/checkouts/gecko/dom/media/CubebUtils.cpp:757
[task 2024-08-22T19:10:36.158Z] 19:10:36     INFO - GECKO(2589) | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to /tmp/tmp8d_vv_ou.mozrunner/runtests_leaks_tab_pid2875.log
[task 2024-08-22T19:10:36.248Z] 19:10:36     INFO - GECKO(2589) | [Child 2875, Main Thread] WARNING: could not set real-time limit in CubebUtils::InitLibrary: file /builds/worker/checkouts/gecko/dom/media/CubebUtils.cpp:757
[task 2024-08-22T19:10:36.311Z] 19:10:36     INFO - GECKO(2589) | [Child 2751, Main Thread] WARNING: NS_ENSURE_TRUE(winInner) failed: file /builds/worker/checkouts/gecko/widget/nsBaseDragService.cpp:1256
[task 2024-08-22T19:10:36.317Z] 19:10:36     INFO - GECKO(2589) | [Child 2751, Main Thread] WARNING: NS_ENSURE_TRUE(widget) failed: file /builds/worker/checkouts/gecko/widget/nsDragServiceProxy.cpp:148
[task 2024-08-22T19:10:36.719Z] 19:10:36     INFO - GECKO(2589) | [Child 2751, Main Thread] WARNING: NS_ENSURE_TRUE(winInner) failed: file /builds/worker/checkouts/gecko/widget/nsBaseDragService.cpp:1256
[task 2024-08-22T19:10:36.719Z] 19:10:36     INFO - GECKO(2589) | [Child 2751, Main Thread] WARNING: NS_ENSURE_TRUE(widget) failed: file /builds/worker/checkouts/gecko/widget/nsDragServiceProxy.cpp:148
[task 2024-08-22T19:10:36.858Z] 19:10:36     INFO - GECKO(2589) | [Child 2751, Main Thread] WARNING: PuppetWidget::GetInputContext() needs to retrieve it with IPC: file /builds/worker/checkouts/gecko/widget/PuppetWidget.cpp:728
[task 2024-08-22T19:10:37.001Z] 19:10:37     INFO - GECKO(2589) | [2589] Assertion failure: !aPrincipal->IsSystemPrincipal(), at /builds/worker/checkouts/gecko/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp:2070
[task 2024-08-22T19:10:37.017Z] 19:10:37     INFO -  Initializing stack-fixing for the first stack frame, this may take a while...
[task 2024-08-22T19:10:47.740Z] 19:10:47     INFO - GECKO(2589) | #01: nsUrlClassifierDBService::Lookup(nsIPrincipal*, nsTSubstring<char> const&, nsIUrlClassifierCallback*) [toolkit/components/url-classifier/nsUrlClassifierDBService.cpp:2070]
[task 2024-08-22T19:10:47.740Z] 19:10:47     INFO - GECKO(2589) | #02: PendingDBLookup::LookupSpecInternal(nsTSubstring<char> const&) [toolkit/components/reputationservice/ApplicationReputation.cpp:855]
[task 2024-08-22T19:10:47.741Z] 19:10:47     INFO - GECKO(2589) | #03: PendingDBLookup::LookupSpec(nsTSubstring<char> const&, LookupType const&) [toolkit/components/reputationservice/ApplicationReputation.cpp:807]
[task 2024-08-22T19:10:47.741Z] 19:10:47     INFO - GECKO(2589) | #04: PendingLookup::LookupNext() [toolkit/components/reputationservice/ApplicationReputation.cpp:0]
[task 2024-08-22T19:10:47.741Z] 19:10:47     INFO - GECKO(2589) | #05: PendingLookup::DoLookupInternal() [toolkit/components/reputationservice/ApplicationReputation.cpp:1472]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #06: ApplicationReputationService::QueryReputationInternal(nsIApplicationReputationQuery*, nsIApplicationReputationCallback*) [toolkit/components/reputationservice/ApplicationReputation.cpp:1973]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #07: ApplicationReputationService::QueryReputation(nsIApplicationReputationQuery*, nsIApplicationReputationCallback*) [toolkit/components/reputationservice/ApplicationReputation.cpp:0]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #08: ??? [/builds/worker/workspace/build/application/firefox/libxul.so + 0x39c193a]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #09: CallMethodHelper::Call() [js/xpconnect/src/XPCWrappedNative.cpp:1174]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #10: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) [js/xpconnect/src/XPCWrappedNative.cpp:1120]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #11: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) [js/xpconnect/src/XPCWrappedNativeJSOps.cpp:966]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #12: CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) [js/src/vm/Interpreter.cpp:487]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #13: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) [js/src/vm/Interpreter.cpp:581]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #14: js::Interpret(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:0]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #15: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:459]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #16: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) [js/src/vm/Interpreter.cpp:613]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #17: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) [js/src/vm/Interpreter.cpp:680]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #18: js::PromiseObject::create(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, bool) [js/src/builtin/Promise.cpp:2864]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #19: PromiseConstructor(JSContext*, unsigned int, JS::Value*) [js/src/builtin/Promise.cpp:2769]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #20: CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) [js/src/vm/Interpreter.cpp:487]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #21: CallJSNativeConstructor(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [js/src/vm/Interpreter.cpp:503]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #22: InternalConstruct(JSContext*, js::AnyConstructArgs const&, js::CallReason) [js/src/vm/Interpreter.cpp:718]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #23: js::Interpret(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:3442]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #24: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:459]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #25: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) [js/src/vm/Interpreter.cpp:613]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #26: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) [js/src/vm/Interpreter.cpp:680]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #27: js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName*>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) [js/src/vm/SelfHosting.cpp:1555]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #28: js::jit::InterpretResume(JSContext*, JS::Handle<JSObject*>, JS::Value*, JS::MutableHandle<JS::Value>) [js/src/jit/VMFunctions.cpp:1140]
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | #29: ??? (???:???)
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | ExceptionHandler::GenerateDump attempting to generate:/tmp/tmp8d_vv_ou.mozrunner/minidumps/5a0e966f-17d3-f510-6f90-211a21e5dd18.dmp
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | ExceptionHandler::GenerateDump cloned child 2906
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | ExceptionHandler::SendContinueSignalToChild sent continue signal to child
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | ExceptionHandler::WaitForContinueSignal waiting for continue signal...
[task 2024-08-22T19:10:47.743Z] 19:10:47     INFO - GECKO(2589) | ExceptionHandler::GenerateDump minidump generation succeeded
[task 2024-08-22T19:10:47.744Z] 19:10:47     INFO - GECKO(2589) | [Child 2751, IPC I/O Child] WARNING: [868FCED7EABF1706.6C501A805980DFBC]: Dropping message '<null>'; no connection to unknown peer 1.1: file /builds/worker/checkouts/gecko/ipc/glue/NodeController.cpp:364
[task 2024-08-22T19:10:47.744Z] 19:10:47     INFO - GECKO(2589) | [Child 2875, IPC I/O Child] WARNING: [FE72A9C3417C2691.C178E8972AD6F46D]: Dropping message '<null>'; no connection to unknown peer 1.1: file /builds/worker/checkouts/gecko/ipc/glue/NodeController.cpp:364
[task 2024-08-22T19:10:47.745Z] 19:10:47     INFO - GECKO(2589) | Exiting due to channel error.
[task 2024-08-22T19:10:47.745Z] 19:10:47     INFO - GECKO(2589) | [Child 2861, IPC I/O Child] WARNING: [163EA4006418DB41.ACD3B8193CEBFAD]: Dropping message '<null>'; no connection to unknown peer 1.1: file /builds/worker/checkouts/gecko/ipc/glue/NodeController.cpp:364
[task 2024-08-22T19:10:47.746Z] 19:10:47     INFO - GECKO(2589) | Exiting due to channel error.
[task 2024-08-22T19:10:47.746Z] 19:10:47     INFO - GECKO(2589) | Exiting due to channel error.
[task 2024-08-22T19:10:47.747Z] 19:10:47     INFO - GECKO(2589) | [Child 2807, IPC I/O Child] WARNING: [10981FCE2BB3A9B6.FA5F5B17F2B4655B]: Dropping message '<null>'; no connection to unknown peer 1.1: file /builds/worker/checkouts/gecko/ipc/glue/NodeController.cpp:364
[task 2024-08-22T19:10:47.748Z] 19:10:47     INFO - GECKO(2589) | [Child 2763, IPC I/O Child] WARNING: [F5ED6A45FA898DC3.34563DF02551377]: Dropping message '<null>'; no connection to unknown peer 1.1: file /builds/worker/checkouts/gecko/ipc/glue/NodeController.cpp:364
[task 2024-08-22T19:10:47.749Z] 19:10:47     INFO - GECKO(2589) | Exiting due to channel error.
[task 2024-08-22T19:10:47.749Z] 19:10:47     INFO - GECKO(2589) | Exiting due to channel error.
[task 2024-08-22T19:10:47.750Z] 19:10:47     INFO - GECKO(2589) | [Child 2750, IPC I/O Child] WARNING: [2FECB97AA8BAA762.189DA0CCBF5EE8D9]: Dropping message '<null>'; no connection to unknown peer 1.1: file /builds/worker/checkouts/gecko/ipc/glue/NodeController.cpp:364
[task 2024-08-22T19:10:47.751Z] 19:10:47     INFO - GECKO(2589) | Exiting due to channel error.
[task 2024-08-22T19:10:47.751Z] 19:10:47     INFO - GECKO(2589) | [Utility 2741, IPC I/O Child] WARNING: [9C16DCB23771DE91.37A1D3EC06078790]: Dropping message '<null>'; no connection to unknown peer 1.1: file /builds/worker/checkouts/gecko/ipc/glue/NodeController.cpp:364
[task 2024-08-22T19:10:47.752Z] 19:10:47     INFO - GECKO(2589) | [Child 2686, IPC I/O Child] WARNING: [3CBBDC1086424D5C.93A2649E143FB8C7]: Dropping message '<null>'; no connection to unknown peer 1.1: file /builds/worker/checkouts/gecko/ipc/glue/NodeController.cpp:364
[task 2024-08-22T19:10:47.753Z] 19:10:47     INFO - GECKO(2589) | Exiting due to channel error.
[task 2024-08-22T19:10:47.753Z] 19:10:47     INFO - GECKO(2589) | [Child 2653, IPC I/O Child] WARNING: [572BF5B398323FD3.3ABB4B486D40689F]: Dropping message '<null>'; no connection to unknown peer 1.1: file /builds/worker/checkouts/gecko/ipc/glue/NodeController.cpp:364
[task 2024-08-22T19:10:47.754Z] 19:10:47     INFO - GECKO(2589) | Exiting due to channel error.
[task 2024-08-22T19:10:47.755Z] 19:10:47     INFO - GECKO(2589) | [Utility 2741, Main Thread] WARNING: Shutting down Utility process early due to a crash!: file /builds/worker/checkouts/gecko/ipc/glue/UtilityProcessChild.cpp:358
[task 2024-08-22T19:10:47.755Z] 19:10:47     INFO - TEST-INFO | Main app process: exit 11
[task 2024-08-22T19:10:47.756Z] 19:10:47    ERROR - TEST-UNEXPECTED-FAIL | ShutdownLeaks | process() called before end of test suite
[task 2024-08-22T19:10:47.757Z] 19:10:47    ERROR - TEST-UNEXPECTED-FAIL | did not see DOCSHELL log strings. this occurs if the DOCSHELL logging gets disabled by something. 15 created seen 0 destroyed seen
[task 2024-08-22T19:10:47.757Z] 19:10:47    ERROR - TEST-UNEXPECTED-FAIL | did not see DOMWINDOW log strings. this occurs if the DOMWINDOW logging gets disabled by something42 created seen 0 destroyed seen
[task 2024-08-22T19:10:47.758Z] 19:10:47     INFO - Buffered messages logged at 19:10:35
[task 2024-08-22T19:10:47.759Z] 19:10:47     INFO - Entering setup bound 
[task 2024-08-22T19:10:47.759Z] 19:10:47     INFO - TEST-PASS | browser/components/screenshots/tests/browser/browser_keyboard_shortcuts.js | The screenshots button was added to the nav bar - "[object XULElement]" == true -
Flags: needinfo?(nbaumgardner)
Flags: needinfo?(nbaumgardner)

:dimi, When a user creates a screenshot, we currently offer them the choice of either copying it to the clipboard, or downloading it. Downloading means it ends up somewhere expected, and we get the entry in the download list and confirmation hint on success. It also means we attempt to pass it through the safe browsing checks, and this fails with this patch as the blob URI is created by privileged code and has the system principal.

the patch in attachment 9413054 [details] wants to avoid re-serializing the screenshot into a data URI and use a blob URI instead. Its blocked on the URL classifier throwing when it encounters the blob URI and its system principal. Gijs wrote up the results of some digging in https://docs.google.com/document/d/1N7Gs-JZz5tsGTJapXZVq4JkWMNZ2kdwMWQMif_7Da0g/edit?usp=sharing, and proposes a few options:

  • make nsUrlClassifierDBService::Lookup or its caller PendingDBLookup::LookupSpecInternal just bail out for system principals without asserting.
  • make the downloads code not call into the application reputation / url classifier code for blob URLs with system principal
  • make screenshots generate blobs that have a non-system principal associated with them

With 1. being probably the "best" option. What are your thoughts?

Flags: needinfo?(nbaumgardner) → needinfo?(dlee)

(In reply to Sam Foster [:sfoster] (he/him) from comment #6)

  • make nsUrlClassifierDBService::Lookup or its caller PendingDBLookup::LookupSpecInternal just bail out for system principals without asserting.
  • make the downloads code not call into the application reputation / url classifier code for blob URLs with system principal
  • make screenshots generate blobs that have a non-system principal associated with them

With 1. being probably the "best" option. What are your thoughts?

Thank you both for digging into this issue!
Yes, I agree that option 1 is the best solution.
I think we should not let the caller determine whether to skip the application reputation check (option 2).
Option 3 is a workaround for the issue and would also result in unnecessary Safe Browsing lookups.
So let's fix this with option 1!

Flags: needinfo?(dlee)
Attachment #9413054 - Attachment is obsolete: true
Pushed by nbaumgardner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3af3ffb0b713 Switch to OffscreenCanvas and blobs for screenshots. r=sfoster,dimi

https://hg.mozilla.org/mozilla-central/rev/3af3ffb0b713

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
status-firefox140: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 140 Branch
QA Whiteboard: [qa-triage-done-c141/b140]
See Also: → 1935810
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: