Closed Bug 1690644 Opened 4 years ago Closed 1 year ago

Structured cloning the data from ChromeUtils.requestProcInfo() fails due to contained nsIURI instances

Categories

(Core :: General, enhancement, P5)

enhancement

Tracking

()

RESOLVED WONTFIX

People

(Reporter: whimboo, Unassigned)

Details

I'm using ChromeUtils.requestProcInfo() from within a WebExtension experiment, and trying to transfer the data to the background script fails, because of structured cloning not being able to handle the documentURI instances of the processes window instances:

https://searchfox.org/mozilla-central/source/dom/chrome-webidl/ChromeUtils.webidl#568

As Nika mentioned on Element:

There's just no code to try to serialize any nsIURI objects in structured clone

Summary: ChromeUtils.requestProcInfo() fails to JSON.serialize due to contained nsIURI instances → Structurerd cloning the data from ChromeUtils.requestProcInfo() fails due to contained nsIURI instances

It would make sense to make the method to return only data which can be structured cloned easily.
So perhaps documentURI could be simply a DOMString. aboutprocesses.js does seem to rely on nsIURI API, but could it create such instance when needed?

Component: DOM: Core & HTML → General
Severity: -- → N/A
Type: defect → enhancement
Priority: -- → P5

Florian, would that be fine with you?

Flags: needinfo?(florian)
Summary: Structurerd cloning the data from ChromeUtils.requestProcInfo() fails due to contained nsIURI instances → Structured cloning the data from ChromeUtils.requestProcInfo() fails due to contained nsIURI instances

I wonder if the URI is something we could access in the about:processes front-end from the outerWindowId.

(In reply to Olli Pettay [:smaug] from comment #1)

So perhaps documentURI could be simply a DOMString. aboutprocesses.js does seem to rely on nsIURI API, but could it create such instance when needed?

The URI converted as a string could be quite large (eg. it could be a 5MB screenshot as a base64 encoded data url), am I guessing correctly that having a URI object in the interface currently avoids doing string copies repeatedly?

(In reply to Florian Quèze [:florian] from comment #3)

The URI converted as a string could be quite large (eg. it could be a 5MB screenshot as a base64 encoded data url), am I guessing correctly that having a URI object in the interface currently avoids doing string copies repeatedly?

I assume this question was targeted towards Olli.

Flags: needinfo?(bugs)

It is true that nsIURI object avoids possible memory copy. Accessing uri.spec makes a copy too (looks like about processes may do that).

The huge memory use would be there still when passing the structured clone around.
I guess one needs to filter out the possible massive data before passing it to the other process, in which case changing the API might not be too useful.

Flags: needinfo?(bugs)

As an aside: If we're producing 5 MiB screenshots internally in Firefox as data URIs, we should probably stop doing that in favor of some use of Blobs (which will have sessionrestore problems compared to data URLs unless the sessionstore grows some special-casing to re-mint blobs) and/or ServiceWorker-like things (which should avoid the sessionrestore problems and should be feasible via WebExtensions in the medium term). If it's from test automation, those should probably switch to just creating artifacts via some combination of log processing (if the screenshots must be emitted inline in the log with no ability to write screenshots directly to a file) and/or helping the treeherder log viewer know how to magically linkify such references.

Does it mean that we better have to leave the behavior as is?

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #7)

Does it mean that we better have to leave the behavior as is?

Not 100% sure if this question is about structured cloning, but to be super clear: Structured cloning as-implemented is exposed to the web/content and it cannot be unilaterally extended by Firefox without webcompat ramifications and potential logistical problems down the road because the data can also be stored in IndexedDB. Trying to create specialized variants for the system principal is possible but highly inadvisable, as we generally also want to be using the system principal in fewer places.

If the question is about changing what's returned by the existing WebIDL interface to be more structured cloning friendly, that seems fine modulo the memory usage meta-concerns.

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #2)

Florian, would that be fine with you?

Clearing my needinfo as this was answered in the comments 3 and 5. It doesn't seem the benefits of changing this API outweigh the disadvantages, so I'll close the bug as WONTFIX.

Status: NEW → RESOLVED
Closed: 1 year ago
Flags: needinfo?(florian)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.