1684967 - Content not loaded when page loads a document with target="_blank"

Status: VERIFIED FIXED

(GeckoView :: General, defect, P1)

Description

[Kevin Brosnan [Ex-Mozilla]]

From github: https://github.com/mozilla-mobile/fenix/issues/16748.

Steps to reproduce

1. open https://bbs.kafan.cn/forum.php?mobile=no

2. type something in the search box, then press the enter key on the keyboard, not a button on the web page, it will pop up a blank page
   

3. the first page died

Expected behavior

Actual behavior

Device information

• Android device: ?
• Fenix version: ?
  Nightly 201123 17:01 (Build #2015777355)
  AC: 68.0.20201122190139, 52be4ca37
  GV: 85.0a1-20201122093438
  AS: 67.0.0

Change performed by the Move to Bugzilla add-on.

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

Same behaviour on https://www.excite.com/.

1. Type your query in search box.
2. Press Enter on your keyboard. If you press the search button there, then the service work.

New page with about:blank opened. All Tabs will freezed; nothing work, not even refresh, until I Force Close or Kill Firefox.

Edit: Forgot;

• Firefox Nightly 201127 05:00 (Build #2015778025)
• Samsung SM-T555, Android 7.1.1 armv7
• Xiaomi Redmi 4A, Android 7.12 arm64

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

Maybe related to bug 1588027

Comment 3

[[ex-Mozilla] Agi Sferro | :agi]

bisect

Comment 4

[[ex-Mozilla] Agi Sferro | :agi]

 8:38.59 INFO: No more integration revisions, bisection finished.
 8:38.59 INFO: Last good revision: c84ca8e6e685d14d5877e55713f65ceaf3e51780
 8:38.59 INFO: First bad revision: e29df844f897b1efc5f07f16925c89e1835ee60d
 8:38.59 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c84ca8e6e685d14d5877e55713f65ceaf3e51780&tochange=e29df844f897b1efc5f07f16925c89e1835ee60d

Looks like this is a regression from Bug 1672095. Makoto, could you take a look?

Comment 6

[Makoto Kato [:m_kato]]

Before landing bug 1612802, this still occurs. I have to check correct regression range.

Comment 7

[Makoto Kato [:m_kato]]

This seems to be regression by bug 1651705, not bug 1672095. Bug 1672095 is revert fix of bug 1612802 on Android only.

I guess that IME_BLUR is fired after SetOnBrowserChild is called. Then we dispose current GeckoEditableChild unfortunately...

Comment 8

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4dd0d42bbfa9c96c1be37c9f96e1f5ff2a59bef7

Comment 9

[Makoto Kato [:m_kato]]

I need a kind of blocker class to use GeckoEditable and dispose it. Also, we cannot write unit test for this since this depends on naive key event and native browser implementation (creating new window and changing focus to it). gv-junit and mochitest are the lack of each functions.

Comment 10

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=12b51bb5418716dea8e45d5767728b0b29199fda

Comment 11

[Makoto Kato [:m_kato]]

Attachment: Bug 1684967 - Add navigation test for form submission. r=#geckoview-reviewers

NavigationDelegateTest has no form submission test.
So I would like to add this.

Comment 12

[Makoto Kato [:m_kato]]

Attachment: Bug 1684967 - Don't dispose GeckoEditableChild during JNI call. r=#geckoview-reviewers

This is a regression by bug 1651705.

After landing it, we use read-write lock to access GeckoEditableChild object.
But when using form submission with target=_blank, since we use some nested
event loop to open new window in same process, we may try to dispose
GeckoEditableChild even if it is still used. Then, it may be dead lock.

So I add a blocker class helper not to dispose GeckoEditableChild immediately.

Depends on D103741

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf1ea09206ac
Add navigation test for form submission. r=geckoview-reviewers,agi
https://hg.mozilla.org/integration/autoland/rev/e9d727985226
Don't dispose GeckoEditableChild during JNI call. r=geckoview-reviewers,aklotz

Comment 14

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/cf1ea09206ac
https://hg.mozilla.org/mozilla-central/rev/e9d727985226

Comment 15

[Kevin Brosnan [Ex-Mozilla]]

[Tracking Requested - why for this release]: Might want to take this on Beta. Fixes a fairly easy to hit web compat issue on several sites including bing.com

Comment 16

[Makoto Kato [:m_kato]]

Comment on attachment 9200601 [details]
Bug 1684967 - Don't dispose GeckoEditableChild during JNI call. r=#geckoview-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: When user submits form data that has target=_blank, Fenix is no response forever.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1. Browse https://bbs.kafan.cn/forum.php?mobile=no

2. Tap searchbox in content
3. Tap enter key on software keyboard

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This issue is a dead lock when disposing Java object. This fix changes the timing of disposing it.
• String changes made/needed: no

Comment 17

[Pascal Chevrel:pascalc]

Comment on attachment 9200601 [details]
Bug 1684967 - Don't dispose GeckoEditableChild during JNI call. r=#geckoview-reviewers

Approved for 86 beta, thanks.

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/058ccaa8aaf0
https://hg.mozilla.org/releases/mozilla-beta/rev/2112d3252b22

Comment 19

[Andrei Bodea[:andrei]]

Verified as fixed on the 86.0.0.beta5 and latest Nightly 2/16 with Samsung Galaxy S10+ (Android 10).
Please, note that after tapping on the enter button from the keyboard the search webpage displayed the correct information. (I used https://bbs.kafan.cn/forum.php?mobile=no and https://www.excite.com/)