Closed Bug 1672964 Opened 4 years ago Closed 4 years ago

[Fission] Enable tests: test_input_files_not_nsIFile.html, test_inputmode.html, test_nestediframe.html and test_viewport_resize.html

Categories

(Core :: DOM: Navigation, defect, P3)

Product:
Core
Component:
DOM: Navigation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
88 Branch
Project Flags:
Fission Milestone M7
Tracking Flags:
Tracking Status
firefox88 --- fixed

People

(Reporter: smaug, Assigned: peterv)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Bug 1672964 - Fix dom/html/test/test_iframe_sandbox_navigation2.html to not rely on BFCache, and reenable test_input_files_not_nsIFile.html, test_inputmode.html, test_nestediframe.html and test_viewport_resize.html with SHIP.
48 bytes, text/x-phabricator-request
Details | Review

I can't reproduce any failures with Fission.
It was disabled because there was one intermittent with SHIP at some point, but the test doesn't seem to do anything session history related.
However, the test has very rarely failed even without Fission.

So, hopefully it works also in CI.

Assignee: nobody → bugs
Status: NEW → ASSIGNED
Pushed by opettay@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/27cf2fe77ed0 [Fission] enable test_viewport_resize.html, r=annyG

uh, yet another intermittent which is impossible to reproduce locally.

Flags: needinfo?(bugs)
Attachment #9183431 - Attachment is obsolete: true
Severity: -- → S3
Fission Milestone: --- → M6c
Priority: -- → P2
Blocks: 1652554
No longer blocks: fission
Priority: P2 → P3

Getting re-enabled in Bug 1677483. We can close this bug once that patch lands and sticks.

Depends on: 1677483

The re-enablement in Bug 1677483 failed and this got re-skipped in Bug 1677799.

Adding all 4 tests in this bug as we expect the fix will be the same for all. Peter, Nika and Olli and I discussed these test failures briefly in a call today and these seem to be affected by some previous test(s) state. These don't fail locally and are intermittent failures on the tree.

The failure may be because of some focus failures in previous test(s) as we saw
Error: Unable to restore focus, expect failures and timeouts
in https://treeherder.mozilla.org/logviewer?job_id=319615464&repo=autoland&lineNumber=18390 . NI'ing Henri.

Or they could also be from sandboxing assertions also seen in one of the test failure logs. NI'ing Peter.

Assignee: bugs → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(peterv)
Flags: needinfo?(hsivonen)
Summary: [Fission] enable test_viewport_resize.html → [Fission] Enable tests: test_input_files_not_nsIFile.html, test_inputmode.html, test_nestediframe.html and test_viewport_resize.html

Try run with my in-progress patches and without the tests before test_inputmode.html in the same .ini:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1412ca63cf1903a13272e214ef74106e592358b5

Flags: needinfo?(hsivonen)

Moving fixing these tests to M7 as this seems to be a test-issue, these tests aren't failing on their own.

Fission Milestone: M6c → M7

