Open Bug 1412236 Opened 2 years ago Updated 2 years ago

Prevent additional registrations of framescripts for multiple client connections

Categories

(Testing :: Marionette, enhancement, P3)

58 Branch
enhancement

Tracking

(Not tracked)

People

(Reporter: whimboo, Unassigned)

References

(Depends on 1 open bug)

Details

When I start multiple client connections the following can be seen:

1509093102043	Marionette	DEBUG	Accepted connection 5 from 127.0.0.1:64938
1509093102047	Marionette	TRACE	5 -> [0,1,"newSession",{}]
1509093102055	Marionette	DEBUG	Register listener.js for window 2147483649
1509093102112	Marionette	TRACE	5 <- [1,1,null,{"sessionId":"57d03275-57bd-8843-9396-447bbe8d5125","capabilities":{}}]
1509093102117	Marionette	DEBUG	Accepted connection 6 from 127.0.0.1:64939
1509093102122	Marionette	DEBUG	Closed connection 6
1509093102123	Marionette	DEBUG	Accepted connection 7 from 127.0.0.1:64940
1509093102131	Marionette	TRACE	7 -> [0,1,"newSession",{}]
1509093102138	Marionette	DEBUG	Register listener.js for window 2147483649
1509093102155	Marionette	TRACE	7 <- [1,1,null,{"sessionId":"a057649a-b2ff-c646-a080-2ee4bf16cc87","capabilities":{}}]
1509093102165	Marionette	DEBUG	Accepted connection 8 from 127.0.0.1:64941
1509093102169	Marionette	DEBUG	Closed connection 8
1509093102172	Marionette	DEBUG	Accepted connection 9 from 127.0.0.1:64942
1509093102177	Marionette	TRACE	9 -> [0,1,"newSession",{}]
1509093102186	Marionette	DEBUG	Register listener.js for window 2147483649
1509093102202	Marionette	TRACE	9 <- [1,1,null,{"sessionId":"82857084-234e-8c4b-8d99-2dd6aa3f8034","capabilities":{}}]

As you can see we register new framescripts for each open window/tab, whenever a new client gets connected. Right now I don't know about the side-effects of that, but given my feeling it could interfere with currently executed commands in the framescript, eg. any kind of navigation.

Andreas, I think we should really turn off the feature to allow multiple client connections. I don't see any benefit at all. Anything you have in mind?
Flags: needinfo?(ato)
(In reply to Henrik Skupin (:whimboo) from comment #0)

> As you can see we register new framescripts for each open
> window/tab, whenever a new client gets connected. Right now I
> don't know about the side-effects of that, but given my feeling
> it could interfere with currently executed commands in the
> framescript, eg. any kind of navigation.

I think it is a valid concern that not every part of Marionette is
written so that multiple connections are safe.  But is it a real
problem that consumers are making more than one connection today?

> I think we should really turn off the feature to allow multiple
> client connections. I don't see any benefit at all. Anything you
> have in mind?

In addition to the emulate-user-interactions part of Marionette
that can cause problems, disallowing multiple connections for the
remainder of browser instrumentation features doesn’t seem ideal.

We have to bear in mind that the Marionette protocol will likely be
replaced by CDP at some point.  CDP allows multiple connections,
so we need to decide whether to disallow multiple connections from
initialising the WebDriver service or if we want to allow it at the
user’s own risk.

I would argue that the right thing to do here is to allow multiple
connections, but to prevent multiple connections from initialising
the WebDriver service.  We need to address https://bugzil.la/1330309
before that can become a reality.
Flags: needinfo?(ato)
(In reply to Andreas Tolfsen ‹:ato› from comment #1)
> > As you can see we register new framescripts for each open
> > window/tab, whenever a new client gets connected. Right now I
> > don't know about the side-effects of that, but given my feeling
> > it could interfere with currently executed commands in the
> > framescript, eg. any kind of navigation.
> 
> I think it is a valid concern that not every part of Marionette is
> written so that multiple connections are safe.  But is it a real
> problem that consumers are making more than one connection today?

I cannot say for sure, because I haven't tested it. But just the multiple creation of the framescripts made me curious. It may be a red herring through.

> We have to bear in mind that the Marionette protocol will likely be
> replaced by CDP at some point.  CDP allows multiple connections,
> so we need to decide whether to disallow multiple connections from
> initialising the WebDriver service or if we want to allow it at the
> user’s own risk.

That would make sense, eg. for retrieving data in parallel while another commands are running.

> I would argue that the right thing to do here is to allow multiple
> connections, but to prevent multiple connections from initialising
> the WebDriver service.  We need to address https://bugzil.la/1330309
> before that can become a reality.

Sure, I just marked as dependency and updated the bug's summary.
Depends on: 1330309
Summary: Should we stop allowing multiple client connections? → Prevent additional registrations of framescripts for multiple client connections
[Mass Change 2018-01-15] Moving bugs to backlog
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.