Closed Bug 1646493 Opened 5 months ago Closed 2 months ago

Fix GetInProcessScriptableTop usage in pointer lock code for OOP iframes

Categories

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

defect

Tracking

()

RESOLVED FIXED
82 Branch
Fission Milestone M6b
Tracking Status
firefox82 --- fixed

People

(Reporter: kmag, Assigned: edgar)

References

(Blocks 1 open bug)

Details

(Whiteboard: patch in review)

Attachments

(2 files, 1 obsolete file)

It's possible we have some other code that makes things work when OOP frames are involved, but I'm not sure.

(This also includes nsContentUtils::IsInPointerLockContext I suppose)

Severity: -- → N/A

Tracking "Figure out GetInProcessTop usage" bugs for Fission M6b.

Fission Milestone: --- → M6b

Olli, can you do this audit and point it to someone else in DOM to fix?

Flags: needinfo?(bugs)

I will take a look at this.

Flags: needinfo?(bugs) → needinfo?(echen)

There are two GetInProcessScriptableTop usages for pointer lock, both of them doesn't work when OOP frames are involved,

  1. https://searchfox.org/mozilla-central/rev/73a14f1b367948faa571ed2fe5d7eb29460787c1/dom/base/Document.cpp#14303 is to check if toplevel document is active, this now doesn't work as expected on fission.

  2. https://searchfox.org/mozilla-central/rev/73a14f1b367948faa571ed2fe5d7eb29460787c1/dom/base/nsContentUtils.cpp#6620-6621 is to check if pointer lock is requested under certain toplevel document, EventStateManager::sPointerLockedDoc won't point to the right thing if the lock is requested by an OOP iframe.

Flags: needinfo?(echen)
Assignee: nobody → echen

by replacing the usage of GetInProcessScriptableTop with {Browsing/Window}Context

Keywords: leave-open
Pushed by echen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0ff509769a96
Make GetPointerLockError Fission compatible; r=smaug
See Also: → 1662587
Status: NEW → ASSIGNED
Type: task → defect
Priority: -- → P2
Summary: Figure out if GetInProcessScriptableTop usage in pointer lock code is OK → Fix GetInProcessScriptableTop usage in pointer lock code for OOP iframes
Whiteboard: patch landed but waiting for bug 1662587 to also close

Unfortunately, this change makes content process and chrome process doesn't go
through the same code path because nsFocusManager handles their state
separately on purpose.

After working on bug 1662587 and understand focus handling a bit more, I think we don't need to wait for bug 1662587, could fix this bug separately.

Attached file Bug 1646493 - Tests; (obsolete) —

Comment on attachment 9175877 [details]
Bug 1646493 - Tests;

Revision D90313 was moved to bug 1662587. Setting attachment 9175877 [details] to obsolete.

Attachment #9175877 - Attachment is obsolete: true
Whiteboard: patch landed but waiting for bug 1662587 to also close → patch in review
Keywords: leave-open
Pushed by echen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/90e073a9c89d
Make pointer auto-unlocking for active top-level document changes fission compatible; r=hsivonen
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
You need to log in before you can comment on or make changes to this bug.