(In reply to Henri Sivonen (:hsivonen) from comment #8)

Try run with my in-progress patches and without the tests before test_inputmode.html in the same .ini:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1412ca63cf1903a13272e214ef74106e592358b5

It indeed looks like the cause is in some test is the same .ini file before test_inputmode.html. Here's a try run with only tests before test_bug595429.html disabled:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6296a74d3f951f6a99c539df0eed7e451670d6f

(In reply to Henri Sivonen (:hsivonen) from comment #8)

Try run with my in-progress patches and without the tests before test_inputmode.html in the same .ini:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1412ca63cf1903a13272e214ef74106e592358b5

Oops. That, but for real this time:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a56eae0cac594d03f7658eed7b1e8ea34948483

Enabled tests from test_hidden until test_inputmode. This is not a proper bisect but a guess based on other annotations that hopefully will be more informative than an actual bisect:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=25d5ec02fd149b5394dcba709584c8a0760ae4b7

Depends on: 1681729
Assignee: nobody → hsivonen
Flags: needinfo?(peterv)
No longer blocks: 1652554

test_htmlcollection.html implicated.

(In reply to Henri Sivonen (:hsivonen) from comment #20)

test_htmlcollection.html followed immediately by test_inputmode.html:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7efe2dcbb282af9f875cd3847384238b73ef87fa

This combination doesn't fail, which suggests this is an interaction of three tests.

[test_htmlcollection.html]
[test_iframe_sandbox_general.html]
tags = openwindow
[test_inputmode.html]

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

OK, so with this cohort of tests remaining in the run, failure is intermittent, which is why trying to narrow the range doesn't work logically.

In the latest attempt of repeats, the failure rate was only 19%, which explains a lot about why things didn't make sense previously.

I think I have a local rr trace of a repro where two tabs for the test for bug 341604 are left behind as the next tests run.

Specifically, it looks like test_iframe_sandbox_popups.html sometimes fails to close its tabs.

(In reply to Henri Sivonen (:hsivonen) from comment #34)

Specifically, it looks like test_iframe_sandbox_popups.html sometimes fails to close its tabs.

No, that's not it.

It's either test_iframe_sandbox_navigation.html or test_iframe_sandbox_navigation2.html.

Tests 22 and 23 in test_iframe_sandbox_navigation2.html don't get their window.opened tabs closed with SHIP. I don't yet know what causes the Fission vs. non-Fission difference more precisely.

The problem isn't the close() call. The problem is that before the close() call, there's an attempt to call window.opener.postMessage but window.opener is null, because in BrowsingContext::GetOpener() the opener is discarded.

The opener becomes discarded with this stack:

::Document::DispatchContentLoadedEvents () at Document.cpp:7583::Document::UnblockOnload () at Document.cpp:11019
::Document::DoUnblockOnload () at Document.cpp:11089::nsLoadGroup::RemoveRequest () at nsLoadGroup.cpp:523
::nsLoadGroup::NotifyRemovalObservers () at nsLoadGroup.cpp:616{virtual override thunk({offset(-8)}, 
nsDocLoader::OnStopRequest)} () at nsDocLoader.cpp:1406nsDocLoader::OnStopRequest () at nsDocLoader.cpp:640
nsDocLoader::DocLoaderIsEmpty () at nsDocLoader.cpp:757nsDocLoader::doStopDocumentLoad () at nsDocLoader.cpp:938
nsDocLoader::DoFireOnStateChange () at nsDocLoader.cpp:1332{virtual override thunk({offset(-448)}, 
nsDocShell::OnStateChange)} () at nsDocShell.cpp:6603
nsDocShell::OnStateChange () at nsDocShell.cpp:5867
nsDocShell::EndPageLoad () at nsDocShell.cpp:6510
nsDocumentViewer::LoadComplete () at nsDocumentViewer.cpp:969
::PresShell::FlushPendingNotifications () at PresShell.h:1413
::PresShell::DoFlushPendingNotifications () at PresShell.cpp:4048
::PresShell::FlushPendingNotifications () at PresShell.h:1422
::PresShell::DoFlushPendingNotifications () at PresShell.cpp:4257
::PresShell::ProcessReflowCommands () at PresShell.cpp:9846
::PresShell::DidDoReflow () at PresShell.cpp:9450
::PresShell::HandlePostedReflowCallbacks () at PresShell.cpp:4016
::PresShell::FlushPendingNotifications () at PresShell.h:1413
::PresShell::DoFlushPendingNotifications () at PresShell.cpp:4048
::PresShell::FlushPendingNotifications () at PresShell.h:1422
::PresShell::DoFlushPendingNotifications () at PresShell.cpp:4257
::PresShell::ProcessReflowCommands () at PresShell.cpp:9878
::PresShell::UnsuppressAndInvalidate () at PresShell.cpp:3890
nsPresContext::EnsureVisible () at nsPresContext.cpp:1716
nsDocumentViewer::Show () at nsDocumentViewer.cpp:2073
nsDocumentViewer::Destroy () at nsDocumentViewer.cpp:1807
nsAutoScriptBlocker::~nsAutoScriptBlocker () at nsContentUtils.h:3478
nsContentUtils::RemoveScriptBlocker () at nsContentUtils.cpp:5581
::RunnableMethodImpl<>::Run () at nsThreadUtils.h:1201
mozilla::detail::RunnableMethodArguments<>::apply<> () at nsThreadUtils.h:1154
mozilla::detail::RunnableMethodArguments<>::applyImpl<> () at nsThreadUtils.h:1148
::Document::MaybeInitializeFinalizeFrameLoaders () at Document.cpp:8827
nsFrameLoaderDestroyRunnable::Run () at nsFrameLoader.cpp:1939
nsFrameLoader::DestroyDocShell () at nsFrameLoader.cpp:2008
::BrowsingContext::Detach () at BrowsingContext.cpp:810

With SHIP, mSHEntry is null here: https://searchfox.org/mozilla-central/rev/0379f315c75a2875d716b4f5e1a18bf27188f1e6/layout/base/nsDocumentViewer.cpp#1656

My best guess so far is that something in the steps in that if block makes it so that we don't end up marking the opener BrowsingContext discarded when we destroy the nsDocumentViewer for http://mochi.test:8888/tests/dom/html/test/file_iframe_sandbox_e_if9.html when we show the nsDocumentViewer for http://mochi.test:8888/tests/dom/html/test/file_iframe_sandbox_top_navigation_pass.html?Test%2022:%20Navigate%20_top%20with%20window.open():%20.

smaug, does something in those steps stand out to you as something that we should do in the SHIP case, or should I continue debugging to find it?

For reference:

  1. We open a window containing file_iframe_sandbox_e_if9.html. This is the window we need to close.
  2. Upon its onload that document navigates a sandboxed iframe to file_iframe_sandbox_e_if11.html.
  3. That file uses window.open to navigate _top to file_iframe_sandbox_top_navigation_pass.html?Test%2022:%20Navigate%20_top%20with%20window.open():%20. I.e. file_iframe_sandbox_e_if9.html navigates to file_iframe_sandbox_top_navigation_pass.html?Test%2022:%20Navigate%20_top%20with%20window.open():%20.
  4. file_iframe_sandbox_top_navigation_pass.html?Test%2022:%20Navigate%20_top%20with%20window.open():%20 runs into an attempt to invoke a JS method on null, because its opener BrowsingContext has become discarded as part of destroying the nsDocumentViewer for file_iframe_sandbox_e_if9.html. The JS error stops the script, so it never gets far enough to close its own window (opened in step 1).
Flags: needinfo?(bugs)

(Anyway, at this point, it's pretty clear that this isn't a focus bug.)

Assignee: hsivonen → peterv
Status: NEW → ASSIGNED
Flags: needinfo?(bugs)
Pushed by pvanderbeken@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/215709181145 Fix dom/html/test/test_iframe_sandbox_navigation2.html to not rely on BFCache, and reenable test_input_files_not_nsIFile.html, test_inputmode.html, test_nestediframe.html and test_viewport_resize.html with SHIP. r=smaug

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

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox88: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: