Bug 172966 - Remove legacy INSPECTOR_SERVER implementation
Summary: Remove legacy INSPECTOR_SERVER implementation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-06 01:55 PDT by Carlos Garcia Campos
Modified: 2022-03-08 13:07 PST (History)
14 users (show)

See Also:


Attachments
Patch (72.61 KB, patch)
2017-06-06 01:58 PDT, Carlos Garcia Campos
zan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2017-06-06 01:55:34 PDT
No upstream port is currently using it nowadays, so it's dead code. WPE still uses it downstream, they didn't upstream it because they plan to switch to the new Remote Inspector.
Comment 1 Carlos Garcia Campos 2017-06-06 01:58:01 PDT
Created attachment 312064 [details]
Patch
Comment 2 Don Olmstead 2017-06-06 09:59:02 PDT
(In reply to Carlos Garcia Campos from comment #0)
> No upstream port is currently using it nowadays, so it's dead code. WPE
> still uses it downstream, they didn't upstream it because they plan to
> switch to the new Remote Inspector.

We've been using it in our PlayStation port and I'm not entirely sure what removing this functionality would mean to our end users as we haven't yet investigated this. Web Inspector isn't something I've spent any time developing so I'm a bit in the dark here. I would assume we'd have a similar situation to WPE where connecting to the device directly would be a good thing.

Can you explain a bit more about what this change means for providing a remote inspector for our end users?
Comment 3 Carlos Garcia Campos 2017-06-07 03:40:56 PDT
There's a new remote inspector implementation, already used by apple and GTK+ ports. The old implementation, the one based on web sockets, is not currently used by any upstream port. If any downstream port wants to keep using web sockets for the remote inspector, they will have to maintain it downstream, because right now it's dead code in upstream. And we don't want to maintain dead code. You can simply revert this patch in your downstream repo.
Comment 4 Carlos Garcia Campos 2017-06-07 03:43:05 PDT
WPE has already switched to the new remote inspector, without a frontend implementation, but it's possible to use any webkitgtk based web view to debug WPE.
Comment 5 Don Olmstead 2017-06-07 12:11:29 PDT
(In reply to Carlos Garcia Campos from comment #3)
> There's a new remote inspector implementation, already used by apple and
> GTK+ ports. The old implementation, the one based on web sockets, is not
> currently used by any upstream port. If any downstream port wants to keep
> using web sockets for the remote inspector, they will have to maintain it
> downstream, because right now it's dead code in upstream. And we don't want
> to maintain dead code. You can simply revert this patch in your downstream
> repo.

I'm all for removing dead code. I just wanted to know what it meant for the end users interacting with it. WPE would appear to be in a similar boat to us as you have end users interacting with a device remotely.

I just wanted that information so we can discuss internally what that would mean for us going forward. If the change is going to be a huge headache for our end users then we would probably discuss what to do and whether that would mean enabling it within WinCairo and continuing development there.
Comment 6 Carlos Garcia Campos 2017-06-07 23:46:33 PDT
Committed r217924: <http://trac.webkit.org/changeset/217924>
Comment 7 Olivier Blin 2017-06-08 11:08:06 PDT
We agree with Don, this will be problematic for embedded devices developers, since this requires much more from the client browser running the inspector frontend.
We are WPE users, and we know that in the set-top box market, quite some web app developers are today connecting to the remote web inspector from a Windows OS.

Which Windows browser will be suitable to talk to this new remote inspector?
Should be have a WinCairo build with dbus support?
Comment 8 Michael Catanzaro 2017-06-08 17:51:36 PDT
(In reply to Olivier Blin from comment #7)
> We are WPE users, and we know that in the set-top box market, quite some web
> app developers are today connecting to the remote web inspector from a
> Windows OS.

I really think compatibility with non-WebKit browsers should be a non-goal. That's really out there. If it works at all today with the WebSocket server, that's just due to luck.

I'm not going to object if you want to add a D-Bus remote inspector implementation to WinCairo, though my understanding is that D-Bus is a second-class citizen on Windows.
Comment 9 Carlos Garcia Campos 2017-06-08 22:16:09 PDT
(In reply to Olivier Blin from comment #7)
> We agree with Don, this will be problematic for embedded devices developers,
> since this requires much more from the client browser running the inspector
> frontend.
> We are WPE users, and we know that in the set-top box market, quite some web
> app developers are today connecting to the remote web inspector from a
> Windows OS.
> 
> Which Windows browser will be suitable to talk to this new remote inspector?
> Should be have a WinCairo build with dbus support?

You are not forced to switch to the new remote inspector. The legacy one had no implementation upstream, which means that if you are using it is because you are adding it in your downstream fork. You can simply include the changes removed here in your implementation in your fork and keep using it.
Comment 10 Olivier Blin 2017-06-09 01:22:54 PDT
(In reply to Michael Catanzaro from comment #8)
> (In reply to Olivier Blin from comment #7)
> > We are WPE users, and we know that in the set-top box market, quite some web
> > app developers are today connecting to the remote web inspector from a
> > Windows OS.
> 
> I really think compatibility with non-WebKit browsers should be a non-goal.
> That's really out there. If it works at all today with the WebSocket server,
> that's just due to luck.

Well, it would make web developers life a lot easier if it just worked from Chrome.

Going from WebSockets to requiring builtin D-Bus support in the client browser make the situation difficult, even from WebKit browsers on Windows.

> I'm not going to object if you want to add a D-Bus remote inspector
> implementation to WinCairo, though my understanding is that D-Bus is a
> second-class citizen on Windows.

Yes, we might get tricky platform support issues with D-Bus on Windows.
Was there any specific reason to pick D-Bus over WebSockets?
Comment 11 Olivier Blin 2017-06-09 01:28:51 PDT
(In reply to Carlos Garcia Campos from comment #9)
>
> You are not forced to switch to the new remote inspector. The legacy one had
> no implementation upstream, which means that if you are using it is because
> you are adding it in your downstream fork.

Not so long ago, there was WebInspectorServerGtk upstream.
And there still is WebInspectorServerWPE in the WebPlatformForEmbedded repo, which is the default "remote" inspector implementation in WPE and enabled by default (see WebKit2/config.h).
Comment 12 Yusuke Suzuki 2017-06-09 03:10:19 PDT
BTW, what is the best approach in Windows in terms of platform IPC mechanism? COM?
Comment 13 Don Olmstead 2017-06-09 10:10:03 PDT
(In reply to Olivier Blin from comment #10)
> (In reply to Michael Catanzaro from comment #8)
> > (In reply to Olivier Blin from comment #7)
> > > We are WPE users, and we know that in the set-top box market, quite some web
> > > app developers are today connecting to the remote web inspector from a
> > > Windows OS.
> > 
> > I really think compatibility with non-WebKit browsers should be a non-goal.
> > That's really out there. If it works at all today with the WebSocket server,
> > that's just due to luck.
> 
> Well, it would make web developers life a lot easier if it just worked from
> Chrome.

From https://blogs.igalia.com/carlosgc/2017/05/03/webkitgtk-remote-debugging-in-2-18/ it sounds like you couldn't even use Safari to do a remote debugging session. Is this correct? Our users are primarily developing on Macs.
Comment 14 Konstantin Tokarev 2017-06-09 12:47:02 PDT
Out of curiosity, how do you debug on device with dbus inspector? Are you using something like Telepathy to connect to the other machine?
Comment 15 Joseph Pecoraro 2017-06-09 14:03:29 PDT
> > I really think compatibility with non-WebKit browsers should be a non-goal.
> > That's really out there. If it works at all today with the WebSocket server,
> > that's just due to luck.
> 
> Well, it would make web developers life a lot easier if it just worked from
> Chrome.

To add to this: The WebKit Web Inspector frontend frequently adopts many features soon after they are added to trunk WebKit / JavaScriptCore. Some of these features may not be implemented / supported by other browsers, including Chrome. We make no effort to ensure the frontend is cross browser compatibility given we are writing it only for WebKit.

One example off the top of my head is we use `-webkit-canvas()` for popovers. I don't think any other browser implements this. In fact I think Chrome/Blink removed it years ago.
Comment 16 Michael Catanzaro 2017-06-09 17:12:24 PDT
I know our web inspector required ES6 features at least a year before they were enabled in Chrome. I think you must have needed a significant amount of patches to keep web inspector working during this time, or maybe were just using a very old version of WebKit?
Comment 17 Carlos Garcia Campos 2017-06-10 01:43:28 PDT
(In reply to Don Olmstead from comment #13)
> (In reply to Olivier Blin from comment #10)
> > (In reply to Michael Catanzaro from comment #8)
> > > (In reply to Olivier Blin from comment #7)
> > > > We are WPE users, and we know that in the set-top box market, quite some web
> > > > app developers are today connecting to the remote web inspector from a
> > > > Windows OS.
> > > 
> > > I really think compatibility with non-WebKit browsers should be a non-goal.
> > > That's really out there. If it works at all today with the WebSocket server,
> > > that's just due to luck.
> > 
> > Well, it would make web developers life a lot easier if it just worked from
> > Chrome.
> 
> From
> https://blogs.igalia.com/carlosgc/2017/05/03/webkitgtk-remote-debugging-in-2-
> 18/ it sounds like you couldn't even use Safari to do a remote debugging
> session. Is this correct? Our users are primarily developing on Macs.

Correct.
Comment 18 Carlos Garcia Campos 2017-06-10 01:44:00 PDT
(In reply to Konstantin Tokarev from comment #14)
> Out of curiosity, how do you debug on device with dbus inspector? Are you
> using something like Telepathy to connect to the other machine?

DBus over TCP.
Comment 19 Carlos Garcia Campos 2017-06-10 01:46:12 PDT
I'm going to say it again. You are not forced to switch to the new remote inspector in downstream. We have removed the web sockets implementation from upstream because it was dead code. If you are currently using it, is because you have an implementation on top of it downstream. You can now include the code removed upstream in your downstream fork and continue using the web sockets.
Comment 20 Konstantin Tokarev 2017-06-10 03:17:19 PDT
Unfortunately, there is no well-defined way how to avoid duplication of work between downstreams when code is not in trunk. In this case WPE, PlayStation and Qt will have to do the same maintenance work 3 times.
Comment 21 Konstantin Tokarev 2017-06-10 03:17:59 PDT
Oh, and EFL if they will be upgrading their WebKit
Comment 22 Michael Catanzaro 2017-06-10 07:52:09 PDT
WPE is upstream and has decided to not use INSPECTOR_SERVER.
Comment 23 Konstantin Tokarev 2017-06-10 09:27:03 PDT
Ok
Comment 24 Don Olmstead 2017-06-11 16:24:02 PDT
(In reply to Carlos Garcia Campos from comment #17)
> (In reply to Don Olmstead from comment #13)
> > (In reply to Olivier Blin from comment #10)
> > > (In reply to Michael Catanzaro from comment #8)
> > > > (In reply to Olivier Blin from comment #7)
> > > > > We are WPE users, and we know that in the set-top box market, quite some web
> > > > > app developers are today connecting to the remote web inspector from a
> > > > > Windows OS.
> > > > 
> > > > I really think compatibility with non-WebKit browsers should be a non-goal.
> > > > That's really out there. If it works at all today with the WebSocket server,
> > > > that's just due to luck.
> > > 
> > > Well, it would make web developers life a lot easier if it just worked from
> > > Chrome.
> > 
> > From
> > https://blogs.igalia.com/carlosgc/2017/05/03/webkitgtk-remote-debugging-in-2-
> > 18/ it sounds like you couldn't even use Safari to do a remote debugging
> > session. Is this correct? Our users are primarily developing on Macs.
> 
> Correct.

Ok well that's a pretty big deal breaker for us.

Is there any way to have a cross-platform remote inspector? If not I think we're going to ask to revert this and then enable the inspector server within wincairo.
Comment 25 Carlos Garcia Campos 2017-06-11 22:47:35 PDT
(In reply to Don Olmstead from comment #24)
> (In reply to Carlos Garcia Campos from comment #17)
> > (In reply to Don Olmstead from comment #13)
> > > (In reply to Olivier Blin from comment #10)
> > > > (In reply to Michael Catanzaro from comment #8)
> > > > > (In reply to Olivier Blin from comment #7)
> > > > > > We are WPE users, and we know that in the set-top box market, quite some web
> > > > > > app developers are today connecting to the remote web inspector from a
> > > > > > Windows OS.
> > > > > 
> > > > > I really think compatibility with non-WebKit browsers should be a non-goal.
> > > > > That's really out there. If it works at all today with the WebSocket server,
> > > > > that's just due to luck.
> > > > 
> > > > Well, it would make web developers life a lot easier if it just worked from
> > > > Chrome.
> > > 
> > > From
> > > https://blogs.igalia.com/carlosgc/2017/05/03/webkitgtk-remote-debugging-in-2-
> > > 18/ it sounds like you couldn't even use Safari to do a remote debugging
> > > session. Is this correct? Our users are primarily developing on Macs.
> > 
> > Correct.
> 
> Ok well that's a pretty big deal breaker for us.

What browser do you currently use for remote debugging? I'm still surprised you could use a non-webkit browser, because it has been impossible for me lately due to the JavaScript features used by WebKit.

> Is there any way to have a cross-platform remote inspector?

You can use a linux virtual machine, I guess. In the case of Mac OS, I think you can build WebKitGTK+ with the quartz backend.

> If not I think
> we're going to ask to revert this and then enable the inspector server
> within wincairo.

Isn't WinCairo a WebKit1 port? The inspector server was in WebKit2, or do you mean only the changes in the inspector itself? I still don't understand why you can add you inspector server implementation to your downstream fork and not the inspector server itself too. You are still assuming you have to switch to the new remote inspector, but again, you don't have to.
Comment 26 Zan Dobersek 2017-06-12 00:18:16 PDT
(In reply to Joseph Pecoraro from comment #15)
> > > I really think compatibility with non-WebKit browsers should be a non-goal.
> > > That's really out there. If it works at all today with the WebSocket server,
> > > that's just due to luck.
> > 
> > Well, it would make web developers life a lot easier if it just worked from
> > Chrome.
> 
> To add to this: The WebKit Web Inspector frontend frequently adopts many
> features soon after they are added to trunk WebKit / JavaScriptCore. Some of
> these features may not be implemented / supported by other browsers,
> including Chrome. We make no effort to ensure the frontend is cross browser
> compatibility given we are writing it only for WebKit.
> 
> One example off the top of my head is we use `-webkit-canvas()` for
> popovers. I don't think any other browser implements this. In fact I think
> Chrome/Blink removed it years ago.

(In reply to Michael Catanzaro from comment #16)
> I know our web inspector required ES6 features at least a year before they
> were enabled in Chrome. I think you must have needed a significant amount of
> patches to keep web inspector working during this time, or maybe were just
> using a very old version of WebKit?

The WebSocket-based remote inspector did not work properly when not used from WPE, WebKitGTK+ or recent Safari (Technology Preview or a development build).
Comment 27 Zan Dobersek 2017-06-12 00:32:15 PDT
(In reply to Michael Catanzaro from comment #22)
> WPE is upstream and has decided to not use INSPECTOR_SERVER.

That's the situation upstream. I really wish it would scale downstream, but it seems unlikely.

Given that the WinCairo port is interested in this feature to be available upstream, we can revert the cross-port code that was removed in the landed patch. At that point upstreaming the WPE bits of INSPECTOR_SERVER can be  discussed again, even if REMOTE_INSPECTOR will stay the preferred feature.
Comment 28 Loïc Yhuel 2017-06-12 00:50:32 PDT
(In reply to Zan Dobersek from comment #26)
> The WebSocket-based remote inspector did not work properly when not used
> from WPE, WebKitGTK+ or recent Safari (Technology Preview or a development
> build).
It worked from WinApple too (We can download binaries from the build bot, which allows us to give something to the web developers on Windows easily).
Comment 29 Olivier Blin 2017-06-12 01:10:51 PDT
(In reply to Zan Dobersek from comment #26)
> The WebSocket-based remote inspector did not work properly when not used
> from WPE, WebKitGTK+ or recent Safari (Technology Preview or a development
> build).

The WinApple builds worked fine as well to load the WebSocket-based remote inspector.
Don also mentioned Otter Browser, based on the QtWebKit revival.

Has this been discussed with the main WPE users of the "downstream" port?
They probably have similar needs, to be able to inspect their applications from Win or Apple systems.

Thanks for your feedback.
Comment 30 Don Olmstead 2017-06-12 11:54:36 PDT
(In reply to Carlos Garcia Campos from comment #25)
> (In reply to Don Olmstead from comment #24)
> > (In reply to Carlos Garcia Campos from comment #17)
> > > (In reply to Don Olmstead from comment #13)
> > > > (In reply to Olivier Blin from comment #10)
> > > > > (In reply to Michael Catanzaro from comment #8)
> > > > > > (In reply to Olivier Blin from comment #7)
> > > > > > > We are WPE users, and we know that in the set-top box market, quite some web
> > > > > > > app developers are today connecting to the remote web inspector from a
> > > > > > > Windows OS.
> > > > > > 
> > > > > > I really think compatibility with non-WebKit browsers should be a non-goal.
> > > > > > That's really out there. If it works at all today with the WebSocket server,
> > > > > > that's just due to luck.
> > > > > 
> > > > > Well, it would make web developers life a lot easier if it just worked from
> > > > > Chrome.
> > > > 
> > > > From
> > > > https://blogs.igalia.com/carlosgc/2017/05/03/webkitgtk-remote-debugging-in-2-
> > > > 18/ it sounds like you couldn't even use Safari to do a remote debugging
> > > > session. Is this correct? Our users are primarily developing on Macs.
> > > 
> > > Correct.
> > 
> > Ok well that's a pretty big deal breaker for us.
> 
> What browser do you currently use for remote debugging? I'm still surprised
> you could use a non-webkit browser, because it has been impossible for me
> lately due to the JavaScript features used by WebKit.

We're still working through trunk to get that enabled but our current working version uses inspector server and is usable by devs through Safari. The team working on the browser is primarily on Windows so for that we either build out WinCairo or install Otter.

We agree that compatibility with Chrome would be nice but we have no expectation of that. The -webkit-canvas bites you there and if I'm remembering correctly there were some DOM4 Node methods that were not enabled on Chrome but they could be polyfilled successfully. In terms of ES6 if you really want you can use babel to turn it to ES5.

Its doable but a real headache that I think we all can agree is ideal to avoid.

> > Is there any way to have a cross-platform remote inspector?
> 
> You can use a linux virtual machine, I guess. In the case of Mac OS, I think
> you can build WebKitGTK+ with the quartz backend.

Having everyone use a VM or distributing WebKitGTK+ builds is a bit of a nightmare with the number of devs using our platforms. Not all of them are in house either so this becomes more problematic.

> > If not I think
> > we're going to ask to revert this and then enable the inspector server
> > within wincairo.
> 
> Isn't WinCairo a WebKit1 port? The inspector server was in WebKit2, or do
> you mean only the changes in the inspector itself?

At the moment its WebKit but we're currently looking at turning it into a WebKit 2 port. We have people on our team investigating this currently.

> I still don't understand
> why you can add you inspector server implementation to your downstream fork
> and not the inspector server itself too. You are still assuming you have to
> switch to the new remote inspector, but again, you don't have to.

We've pushed some patches for remote inspector such as https://trac.webkit.org/changeset/215357/webkit which are for inspector server and had planned to push more before seeing this change.

With this whole conversation I'm trying to get an idea on what our next steps are and really appreciated the notification from Michael about this bug. Having less platform specific code is a good thing and we should definitely go forward with that. But like some of the WPE users that have chimed in this seems to bork workflows for developers using WebKit.

Our current requirements are being able to connect to the inspector from a WebKit based browser. This would be WinCairo/Otter on Windows, Safari on Mac, and WebKitGTK+ on Linux. This seems to jive with WPE users as well.

Is there a way to have a Remote Inspector implementation to rule them all? If so we're happy to assign developer resources to make that a reality. We're also interested in helping Igalia with an open Web Driver implementation that can be used across ports.

I personally don't have familiarity with the internals of the Remote Inspector so I'm defaulting to the experts over at Igalia and Apple on what the best path forward is.
Comment 31 Joseph Pecoraro 2022-03-08 13:05:52 PST
Comment on attachment 312064 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312064&action=review

Nice! Thanks for catching the WebInspectorUI bits!

> Source/WebInspectorUI/ChangeLog:8
> +        Remove InspectorFrontendHostStub and thr web sockets initialization.

Typo: "thr" => "the"
Comment 32 Joseph Pecoraro 2022-03-08 13:07:42 PST
Ugh another situation of me commenting on a very old thread that I got an email about. Ignore me :/