1893119 - Separate nsIDragSession objects from nsIDragService

Status: RESOLVED FIXED

(Core :: Widget, task, P1)

Description

[David Parks [:handyman]]

Separating the objects actually simplifies the implementations in a post-e10s, post-Fission world. nsiDragSessions will be tied to the widget that they target and most of the methods that are currently defined for the nsIDragService will be moved to specific instances of nsiDragSession, which will make them make more sense. Finally, clients will be able to get the drag session from an nsIWidget or a window.

This is also needed for bug 1886604.

Comment 1

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 16 - Make nsBaseDragService weakly remember PBrowsers instead of PContents r=nika!,#win-reviewers!,edgar

nsBaseDragService keeps track of the processes that might need to cancel a drag with EndDragSession and this is technically correct since the drag session is a singleton in the child process, but this series of patches changes those sessions to be per-widget. This allows content processes to distinguish which of its browsers is engaged in a drag.

Depends on D203166

Comment 2

[David Parks [:handyman]]

Attachment: Bug 1893119: Add GetDragSession to nsIWidget r=gstoll,rkraesig,edgar

This pre-sages the coming changes that remove GetCurrentSession() from nsIDragService and make nsIDragSessions widget-specific (instead of being a process-wide singleton).

Depends on D211062

Comment 3

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 6 - Make JS clients of nsIDragService::GetCurrentSession use a (implicit) window r=gstoll,rkraesig,edgar!

When later patches in the series update nsIWidget::GetDragSession, these
callers will use a drag session that is specific to the widget they supply
here.

This also adds GetDragSession to DOMWindowUtils since window.dragSession is
ChromeOnly and plain mochitests need to be able to interrogate it.

Depends on D211063

Comment 4

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 2 - Make C++ callers of nsIDragService::GetCurrentSession pass a widget r=gstoll,rkraesig

Callers must call nsIWidget::GetDragSession for the correct widget under the drag.

Depends on D211064

Comment 5

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 4 - DataTransfer operations that require a drag must use a widget r=edgar!

DataTransfer methods that use a drag session should get the one for the "correct" widget. This is especially important in content processes, especially especially since the DataTransfer can be used long after a related drag session completes. We use the widget associated with the DataTransfer's owner.

Depends on D211065

Comment 6

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 3 - Add widget to nsContentUtils::GetDragSession r=gstoll,rkraesig,edgar

Updates each client of the nsContentUtils method to get the right drag session -- the one for the widget that is currently the target of the drag session.
The change to nsDOMWindowUtils::DispatchDOMEventViaPresShellForTesting() supports the change to WidgetDragEvent::InitDropEffectForTests() and enabled a
large number of test fixes in the next patch.

Depends on D211066

Comment 7

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 5 - Fix coordinates use in dispatchDOMEventViaPresShellForTesting r=masayuki!,mconley!,edgar!,#places-reviewers!

Since we now add the widget to the event in
dispatchDOMEventViaPresShellForTesting, WidgetGUIEvents that are sent in
mochitests via that method need to transform their screen coordinates by
nsIWidget::WidgetToScreenOffset, to mirror the transformation by
BrowserParent::TransformParentToChild that they don't get because they
skip the parent process.

This exposes a bunch of things that were done to work around this bug. They
are cleaned up here.

Depends on D211067

Comment 8

[David Parks [:handyman]]

Attachment: Bug 1893119: Use BrowsingContext to identify widget in IPDL DND message handlers r=gstoll,rkraesig

In order to get the drag session for these IPDL messages, we need the widget. We
send the BrowsingContext for this.

Depends on D211068

Comment 9

[David Parks [:handyman]]

Attachment: WIP: Bug 1893119: Remove nsIDragService::GetCurrentSession

The implementation of nsIWidget::GetDragSession() still uses a temporary singleton -- that is replaced at the end of the patch series.

Depends on D211069

Comment 10

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 7 - Access sourceNode on nsIDragSession instead of nsIDragService r=edgar!

A small fix to existing code that used the service where it should have
used the session.

Depends on D211070

Comment 11

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 8 - Prepare for nsBaseDragSession to be broken out of nsBaseDragService r=gstoll,rkraesig

