1695381 - Remote GTK icon lookup for moz-icon:// URLs

Status: RESOLVED FIXED

(Core :: Widget: Gtk, task)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

The one remaining use of GTK in content processes (besides WebGL) is to look up icons for moz-icon:// URLs; they can be referenced from file:// content, and are used for directory listings.

This can be reproduced by setting security.sandbox.content.headless and opening a directory; the icons will be missing or replaced by alt text. There's also a reftest of image downscaling applies to moz-icon that fails; disturbingly, nothing else in the testsuite seems to have caught this.

Web content isn't allowed to reference moz-icons directly, but FTP directory listings currently use moz-icon in a remote content process, and there may be other cases where remote content results in resource: or chrome: content using icons.

Assuming there's nothing in that last category then, in the short term for Fission, we can continue loading GTK in the dedicated file:// content process, and in content processes for FTP origins if they're still supported, but in the long term for sandboxing we'll need a general solution.

I don't yet know if other platforms will have similar issues, and if it would be better to remote moz-icon:// in a cross-platform way vs. do something GTK-specific. These images are (hopefully?) small enough that we could send them inline in IPC messages.

Comment 1

[Timothy Nikkel (:tnikkel)]

moz-icon is not well tested, there are a few more reftests in image/reftest/generic and there is a mochitest that is run at least on Windows that tests an edge case of moz-icon but I can't remember where it is.

Comment 2

[Timothy Nikkel (:tnikkel)]

The test I was thinking of was
https://searchfox.org/mozilla-central/source/image/test/mochitest/test_bug415761.html

I also found
https://searchfox.org/mozilla-central/source/image/test/unit/test_moz_icon_uri.js
https://searchfox.org/mozilla-central/source/netwerk/test/unit/test_URIs.js

Comment 3

[:gerard-majax]

(In reply to Timothy Nikkel (:tnikkel) from comment #2)

[...]

I also found
https://searchfox.org/mozilla-central/source/image/test/unit/test_moz_icon_uri.js

This one seems to be testing the moz-icon protocol itself, and I see it passing locally (albeit weirdly crashing like in bug 1186769) when trying to manually load moz-icon://.txt?size=128 fails :)

Comment 4

[:gerard-majax]

Attachment: Bug 1695381 - Refactor GTK icon processing in preparation for remoting. r?tnikkel

Comment 5

[:gerard-majax]

Attachment: Bug 1695381 - Use IPC for content processes' moz-icon URL loading. r?nika,tnikkel

Depends on D118595

Comment 6

[:gerard-majax]

Attachment: Bug 1695381 - Add moz-icon testing for file:// r?tnikkel

Depends on D118596

Comment 7

[:gerard-majax]

(In reply to Jed Davis [:jld] ⟨⏰|UTC-6⟩ ⟦he/him⟧ from comment #0)

This can be reproduced by setting security.sandbox.content.headless and opening a directory; the icons will be missing or replaced by alt text. There's also a reftest of image downscaling applies to moz-icon that fails; disturbingly, nothing else in the testsuite seems to have caught this.

I suspect it is this one?

Unexpected Results
------------------
image/test/reftest/generic/moz-icon-blank-1-almostref.html == image/test/reftest/generic/moz-icon-blank-1-ref.html
  FAIL image comparison, max difference: 142, number of differing pixels: 209

Comment 8

[:gerard-majax]

So those tests are failing for me (linux):

Unexpected Results
------------------
image/test/reftest/generic/moz-icon-blank-1.html == image/test/reftest/generic/moz-icon-blank-1-ref.html
  FAIL image comparison, max difference: 142, number of differing pixels: 209
image/test/reftest/generic/moz-icon-blank-1-almostref.html == image/test/reftest/generic/moz-icon-blank-1-ref.html
  FAIL image comparison, max difference: 142, number of differing pixels: 209

Worst, it's even broken on central, it seems unrelated to those changes.

Comment 9

[:gerard-majax]

FTR, the reftests on try seems to be OK, so this could be a local-only issue?

Comment 10

[:gerard-majax]

https://treeherder.mozilla.org/jobs?repo=try&revision=68002ecd813869260a6322887e901b7f6d96698d

Comment 11

[Timothy Nikkel (:tnikkel)]

(In reply to Alexandre LISSY :gerard-majax from comment #9)

FTR, the reftests on try seems to be OK, so this could be a local-only issue?

If it works on try I'm not too surprised if it fails locally, lots of tests are finicky like that, and these ones seem especially dependant on local configuration.

Comment 12

[:gerard-majax]

https://treeherder.mozilla.org/jobs?repo=try&revision=e6d5b109474b52d9e4387ed67e936e70c0605be7

Comment 13

[:gerard-majax]

https://treeherder.mozilla.org/jobs?repo=try&revision=b53374fad1b78d91ff067e39bc7203bb2e03033a

Comment 14

[:gerard-majax]

https://treeherder.mozilla.org/jobs?repo=try&revision=db4dcf52bc921919cd369a6d97431e8d36b19846

Comment 15

[:gerard-majax]

So the current bug itself tests well with the pref to disable remoting, but this is regressed once mixed with bug 1635451: https://treeherder.mozilla.org/jobs?repo=try&revision=a86ab5eb25d173c5cade9bb8105da30027c19c65&selectedTaskRun=Yh3AlxsNTLybNeAZQzF4VQ.0

Comment 16

[:gerard-majax]

https://treeherder.mozilla.org/jobs?repo=try&revision=7c836ae7dd3a1992b99da7f718ccd71cc8e03ab1

Comment 17

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #15)

So the current bug itself tests well with the pref to disable remoting, but this is regressed once mixed with bug 1635451

We will just remove the temp pref in bug 1635451

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9264e5f7187
Refactor GTK icon processing in preparation for remoting. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/bfb9977f68cc
Use IPC for content processes' moz-icon URL loading. r=tnikkel,nika
https://hg.mozilla.org/integration/autoland/rev/3af973686928
Add moz-icon testing for file:// r=tnikkel

Comment 19

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/f9264e5f7187
https://hg.mozilla.org/mozilla-central/rev/bfb9977f68cc
https://hg.mozilla.org/mozilla-central/rev/3af973686928