1770944 - Remove dom/browser-element/BrowserElementParent.jsm

Status: RESOLVED FIXED

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

Description

[Andrew McCreight [:mccr8]]

It sounds like this isn't used any more, so we should try removing it. Gijs said it was confusing to have this around, because it kind of looks like something useful, but it isn't actually.

Comment 1

[Andrew McCreight [:mccr8]]

jstutte said jkrause might be able to work on this, so I'll mark them as the assignee for now.

Comment 2

[Jan Rio Krause]

Yes, I'm on it. Thank you.

Comment 3

[Kris Maglione [:kmag]]

Note that the various references to the "mozbrowser" attribute should also be removed.

Please flag me for review. Thanks!

Comment 4

[Andrew McCreight [:mccr8]]

Presumably everything under dom/browser-element/ can be removed. You should also talk to smaug after that happens, so he can deal with getting the module for this code removed.

Comment 5

[Jan Rio Krause]

I will split it into several commits to make it easier to review.

Comment 6

[Jan Rio Krause]

Attachment: Bug 1770944 - Remove `dom/browser-element`. r=smaug

Comment 7

[Emilio Cobos Álvarez [:emilio]]

I think we should also remove dom/interfaces/html/nsIMozBrowserFrame.idl and related code.

Comment 8

[Emilio Cobos Álvarez [:emilio]]

(Thanks for working on this!)

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Module owner thanks :)

And yes, please remove the interface too and all the places it is used. That can be a separate patch.
Looks like there is also nsIDOMMozBrowserFrame.

Comment 11

[Jan Rio Krause]

Attachment: Bug 1770944 - Remove `nsIMozBrowserFrame`. r=smaug

Depends on D155254

Comment 12

[Jan Rio Krause]

Attachment: Bug 1770944 - Remove `nsIDOMMozBrowserFrame`. r=smaug

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jkrause, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

jkrause, hmm, should we land the patches.
https://phabricator.services.mozilla.com/D175898 has some error in moz.build

Comment 15

[Jan Rio Krause]

I will fix the errors in moz.build.

Lastly, there will be some more patches removing the attribute mInIsolatedMozBrowser and some dangling textual references of mozbrowser.

What about deprecated methods like nsIDocShell getSameTypeInProcessParentIgnoreBrowserBoundaries()?

Comment 16

[Jan Rio Krause]

Attachment: Bug 1770944 - Remove `nsGkAtoms::mozbrowser`. r=smaug

Depends on D175898

Comment 17

[Jan Rio Krause]

Attachment: Bug 1770944 - Remove `isInIsolatedMozBrowserElement`. r=smaug

Depends on D183229

Comment 18

[Artur Iunusov]

So, this patch must be finished (I am working on it): https://phabricator.services.mozilla.com/D183230

plus, removing mInIsolatedMozBrowser and mozbrowser

Comment 19

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1770944 - Remove other references to inBrowserElement, r=janv

Comment 20

[Emilio Cobos Álvarez [:emilio]]