Creates the nsBaseDragSession class and moves nsIDragSession's methods and data
into it. This patch also does this for each platform-specific nsDragService
and the nsDragServiceProxy. The result is that the drag service and drag
session are still one object in both the main and child processes, and the
inheritance diagram for each platform implementation goes from its present
from:

nsDragService -> nsBaseDragService -> nsIDragService + nsIDragSession

to:

nsDragService -> nsDragSession -> nsBaseDragService -> nsIDragService + nsBaseDragSession -> nsIDragSession

(and similar for the nsDragServiceProxy).

At the end of this patch series, the objects will be separated and the
inheritance diagram will be as expected:

nsDragService -> nsBaseDragService -> nsIDragService

nsDragSession -> nsBaseDragSession -> nsIDragSession

There are no substantive changes here -- it's essentially all cut
and paste to prepare to smoothly sever the unwanted inheritance.

Depends on D211071

Comment 12

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 9 - Move additional parts of nsIDragService to nsIDragSession r=gstoll,rkraesig

Moving additional methods and fields. No substantive changes.

Depends on D211072

Comment 13

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 10 - Move MaybeEditorDeletedSourceNode from nsIDragService to nsIDragSession r=gstoll,rkraesig

MaybeEditorDeletedSourceNode was defined on nsIDragService but is more appropriately defined on nsIDragSession. This was not an issue previously since those were the same object.

Depends on D211073

Comment 14

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 11 - Add widget to nsIDragService::InvokeDragSession and friends r=gstoll,rkraesig

This also moves InvokeDragSession from an IDL method to a private one.

Depends on D211074

Comment 15

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 12 - Move DragMoved from nsIDragService to nsIDragSession r=gstoll,rkraesig

Depends on D211075

Comment 16

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 13 - Move FireDragEventAtSource from nsIDragService to nsIDragSession r=gstoll,rkraesig

Depends on D211076

Comment 17

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 14 - Move SetDragEndPoint from nsBaseDragService to nsIDragSession r=gstoll,rkraesig

Depends on D211077

Comment 18

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 15 - Give StartDragSessionForTests a window to start the drag session on r=gstoll,rkraesig

Depends on D211078

Comment 19

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 17 - Pass the widget to StartDragSession r=gstoll,rkraesig

Depends on D211079

Comment 20

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 18 - Copy/Move MaybeAddBrowser, RemoveAllBrowsers and MustUpdateDataTransfer from nsIDragService to nsIDragSession r=gstoll,rkraesig

MustUpdateDataTransfer is a simple function that makes more sense on nsIDragSession.
MaybeAddBrowser records browsing context IDs for processes with existing drag sessions. There may be such processes before the parent process even has a drag session, so they cannot be tied to drag sessions. We tie them to the widget, until the widget creates a drag session in the process, at which point we move them to the drag session. We also rename them for clarity, since they are no longer on a drag-specific object.

Depends on D211080

Comment 21

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 19 - Make WindowWatcher call EndDragSession on session object r=gstoll,rkraesig,gijs!

Enumerates all top-level widgets, closing any existing drag sessions.
See bug 1544167 for the Windows bug that this API fixes.

Depends on D211081

Comment 22

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 20 - Move EndDragSession from nsIDragService to nsIDragSession r=gstoll,rkraesig

This also adds the widget that hosts the drag session as a parameter
to EndDragSession. When the service/session separation happens in
a later patch, this will be used to remove the session from the widget.
Finally, it also creates endDragSessionForWindow, so that JS can
still call EndDragSession.

Note that EndDragSession now needs to be called on for both
the source and target widgets of the drag, if the drag originated
in Gecko. We have always done this -- it just previously resulted
in double EndDragSession calls (which are harmless).

Depends on D211082

Comment 23

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 21 - Separate nsIDragService and nsIDragSession implementations r=gstoll,rkraesig,edgar

Split the class inheritance trees for nsIDragService and nsIDragSession.
Remember that, before the start of this patch series, the inheritance
diagram was:

nsDragService -> nsBaseDragService -> nsIDragService + nsIDragSession

and the only instance was a singleton. We switched it to:

