Mark methods of nsIPresShell and mozilla::PresShell as MOZ_CAN_RUN_SCRIPT (or at least MOZ_CAN_RUN_SCRIPT_BOUNDARY) if necessary

ASSIGNED
Assigned to

Status

()

task
P3
normal
ASSIGNED
a month ago
13 days ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

({leave-open})

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(17 attachments)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
Summary: Mark methods of nsIPresShell and mozilla::PresShell as MOZ_CAN_RUN_SCRIPT_BOUNDARY if necessary → Mark methods of nsIPresShell and mozilla::PresShell as MOZ_CAN_RUN_SCRIPT (or at least MOZ_CAN_RUN_SCRIPT_BOUNDARY) if necessary

I'll create some patches for example. Then, you're welcome to post patches instead of me. At that time, please let me know which method will you mark.

First of all, we should mark nsIPresShell::FlushPendingNotifications() as
MOZ_CAN_RUN_SCRIPT as soon as possible. Therefore, I'll mark all its callers
in PresShell as MOZ_CAN_RUN_SCRIPT first.

They are debug build only methods. So, callers of them shouldn't be marked
as MOZ_CAN_RUN_SCRIPT only for them. Therefore, MOZ_CAN_RUN_SCRIPT_BOUNDARY
must be better.

So, this patch makes all caller of it safe including its arguments unless
they come from other methods.

Comment 11

a month ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/dfce19e74038
part 1: Mark EventDispatchingCallback as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/f1a796212f61
part 2: Mark VerifyIncrementalReflow() and DoVerifyReflow() as MOZ_CAN_RUN_SCRIPT_BOUNDARY r=smaug
https://hg.mozilla.org/integration/autoland/rev/33a2dec4cfd3
part 3: Mark WillPaint() as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/10608c915cc5
part 4: Mark ResizeReflow() as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/4f65a6a58238
part 5: Mark ResizeReflowIgnoreOverride() as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/7bc3d65d627f
part 6: Mark ProcessReflowCommands() as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/32c1716986d2
part 7: Mark DidDoReflow() as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/a25a14f7150d
part 8: Mark HandlePostedReflowCallbacks() as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/a57a60c0278d
part 9: Mark nsIPresShell::FlushPendingNotifications() as MOZ_CAN_RUN_SCRIPT r=smaug
Type: defect → task
Priority: -- → P3

Next, we should mark PresShell::ScrollContentIntoView() as
MOZ_CAN_RUN_SCRIPT because it's used widely.

This patch marks its PresShell users, GoToAnchor() and ScrollToAnchor(),
as MOZ_CAN_RUN_SCRIPT. Additionally, this patch moves them from
nsIPresShell to PresShell because all callers refers PresShell directly.

This patch marks ScrollContentIntoView() as MOZ_CAN_RUN_SCRIPT and changing
some callers of them to guarantee thar their parent callers are also safe.

Additionally, this patch moves it from nsIPresShell to PresShell because
all callers can refer PresShell directly.

Unfortunately, this patch changes a lot of methods in autocomplete and satchel
since this patch needs to mark some interface methods as can_run_script and
they are called each other. This means that autocomplete module is really
sensitive like editor module. Perhaps, autocomplete and satchel should do
scroll asynchronously and unmark some of them as MOZ_CAN_RUN_SCRIPT again.

Now, we can mark DoScrollContentIntoView() as MOZ_CAN_RUN_SCRIPT and move
it from nsIPresShell to PresShell with a member.

Comment 16

a month ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/3d921a5274f5
part 10: Mark nsIPresShell::GoToAnchor() and nsIPresShell::ScrollToAnchor() as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/7d67598c9043
part 11: Mark nsIPresShell::ScrollContentIntoView() as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/7b69b606bb29
part 12: Mark nsIPresShell::DoScrollContentIntoView() as MOZ_CAN_RUN_SCRIPT r=smaug

Unfortunately, EventChainVisitor does not grab the nsPresContext with
RefPtr by itself. Therefore, there is no guarantee of the lifetime without
checking the origin when its subclasses are instantiated. This patch changes
it and subclasses to MOZ_STACK_CLASS since only EventDispatcher::Dispatch()
creates them in the stack with given nsPresContext. Additionally, it's
already been marked as MOZ_CAN_RUN_SCRIPT_BOUNDARY. Therefore, thensPresContextinstance has already been guaranteed its lifetime by the caller. For making this fact stronger, this patch marks their constructors asMOZ_CAN_RUN_SCRIPT. Therefore, nobody can create those instances without guaranteeing the lifetime ofnsPresContextanddom::Event. Note that it may look like thatmPresContextofEventChainPostVisitoris not guaranteed. However,EventChainPreVisitorwhich givesnsPresContextto it is also a stack only class. So, it won't be deleted beforeEventChainPostVisitor` instance.

Comment 22

17 days ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/59633bc3f761
part 13: Mark PresShell::Paint() as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/e2969c920810
part 14: Mark PresShell::WillPaintWindow() and PresShell::DidPaintWindow() as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/b4cbda10ffaf
part 15: Mark PresShell::ScrollFrameRectIntoView() as MOZ_CAN_RUN_SCRIPT r=smaug
https://hg.mozilla.org/integration/autoland/rev/801ff1a58ffc
part 16: Mark PresShell::HandleEventWithTarget() as MOZ_CAN_RUN_SCRIPT r=smaug

Comment 25

14 days ago
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/e754d9ad197d
part 17: Mark PresShell::HandleDOMEventWithTarget() as MOZ_CAN_RUN_SCRIPT r=smaug
You need to log in before you can comment on or make changes to this bug.