(In reply to Artur Iunusov from comment #18)

So, this patch must be finished (I am working on it): https://phabricator.services.mozilla.com/D183230

plus, removing mInIsolatedMozBrowser and mozbrowser

Ah, I took the liberty of rebasing and fixing the build / killing a bunch of other code in https://phabricator.services.mozilla.com/D189490. Feel free to reuse whatever you need, sorry, didn't know it was actively being worked on.

Comment 21

[Artur Iunusov]

No worries. I can update and put D183230 patch to the review and include removal of the other stuff ("dangling textual references of mozbrowser" and /dom/browser-element)

Comment 22

[Artur Iunusov]

Attachment: Bug 1770944 - remove dangling textual references of mozbrowser, r=smaug

Depends on D189490

Comment 23

[Artur Iunusov]

need basically just a try server run, to confirm that nothing is broken

Comment 24

[Artur Iunusov]

sounds like all of these references must be also removed:

https://searchfox.org/mozilla-central/search?q=browser-element&path=&case=true&regexp=false

Comment 25

[Artur Iunusov]

Attachment: Bug 1770944 - remove /dom/browser-element/ path from other places, r=smaug

Comment 26

[Artur Iunusov]

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

Comment 27

[Artur Iunusov]

Attachment: Bug 1770944 - removed unused ProcessPriorityManagerImpl::ResetPriority

Depends on D190918

Comment 28

[Artur Iunusov]

some build failures on try server https://treeherder.mozilla.org/push-health/push?repo=try&revision=2c0ceccc682e724afbe1a7a534dc3b38ffb76aa1&tab=builds

could not reproduce locally, though (still looking the logs. UPD: fixed)

Comment 29

[Artur Iunusov]

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

Comment 30

[Artur Iunusov]

https://treeherder.mozilla.org/push-health/push?repo=try&revision=d6486bfc8316d38270c4ef2457777654b1c1d0ff&tab=tests&testGroup=pr

Comment 31

[Artur Iunusov]

Attachment: Bug 1770944 - Remove non-existent test_swapFrameLoaders, r=smaug

Comment 32

[Artur Iunusov]

Attachment: Bug 1770944 - fixed isolatedMozBrowser related tests, r=smaug

Comment 33

[Artur Iunusov]

Attachment: Bug 1770944: test_cookies_upgrade_10.js: bump schema version, r=smaug

Comment 34

[Artur Iunusov]

Attachment: Bug 1770944: Fixed permission manager tests, r=smaug

Comment 35

[Artur Iunusov]

Attachment: Bug 1770944 - fixed test_cookiejars.js, r=smaug

• as a part of "cookie jars" for apps feature,
  see https://bugzilla.mozilla.org/show_bug.cgi?id=756648

Comment 36

[Artur Iunusov]

Attachment: Bug 1770944 - Fixed all test_permmanager_migrate.js tests, r=smaug

-before removal isInBrowserElement and migration to the new schema

Comment 37

[Artur Iunusov]

Attachment: WIP: Bug 1770944 - Fixed dom\quota\test\xpcshell\upgrades tests, r=smaug

• still expect AppID, mInIsolatedMozBrowser and parse it
• but keep unused

Comment 39

[Artur Iunusov]

Attachment: Bug 1770944 - test_permmanager_matches:removed inIsolatedMozBrowser related tests, r=smaug

Comment 41

[Jan Varga [:janv]]

Attachment: Bug 1770944 - Fix StorageOriginAttributes::CreateSuffix; r=#dom-storage

This patch cleans up some existing code and adds full support for origin
attribute serialization.

Comment 42

[Artur Iunusov]

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

Comment 43

[Artur Iunusov]

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

Comment 44

[Artur Iunusov]

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

last try push looks good. Let's land it right after soft freeze

Comment 45

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b18524f541e
Remove `dom/browser-element`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/a4197071d10a
Remove `nsIMozBrowserFrame`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/55b0886f9025
Remove `nsIDOMMozBrowserFrame`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/1828b66c2f7c
Remove `nsGkAtoms::mozbrowser`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/9ea253525227
Remove `isInIsolatedMozBrowserElement`. r=smaug,necko-reviewers,kershaw,valentin
https://hg.mozilla.org/integration/autoland/rev/e3960ad85182
Remove other references to inBrowserElement, r=cookie-reviewers,valentin,janv,decoder
https://hg.mozilla.org/integration/autoland/rev/4b6ae7dd0e69
Fix StorageOriginAttributes::CreateSuffix; r=aiunusov
https://hg.mozilla.org/integration/autoland/rev/3a149ef794c2
remove dangling textual references of mozbrowser, r=smaug
https://hg.mozilla.org/integration/autoland/rev/0a138f3b56b9
remove /dom/browser-element/ path from other places, r=smaug,zeid
https://hg.mozilla.org/integration/autoland/rev/708b2671410e
removed unused ProcessPriorityManagerImpl::ResetPriority r=smaug
https://hg.mozilla.org/integration/autoland/rev/be28bb62e9f3
Remove non-existent test_swapFrameLoaders, r=smaug
https://hg.mozilla.org/integration/autoland/rev/0d4318bf3239
fixed isolatedMozBrowser related tests, r=smaug,necko-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/97aa839dc200
Fixed permission manager tests, r=smaug
https://hg.mozilla.org/integration/autoland/rev/15661e24d8e1
fixed test_cookiejars.js, r=smaug,necko-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/23d0dc98eb51
Fixed all test_permmanager_migrate.js tests, r=smaug
https://hg.mozilla.org/integration/autoland/rev/b73885731e73
test_permmanager_matches:removed inIsolatedMozBrowser related tests, r=smaug

Comment 46

[Sandor Molnar[:smolnar]]

Backed out for causing asan failures @ dom/base/InProcessBrowserChildMessageManager.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/992ec16aa47ba11f567284847dd2c394ef7f0ac9

Push with failures

Failure log -> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /builds/worker/checkouts/gecko/dom/base/InProcessBrowserChildMessageManager.cpp

Comment 47

[Artur Iunusov]

Thanks! Looking into it.

Comment 48

[Artur Iunusov]

Okay, was able to reproduce it locally

Comment 49

[Artur Iunusov]

Fixed.

Comment 50

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fb5a54b3cbe9
Remove `dom/browser-element`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/9ba54ab06348
Remove `nsIMozBrowserFrame`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/36c378ba8212
Remove `nsIDOMMozBrowserFrame`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/bd53c42038f7
Remove `nsGkAtoms::mozbrowser`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/f65e0967ad75
Remove `isInIsolatedMozBrowserElement`. r=smaug,necko-reviewers,kershaw,valentin
https://hg.mozilla.org/integration/autoland/rev/14952f872b77
Remove other references to inBrowserElement, r=cookie-reviewers,valentin,janv,decoder
https://hg.mozilla.org/integration/autoland/rev/63ac518aceb0
Fix StorageOriginAttributes::CreateSuffix; r=aiunusov
https://hg.mozilla.org/integration/autoland/rev/79de4f83fe2e
remove dangling textual references of mozbrowser, r=smaug
https://hg.mozilla.org/integration/autoland/rev/9416bafd9982
remove /dom/browser-element/ path from other places, r=smaug,zeid
https://hg.mozilla.org/integration/autoland/rev/0a8c4c2460ee
removed unused ProcessPriorityManagerImpl::ResetPriority r=smaug
https://hg.mozilla.org/integration/autoland/rev/58e02566db85
Remove non-existent test_swapFrameLoaders, r=smaug
https://hg.mozilla.org/integration/autoland/rev/0d2432765ca0
fixed isolatedMozBrowser related tests, r=smaug,necko-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/6435f48c96bf
Fixed permission manager tests, r=smaug
https://hg.mozilla.org/integration/autoland/rev/8a217eff7bcd
fixed test_cookiejars.js, r=smaug,necko-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/4ff0c45db93b
Fixed all test_permmanager_migrate.js tests, r=smaug
https://hg.mozilla.org/integration/autoland/rev/61af32f40777
test_permmanager_matches:removed inIsolatedMozBrowser related tests, r=smaug

Comment 51

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/fb5a54b3cbe9
https://hg.mozilla.org/mozilla-central/rev/9ba54ab06348
https://hg.mozilla.org/mozilla-central/rev/36c378ba8212
https://hg.mozilla.org/mozilla-central/rev/bd53c42038f7
https://hg.mozilla.org/mozilla-central/rev/f65e0967ad75
https://hg.mozilla.org/mozilla-central/rev/14952f872b77
https://hg.mozilla.org/mozilla-central/rev/63ac518aceb0
https://hg.mozilla.org/mozilla-central/rev/79de4f83fe2e
https://hg.mozilla.org/mozilla-central/rev/9416bafd9982
https://hg.mozilla.org/mozilla-central/rev/0a8c4c2460ee
https://hg.mozilla.org/mozilla-central/rev/58e02566db85
https://hg.mozilla.org/mozilla-central/rev/0d2432765ca0
https://hg.mozilla.org/mozilla-central/rev/6435f48c96bf
https://hg.mozilla.org/mozilla-central/rev/8a217eff7bcd
https://hg.mozilla.org/mozilla-central/rev/4ff0c45db93b
https://hg.mozilla.org/mozilla-central/rev/61af32f40777

Comment 52

[Andrew Sutherland [:asuth] (he/him)]

https://phabricator.services.mozilla.com/D189490 made some unreviewed changes to XPCOM JSM-related code that sound like they broke Thunderbird and I'm going to pursue getting this backed out per discussion in the XPCOM matrix channel.

Comment 53

[Narcis Beleuzu [:NarcisB]]

Backed out as requested

Backout link: https://hg.mozilla.org/integration/autoland/rev/35f442841c45a23c95df5814cb049c54feafce22

Comment 54

[Artur Iunusov]

Looking into it. Thanks!

Comment 55

[Artur Iunusov]

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

Comment 56

[Artur Iunusov]

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

Comment 57

[Artur Iunusov]

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

Comment 58

[Artur Iunusov]

https://treeherder.mozilla.org/jobs?repo=try&revision=26dd81740262bcdaebb337c47c056dce0b161601 push without changes

https://treeherder.mozilla.org/jobs?repo=try&revision=db7d6558b2a559a245f3ffbf992baffcba15fc24
https://treeherder.mozilla.org/jobs?repo=try&revision=771a7f7a37ff484d2d63eb46085dbd426f4e967e - with the changes

Comment 59

[Artur Iunusov]

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

Comment 60

[Artur Iunusov]

try push looks fine, except the test that were broken before my changes:
(UNEXPECTED PASS)
browser/components/firefoxview/tests/browser/browser_history_firefoxview.js
browser/components/screenshots/tests/browser/browser_screenshots_focus_test.js
editing/crashtests/insertparagraph-in-listitem-in-svg-followed-by-collapsible-spaces.html
mediacapture-streams/crashtests/enumerateDevices-after-discard-1.https.html

So. I think we can land it

Comment 61

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1b442b0a2086
Remove `dom/browser-element`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/a02fdaf52893
Remove `nsIMozBrowserFrame`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/fa5382cdad83
Remove `nsIDOMMozBrowserFrame`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/da61e4f3f305
Remove `nsGkAtoms::mozbrowser`. r=smaug
https://hg.mozilla.org/integration/autoland/rev/35b608130b74
Remove `isInIsolatedMozBrowserElement`. r=smaug,necko-reviewers,kershaw,valentin
https://hg.mozilla.org/integration/autoland/rev/7ad01126dfc5
Remove other references to inBrowserElement, r=cookie-reviewers,valentin,janv,decoder
https://hg.mozilla.org/integration/autoland/rev/0f276d3694d5
Fix StorageOriginAttributes::CreateSuffix; r=aiunusov
https://hg.mozilla.org/integration/autoland/rev/d302f6d7c392
remove dangling textual references of mozbrowser, r=smaug
https://hg.mozilla.org/integration/autoland/rev/e9bbef712077
remove /dom/browser-element/ path from other places, r=smaug,zeid
https://hg.mozilla.org/integration/autoland/rev/f42069fedaa5
removed unused ProcessPriorityManagerImpl::ResetPriority r=smaug
https://hg.mozilla.org/integration/autoland/rev/af3de5c08748
Remove non-existent test_swapFrameLoaders, r=smaug
https://hg.mozilla.org/integration/autoland/rev/87dae4f52865
fixed isolatedMozBrowser related tests, r=smaug,necko-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/3cd42773638e
Fixed permission manager tests, r=smaug
https://hg.mozilla.org/integration/autoland/rev/afa2d0e7f98f
fixed test_cookiejars.js, r=smaug,necko-reviewers,jesup
https://hg.mozilla.org/integration/autoland/rev/56eb48601758
Fixed all test_permmanager_migrate.js tests, r=smaug
https://hg.mozilla.org/integration/autoland/rev/9f05ae959301
test_permmanager_matches:removed inIsolatedMozBrowser related tests, r=smaug

Comment 62

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/1b442b0a2086
https://hg.mozilla.org/mozilla-central/rev/a02fdaf52893
https://hg.mozilla.org/mozilla-central/rev/fa5382cdad83
https://hg.mozilla.org/mozilla-central/rev/da61e4f3f305
https://hg.mozilla.org/mozilla-central/rev/35b608130b74
https://hg.mozilla.org/mozilla-central/rev/7ad01126dfc5
https://hg.mozilla.org/mozilla-central/rev/0f276d3694d5
https://hg.mozilla.org/mozilla-central/rev/d302f6d7c392
https://hg.mozilla.org/mozilla-central/rev/e9bbef712077
https://hg.mozilla.org/mozilla-central/rev/f42069fedaa5
https://hg.mozilla.org/mozilla-central/rev/af3de5c08748
https://hg.mozilla.org/mozilla-central/rev/87dae4f52865
https://hg.mozilla.org/mozilla-central/rev/3cd42773638e
https://hg.mozilla.org/mozilla-central/rev/afa2d0e7f98f
https://hg.mozilla.org/mozilla-central/rev/56eb48601758
https://hg.mozilla.org/mozilla-central/rev/9f05ae959301