nsDragService -> nsDragSession -> nsBaseDragService -> nsIDragService + nsBaseDragSession -> nsIDragSession

and maintained the singleton. This allowed us to allow us to move things to
the new classes without breaking behavior. We are done with that,
so we can now change the inheritance to its final form:

nsDragService -> nsBaseDragService -> nsIDragService

nsDragSession -> nsBaseDragSession -> nsIDragSession

Of course, we also need to properly construct and release the
nsIDragSessions (formerly part of the singleton), so that is done here as
well. This is all fairly straightforward, except in the case of gtk, where
we need to change some callback behavior.

Depends on D211083

Comment 24

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 22 - Make nsIDragService::StartDragSession return the nsIDragSession r=rkraesig!

Depends on D211084

Comment 25

[David Parks [:handyman]]

Attachment: Bug 1893119: Part 1 - Add widget/window parameter to nsIDragService::GetCurrentSession r=gstoll,rkraesig

We will need this to get the correct drag session in content when there are
multiple choices, and as a sanity check elsewhere.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21e7f9ac67f8
Part 1 - Add widget/window parameter to nsIDragService::GetCurrentSession r=gstoll
https://hg.mozilla.org/integration/autoland/rev/1fa3e699ce84
Part 2 - Make C++ callers of nsIDragService::GetCurrentSession pass a widget r=gstoll,win-reviewers,geckoview-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/4ea26902d0de
Part 3 - Add widget to nsContentUtils::GetDragSession r=gstoll,rkraesig
https://hg.mozilla.org/integration/autoland/rev/e9bf191cd706
Part 4 - DataTransfer operations that require a drag must use a widget r=edgar
https://hg.mozilla.org/integration/autoland/rev/ac0387ad6e61
Part 5 - Fix coordinates use in dispatchDOMEventViaPresShellForTesting r=masayuki,mconley,places-reviewers,tabbrowser-reviewers,dao
https://hg.mozilla.org/integration/autoland/rev/a0ed1d9f8f47
Part 6 - Make JS clients of nsIDragService::GetCurrentSession use a (implicit) window r=webdriver-reviewers,places-reviewers,gstoll,edgar,whimboo,mak
https://hg.mozilla.org/integration/autoland/rev/7389cd2118f7
Part 7 - Access sourceNode on nsIDragSession instead of nsIDragService r=edgar
https://hg.mozilla.org/integration/autoland/rev/f241ae3cb8e4
Part 8 - Prepare for nsBaseDragSession to be broken out of nsBaseDragService r=gstoll,rkraesig,win-reviewers,geckoview-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/5f877d88c524
Part 9 - Move additional parts of nsIDragService to nsIDragSession r=gstoll,rkraesig,geckoview-reviewers,win-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/d8cfdbbb4485
Part 10 - Move MaybeEditorDeletedSourceNode from nsIDragService to nsIDragSession r=gstoll,masayuki
https://hg.mozilla.org/integration/autoland/rev/ebf3051cb224
Part 11 - Add widget to nsIDragService::InvokeDragSession and friends r=gstoll,rkraesig
https://hg.mozilla.org/integration/autoland/rev/62fb6cf1058e
Part 12 - Move DragMoved from nsIDragService to nsIDragSession r=gstoll,rkraesig,win-reviewers,spohl
https://hg.mozilla.org/integration/autoland/rev/d37f779fefc1
Part 13 - Move FireDragEventAtSource from nsIDragService to nsIDragSession r=gstoll,rkraesig,win-reviewers,geckoview-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/fb6296e6363d
Part 14 - Move SetDragEndPoint from nsBaseDragService to nsIDragSession r=gstoll,rkraesig,win-reviewers,geckoview-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/d3ed47f8ec53
Part 15 - Give StartDragSessionForTests a window to start the drag session on r=gstoll,rkraesig
https://hg.mozilla.org/integration/autoland/rev/4040c18ce889
Part 16 - Make nsBaseDragService weakly remember PBrowsers instead of PContents r=nika,win-reviewers,rkraesig
https://hg.mozilla.org/integration/autoland/rev/7c0e7e5ad413
Part 17 - Pass the widget to StartDragSession r=gstoll,rkraesig,win-reviewers,geckoview-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/33088c246d95
Part 18 - Copy/Move MaybeAddBrowser, RemoveAllBrowsers and MustUpdateDataTransfer from nsIDragService to nsIDragSession r=geckoview-reviewers,gstoll,m_kato
https://hg.mozilla.org/integration/autoland/rev/6c3ffc8c534c
Part 19 - Make WindowWatcher call EndDragSession on session object r=gstoll,Gijs,win-reviewers
https://hg.mozilla.org/integration/autoland/rev/beeda5ef3712
Part 20 - Move EndDragSession from nsIDragService to nsIDragSession r=gstoll,rkraesig,win-reviewers,geckoview-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/384b025f56d3
Part 21 - Separate nsIDragService and nsIDragSession implementations r=gstoll,geckoview-reviewers,rkraesig,win-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/df85fe3d7cb6
Part 22 - Make nsIDragService::StartDragSession return the nsIDragSession r=rkraesig,win-reviewers,gstoll,geckoview-reviewers,m_kato
https://hg.mozilla.org/integration/autoland/rev/01bfc9faa690
apply code formatting via Lando

