Closed Bug 1610657 Opened 2 years ago Closed 2 years ago

Assertion hit when both RDM and Notes open at the same time.

Categories

(Core :: Panning and Zooming, defect, P3)

defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: bradwerth, Unassigned)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

Steps to Reproduce:

  1. Install the Notes add-on from https://addons.mozilla.org/en-US/firefox/addon/notes-by-firefox.
  2. Navigate to any page and open devtools' Responsive Design Mode.
  3. Ensure that RDM Touch Simulation is enabled -- the button should be highlighted.
  4. Choose the menu option: View -> Sidebar -> Notes

Assertion failure: !mUsingAsyncZoomContainer || !haveRootContentOutsideAsyncZoomContainer
#01: void mozilla::layers::APZCTreeManager::UpdateHitTestingTreeImpl<mozilla::layers::LayerMetricsWrapper>(mozilla::layers::LayerMetricsWrapper const&, bool, mozilla::layers::WRRootId, unsigned int)

This action has no obvious adverse effects in Release Firefox.

Flags: needinfo?(botond)

The assertion is concerning but in the absence of visible adverse effects I'm not going to treat it with immediate priority.

I'm going to hang this off of the desktop zooming meta bug, since zooming is the scenario where the assertion failing is most likely to cause actual trouble.

Flags: needinfo?(botond)
Priority: -- → P3

I've run into this with STR that doesn't involve a sidebar or addon:

  1. In a debug build, navigate to any page and open RDM.
  2. Click on the device selector dropdown, and change the selection to any other device.
Attached file Layer tree

Here's the offending layer tree with this STR

So this is related to bug 1534459 after all. We don't have two nested async zoom containers because desktop zooming isn't enabled, but we do have two nested root content documents (an inner one for the content inside the RDM, and an outer one presumably for the document containing the RDM controls), and the inner one has an async zoom container, thereby tripping the assertion.

What we want here from APZ's point of view is to suppress the outer root content document (in the sense of marking it as FrameMetrics::mIsRootContent = false). Whatever mechanism we use to do that, we can also use to suppress the outer async zoom container with desktop zooming enabled, thereby fixing bug 1534459.

See Also: → 1534459

One thing that's still a mystery to me is why it takes me switching to a new device in the device selector dropdown (or opening a sidebar in the original STR) for the outer RCD to appear.

Setting devtools.responsive.browserUI.enabled=true appears to fix the STR from comment 2, but not the STR from comment 0.

It probably still makes sense to investigate the STR from comment 0 with devtools.responsive.browserUI.enabled=true in place, since that it likely to change aspects of the document structure that are relevant to how we compute mIsRootContent.

Depends on: 1549775
See Also: → 1644180

I can't repro this assertion failure using either set of STR (comment 0 and comment 2). Tried with both WR and non-WR on macOS. Can you still repro?

Flags: needinfo?(bwerth)

I can no longer reproduce it. It appears to be fixed.

Flags: needinfo?(bwerth)

Great, thanks! Feel free to reopen if it comes back.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.