Closed Bug 1506007 Opened 3 years ago Closed 3 years ago

Don't create checkpoints while painting with AutoAssertNoContentJS on the stack

Categories

(Core Graveyard :: Web Replay, defect)

defect
Not set
normal

Tracking

(firefox65 fixed)

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: bhackett1024, Assigned: bhackett1024)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
Currently, Web Replay checkpoints are created after each paint as part of the NotifyPaintStart call.  This is problematic because creating a checkpoint can cause content JS to run --- for example, the debugger could trigger a synchronous paint, which runs content JS via nsRefreshDriver::RunFrameRequestCallbacks --- and content JS may be prohibited by the calling PresShell::Paint frame.

The attached patch resolves this by moving the CreateCheckpoint call so that it happens in PresShell::Paint when content JS is allowed again.
Attachment #9023846 - Flags: review?(nical.bugzilla)
Comment on attachment 9023846 [details] [diff] [review]
patch

Review of attachment 9023846 [details] [diff] [review]:
-----------------------------------------------------------------

I never touched the innards of PresShell.cpp so I would rather transfer the review to someone from the layout team.
Attachment #9023846 - Flags: review?(nical.bugzilla) → review?(mstange)
Comment on attachment 9023846 [details] [diff] [review]
patch

Review of attachment 9023846 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, moving this call from NotifyPaintStart (which runs during ShadowLayerForwarder::EndTransaction) to PresShell::Paint seems fine to me. It would be nicer to move it even more up the call stack, but I can't see a good place for that, so I think this is good the way it is.
Attachment #9023846 - Flags: review?(mstange) → review+
Pushed by bhackett@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc6ffce8b9b6
Don't create checkpoints while painting with AutoAssertNoContentJS on the stack, r=mstange.
https://hg.mozilla.org/mozilla-central/rev/bc6ffce8b9b6
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.