Comment 27

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/21e7f9ac67f8
https://hg.mozilla.org/mozilla-central/rev/1fa3e699ce84
https://hg.mozilla.org/mozilla-central/rev/4ea26902d0de
https://hg.mozilla.org/mozilla-central/rev/e9bf191cd706
https://hg.mozilla.org/mozilla-central/rev/ac0387ad6e61
https://hg.mozilla.org/mozilla-central/rev/a0ed1d9f8f47
https://hg.mozilla.org/mozilla-central/rev/7389cd2118f7
https://hg.mozilla.org/mozilla-central/rev/f241ae3cb8e4
https://hg.mozilla.org/mozilla-central/rev/5f877d88c524
https://hg.mozilla.org/mozilla-central/rev/d8cfdbbb4485
https://hg.mozilla.org/mozilla-central/rev/ebf3051cb224
https://hg.mozilla.org/mozilla-central/rev/62fb6cf1058e
https://hg.mozilla.org/mozilla-central/rev/d37f779fefc1
https://hg.mozilla.org/mozilla-central/rev/fb6296e6363d
https://hg.mozilla.org/mozilla-central/rev/d3ed47f8ec53
https://hg.mozilla.org/mozilla-central/rev/4040c18ce889
https://hg.mozilla.org/mozilla-central/rev/7c0e7e5ad413
https://hg.mozilla.org/mozilla-central/rev/33088c246d95
https://hg.mozilla.org/mozilla-central/rev/6c3ffc8c534c
https://hg.mozilla.org/mozilla-central/rev/beeda5ef3712
https://hg.mozilla.org/mozilla-central/rev/384b025f56d3
https://hg.mozilla.org/mozilla-central/rev/df85fe3d7cb6
https://hg.mozilla.org/mozilla-central/rev/01bfc9faa690

Comment 28

[Andrej (:aglavic)]

