1910849 - nsIWindowWatcher::OpenWindowWithRemoteTab doesn't work on GeckoView

Status: RESOLVED FIXED

(GeckoView :: General, defect, P2)

Description

[Makoto Kato [:m_kato]]

Some mochitests seems to set browser.link.open_newwindow=2 (nsIBrowserDOMWindow.OPEN_NEWWINDOW) such as test_notification_serviceworker_openWindow.html.

When setting it, window.open etc might call nsIWindowWatcher::OpenWindowWithRemoteTab to create new Chrome window. But since it doesn't use GeckoView's delegation (GeckoView:OnNewSession etc), window creation will be failed.

Current GeckoView architecture is that new tab is same as new window and we don't consider that anyone modifies browser.link.open_newwindow.

So, I think we have to handle which the following.

• nsWindowWatcher::GetWindowOpenLocation shouldn't return nsIBrowserDOMWindow.OPEN_NEWWINDOW on GeckoView even if browser.link.open_newwindow=2.
  Or
• Implement GeckoView version of nsIWindowWatcher::OpenWindowWithRemoteTab (or nsWindowWatcher::CreateChromeWindow).

Comment 1

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

The severity field is not set for this bug.
:owlish, could you have a look please?

For more information, please visit BugBot documentation.

Comment 2

[[:owlish] 🦉 PST]

Hey Makoto, is this something you are planning to work on?

Comment 3

[Makoto Kato [:m_kato]]

Short term is No, but we have to consider something way such as ignoring browser.link.open_newwindow preference.

Comment 4

[Makoto Kato [:m_kato]]

I have landed bug 1825910, so I should fix this to turn on mochitest.

Comment 5

[Makoto Kato [:m_kato]]

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

Comment 6

[Makoto Kato [:m_kato]]

Attachment: Bug 1910849 - Don't use nsIWindowWatcher::OpenWindowWithRemoteTab on GeckoView. r=#dom-core

Chrome window has one browser element only in GeckoView. So even if
browser (Fenix etc) shows tabs, it is window.

When creating chrome window, we have to call Java code via
nsIBrowserDOMWindow.createContentWindow or
nsIBrowserDOMWindow.createContentWindowInFrame. So if we open
geckoview.xhtml directly, geckoview.xhtml throws an exception due to
missing initializing parameters.

Although test_notification_serviceworker_openWindow.html modifies
browser.link.open_newwindow preference, some values won't work since
OpenWindowWithRemoteTab is used to create browser and window.

OpenWindowWithRemoteTab opens geckoview.xhtml directly, so it doesn't
work on GeckoView.

I would like to avoid this call and OpenWindowWithRemoteTab should
return an error in GeckoView immediately.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bab37caaf04f
Don't use nsIWindowWatcher::OpenWindowWithRemoteTab on GeckoView. r=dom-core,farre

Comment 8

[amarc]

Backed out for causing reftest crashes @ nsWindowWatcher::OpenWindowWithRemoteTab

• Backout link
• Push with failures
• Failure Log
• Failure line:

REFTEST PROCESS-CRASH | MOZ_RELEASE_ASSERT(false) (GeckoView should use nsIBrowserDOMWindow instead) [@ nsWindowWatcher::OpenWindowWithRemoteTab] | layout/svg/crashtests/696326-1.svg

Comment 9

[Makoto Kato [:m_kato]]

I make sense this crash since I have added this by farre suggestion. But, hmm I cannot reproduce this...

Comment 10

[Makoto Kato [:m_kato]]

Ah, this occurs without fission, I see.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf808a1cffef
Don't use nsIWindowWatcher::OpenWindowWithRemoteTab on GeckoView. r=dom-core,farre

Comment 12

[Atila Butkovits]

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