(In reply to Iulian Moraru from comment #27)

https://hg.mozilla.org/mozilla-central/rev/21e7f9ac67f8
https://hg.mozilla.org/mozilla-central/rev/1fa3e699ce84
https://hg.mozilla.org/mozilla-central/rev/4ea26902d0de
https://hg.mozilla.org/mozilla-central/rev/e9bf191cd706
https://hg.mozilla.org/mozilla-central/rev/ac0387ad6e61
https://hg.mozilla.org/mozilla-central/rev/a0ed1d9f8f47
https://hg.mozilla.org/mozilla-central/rev/7389cd2118f7
https://hg.mozilla.org/mozilla-central/rev/f241ae3cb8e4
https://hg.mozilla.org/mozilla-central/rev/5f877d88c524
https://hg.mozilla.org/mozilla-central/rev/d8cfdbbb4485
https://hg.mozilla.org/mozilla-central/rev/ebf3051cb224
https://hg.mozilla.org/mozilla-central/rev/62fb6cf1058e
https://hg.mozilla.org/mozilla-central/rev/d37f779fefc1
https://hg.mozilla.org/mozilla-central/rev/fb6296e6363d
https://hg.mozilla.org/mozilla-central/rev/d3ed47f8ec53
https://hg.mozilla.org/mozilla-central/rev/4040c18ce889
https://hg.mozilla.org/mozilla-central/rev/7c0e7e5ad413
https://hg.mozilla.org/mozilla-central/rev/33088c246d95
https://hg.mozilla.org/mozilla-central/rev/6c3ffc8c534c
https://hg.mozilla.org/mozilla-central/rev/beeda5ef3712
https://hg.mozilla.org/mozilla-central/rev/384b025f56d3
https://hg.mozilla.org/mozilla-central/rev/df85fe3d7cb6
https://hg.mozilla.org/mozilla-central/rev/01bfc9faa690

Perfherder has detected a talos performance change from push ee40fe578b2cc3f10b18847f5e9c82b145cd73b0.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
8%	perf_reftest_singletons svg-text-getExtentOfChar-1.html	windows11-64-qr	e10s fission stylo webrender	3,655.43 -> 3,373.73
7%	perf_reftest_singletons getElementById-1.html	windows11-64-qr	e10s fission stylo webrender	23.77 -> 22.02
5%	perf_reftest_singletons window-named-property-get.html	windows11-64-qr	e10s fission stylo webrender	492.22 -> 466.90

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 1067

For more information on performance sheriffing please see our FAQ.

Comment 29

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 1 - Add widget/window parameter to nsIDragService::GetCurrentSession

We will need this to get the correct drag session in content when there are
multiple choices, and as a sanity check elsewhere.

Original Revision: https://phabricator.services.mozilla.com/D215033

Comment 30

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 2 - Make C++ callers of nsIDragService::GetCurrentSession pass a widget

Content process callers must call nsIDragService::GetCurrentSession with the
correct widget under the drag. For all callers, it is a sanity check.

Original Revision: https://phabricator.services.mozilla.com/D211065

Comment 31

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 3 - Add widget to nsContentUtils::GetDragSession

Updates each client of the nsContentUtils method to get the right drag session -- the one for the widget that is currently the source or target of the drag session.
The change to nsDOMWindowUtils::DispatchDOMEventViaPresShellForTesting() supports the change to WidgetDragEvent::InitDropEffectForTests() and enabled a
large number of test fixes in the next patch.

Original Revision: https://phabricator.services.mozilla.com/D211067

Comment 32

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 4 - DataTransfer operations that require a drag must use a widget

DataTransfer methods that use a drag session should get the one for the "correct" widget. This is especially important in content processes, especially especially since the DataTransfer can be used long after a related drag session completes. We use the widget associated with the DataTransfer's owner.

Original Revision: https://phabricator.services.mozilla.com/D211066

Comment 33

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 5 - Fix coordinates use in dispatchDOMEventViaPresShellForTesting

Since we now add the widget to the event in
dispatchDOMEventViaPresShellForTesting, WidgetGUIEvents that are sent in
mochitests via that method need to transform their screen coordinates by
nsIWidget::WidgetToScreenOffset, to mirror the transformation by
BrowserParent::TransformParentToChild that they don't get because they
skip the parent process.

This exposes a bunch of things that were done to work around this bug. They
are cleaned up here.

Original Revision: https://phabricator.services.mozilla.com/D211068

Comment 34

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 6 - Make JS clients of nsIDragService::GetCurrentSession use a (implicit) window

GetCurrentSession now needs to know the widget to return the drag
session in content processes. It now uses an explicitly provided
window or the entry global JS context (that must be a window).

This also adds GetDragSession to DOMWindowUtils, and fixes some small
bugs in EventUtils.

Original Revision: https://phabricator.services.mozilla.com/D211064

Comment 35

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 7 - Access sourceNode on nsIDragSession instead of nsIDragService

A small fix to existing code that used the service where it should have
used the session.

Original Revision: https://phabricator.services.mozilla.com/D211071

Comment 36

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 8 - Prepare for nsBaseDragSession to be broken out of nsBaseDragService

Creates the nsBaseDragSession class and moves nsIDragSession's methods and data
into it. This patch also does this for each platform-specific nsDragService
and the nsDragServiceProxy. The result is that the drag service and drag
session are still one object in both the main and child processes, and the
inheritance diagram for each platform implementation goes from its present
from:

nsDragService -> nsBaseDragService -> nsIDragService + nsIDragSession

to:

nsDragService -> nsDragSession -> nsBaseDragService -> nsIDragService + nsBaseDragSession -> nsIDragSession

(and similar for the nsDragServiceProxy).

At the end of this patch series, the objects will be separated and the
inheritance diagram will be as expected:

nsDragService -> nsBaseDragService -> nsIDragService

nsDragSession -> nsBaseDragSession -> nsIDragSession

There are no substantive changes here -- it's essentially all cut
and paste to prepare to smoothly sever the unwanted inheritance.

Original Revision: https://phabricator.services.mozilla.com/D211072

Comment 37

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 9 - Move additional parts of nsIDragService to nsIDragSession

Moving additional methods and fields. No substantive changes.

Original Revision: https://phabricator.services.mozilla.com/D211073

Comment 38

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 10 - Move MaybeEditorDeletedSourceNode from nsIDragService to nsIDragSession

MaybeEditorDeletedSourceNode was defined on nsIDragService but is more appropriately defined on nsIDragSession. This was not an issue previously since those were the same object.

Original Revision: https://phabricator.services.mozilla.com/D211074

Comment 39

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 11 - Add widget to nsIDragService::InvokeDragSession and friends

This also moves InvokeDragSession from an IDL to a private one.

Original Revision: https://phabricator.services.mozilla.com/D211075

Comment 40

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 12 - Move DragMoved from nsIDragService to nsIDragSession

Original Revision: https://phabricator.services.mozilla.com/D211076

Comment 41

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 13 - Move FireDragEventAtSource from nsIDragService to nsIDragSession

Original Revision: https://phabricator.services.mozilla.com/D211077

Comment 42

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 14 - Move SetDragEndPoint from nsBaseDragService to nsIDragSession

Original Revision: https://phabricator.services.mozilla.com/D211078

Comment 43

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 15 - Give StartDragSessionForTests a window to start the drag session on

Original Revision: https://phabricator.services.mozilla.com/D211079

Comment 44

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 16 - Make nsBaseDragService weakly remember PBrowsers instead of PContents

nsBaseDragService keeps track of the processes that might need to cancel a drag with EndDragSession and this is technically correct since the drag session is a singleton in the child process, but this series of patches changes those sessions to be per-PuppetWidget/BrowserChild. This allows content processes to distinguish which of its browsers is engaged in a drag.

Original Revision: https://phabricator.services.mozilla.com/D211062

Conflicts:

dom/ipc/ContentParent.h

Comment 45

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 17 - Pass the widget to StartDragSession

Original Revision: https://phabricator.services.mozilla.com/D211080

Comment 46

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 18 - Copy/Move MaybeAddBrowser, RemoveAllBrowsers and MustUpdateDataTransfer from nsIDragService to nsIDragSession

MustUpdateDataTransfer is a simple function that makes more sense on
nsIDragSession.

MaybeAddBrowser records BrowserParents for processes with existing drag
sessions. There may be such processes before the parent process
even has a drag session, so they cannot be tied to drag sessions. We tie
them to the drag service, until the service creates a drag session in
the process, at which point we move them to the drag session. Since
the session and service are still the same object and each now has methods
with these names, we leave part of session's implementation commented out
so that we run correctly instead of looping infinitely. The code is
exposed when the objects separate later in the patch series.

Finally, the implementation of TakeSessionBrowserListFromService currently
amounts to a nop, but it will have substace when the service and session
are separated.

Original Revision: https://phabricator.services.mozilla.com/D211081

Comment 47

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 19 - Make WindowWatcher call EndDragSession on session object

CreateChromeWindow creates windows in the parent process. EndDragSession is
moving to the drag session, so call it on the paren't session, if one exists.
Note that this patch will not build without the next one.

Original Revision: https://phabricator.services.mozilla.com/D211082

Comment 48

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 20 - Move EndDragSession from nsIDragService to nsIDragSession

This is a straightforward move. It does take the liberty of breaking out an
EndDragSessionImpl method, which will be needed later.

Original Revision: https://phabricator.services.mozilla.com/D211083

Comment 49

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 21 - Separate nsIDragService and nsIDragSession implementations

Split the class inheritance trees for nsIDragService and nsIDragSession.
Remember that, before the start of this patch series, the inheritance
diagram was:

nsDragService -> nsBaseDragService -> nsIDragService + nsIDragSession

and the only instance was a singleton. We switched it to:

nsDragService -> nsDragSession -> nsBaseDragService -> nsIDragService + nsBaseDragSession -> nsIDragSession

and maintained the singleton. This allowed us to allow us to move things to
the new classes without breaking behavior. We are done with that,
so we can now change the inheritance to its final form:

nsDragService -> nsBaseDragService -> nsIDragService

nsDragSession -> nsBaseDragSession -> nsIDragSession

Of course, we also need to properly construct and release the
nsIDragSessions (formerly part of the singleton), so that is done here as
well. That happens in nsBaseDrag[Service|Session] for parent process drags
and in nsDrag[Service|Session]Proxy for content. This is all fairly
straightforward, except in the case of gtk, where we need to change
some callback behavior.

Original Revision: https://phabricator.services.mozilla.com/D211084

Comment 50

[Greg Stoll :gstoll]

Attachment: Bug 1893119: Part 22 - Make nsIDragService::StartDragSession return the nsIDragSession

Original Revision: https://phabricator.services.mozilla.com/D212298

Comment 51

[Greg Stoll :gstoll]

Attachment: Bug 1893119: apply code formatting via Lando

ignore-this-changeset

Comment 52

[Greg Stoll :gstoll]

esr128 Uplift Approval Request

• User impact if declined: this set of patches is needed to cover drag and drop in DLP
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: medium
• Explanation of risk level: drag and drop changes, but they've already made it to release
• String changes made/needed: no
• Is Android affected?: no

Comment 53

[BugBot [:suhaib / :marco/ :calixte]]

A patch has been attached on this bug, which was already closed. Filing a separate bug will ensure better tracking. If this was not by mistake and further action is needed, please alert the appropriate party. (Or: if the patch doesn't change behavior -- e.g. landing a test case, or fixing a typo -- then feel free to disregard this message)

Comment 54

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/954312aea049
https://hg.mozilla.org/releases/mozilla-esr128/rev/f5d56d81d2ac
https://hg.mozilla.org/releases/mozilla-esr128/rev/1f75f3146542
https://hg.mozilla.org/releases/mozilla-esr128/rev/94978507d843
https://hg.mozilla.org/releases/mozilla-esr128/rev/55d6419322b2
https://hg.mozilla.org/releases/mozilla-esr128/rev/1831bda84f46
https://hg.mozilla.org/releases/mozilla-esr128/rev/75e3b4851f6b
https://hg.mozilla.org/releases/mozilla-esr128/rev/3258de22c9bd
https://hg.mozilla.org/releases/mozilla-esr128/rev/2c167a762871
https://hg.mozilla.org/releases/mozilla-esr128/rev/ccf869e73000
https://hg.mozilla.org/releases/mozilla-esr128/rev/b461b84653d2
https://hg.mozilla.org/releases/mozilla-esr128/rev/495ca2baba0d
https://hg.mozilla.org/releases/mozilla-esr128/rev/36cdfd7c2aab
https://hg.mozilla.org/releases/mozilla-esr128/rev/a8096307656a
https://hg.mozilla.org/releases/mozilla-esr128/rev/d8018c548ed6
https://hg.mozilla.org/releases/mozilla-esr128/rev/2cd1c5abfbc7
https://hg.mozilla.org/releases/mozilla-esr128/rev/11285edf00ff
https://hg.mozilla.org/releases/mozilla-esr128/rev/54b307ef9105
https://hg.mozilla.org/releases/mozilla-esr128/rev/aff320656025
https://hg.mozilla.org/releases/mozilla-esr128/rev/844225266de8
https://hg.mozilla.org/releases/mozilla-esr128/rev/c5fbbd12c8d6
https://hg.mozilla.org/releases/mozilla-esr128/rev/d97a954b58c0
https://hg.mozilla.org/releases/mozilla-esr128/rev/ba49ddd82c7a