1397452 - Remove all SDK compatibility code from devtools

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Alexandre Poirot [:ochameau]]

Whenever we feel there is no more value in supporting SDK addons,
we should remove this code that use to support SDK features:
http://searchfox.org/mozilla-central/rev/4d8e389498a08668cce9ebf6232cc96be178c3e4/devtools/client/framework/devtools.js#21-24
http://searchfox.org/mozilla-central/rev/4d8e389498a08668cce9ebf6232cc96be178c3e4/devtools/client/framework/devtools.js#550-585
http://searchfox.org/mozilla-central/source/devtools/shim/DevToolsShim.jsm#278-292
http://searchfox.org/mozilla-central/source/devtools/shim/DevToolsShim.jsm#147-193

But also look into this:
http://searchfox.org/mozilla-central/source/devtools/server/actors/tab.js#530-550

Comment 1

[Alexandre Poirot [:ochameau]]

This isn't directly related to SDK, but to bootstrapped add-on which are going to be rejected as SDK add-ons.
We could also remove such code:
http://searchfox.org/mozilla-central/source/devtools/shared/protocol.js#1129-1170
I'm wondering if we have some other code like that, kept just for add-ons (not only sdk ones)??

Comment 2

[Alexandre Poirot [:ochameau]]

About comment 1 and protocol.js compatiblity code, there is more context in bug 1295171.

Honza, What is the status of RDP monitor?
Is it a WebExtension? If not, does it still work? Does it still uses ActorClass and ActorClassWithSpec?

Otherwise you may have some input on dropping SDK addon supports mentioned in comment 0?
Should we wait FF58? Or just wait for complete SDK removal first?

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Alexandre Poirot [:ochameau] from comment #2)
> About comment 1 and protocol.js compatiblity code, there is more context in
> bug 1295171.
> 
> Honza, What is the status of RDP monitor?
> Is it a WebExtension? If not, does it still work? Does it still uses
It still implemented as old-school Add-on SDK based extension, so
not compatible with the latest Firefox

> ActorClass and ActorClassWithSpec?
Yes, it's using actor and dynamic actor registration API.

> Otherwise you may have some input on dropping SDK addon supports mentioned
> in comment 0?
> Should we wait FF58? Or just wait for complete SDK removal first?
The extension should be transformed into WebExtension and in the
current state it should not block the NO-SDK effort.

Also the importance of the RDPi decreases as we consider supporting CDP.

Honza

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1397452 - Remove all SDK compatiblity code from DevTools.

Review commit: https://reviewboard.mozilla.org/r/183236/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183236/

Comment 5

[Alexandre Poirot [:ochameau]]

Comment on attachment 8911809 [details]
Bug 1397452 - Remove all SDK compatiblity code from DevTools.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183236/diff/1-2/

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8911809 [details]
Bug 1397452 - Remove all SDK compatiblity code from DevTools.

https://reviewboard.mozilla.org/r/183236/#review188398

Thanks for cleaning up my code :)

::: devtools/shim/DevToolsShim.jsm:36
(Diff revision 2)
>   * existing mozilla-central code that still needs to interact with DevTools (such as
>   * web-extensions).
>   *
>   * DevToolsShim is a singleton that provides a set of helpers to interact with DevTools,
>   * that work whether the DevTools addon is installed or not. It can be used to start
> - * listening to events, register tools, themes. As soon as a DevTools addon is installed
> + * listening to events. As soon as a DevTools addon is installed the DevToolsShim willi

s/willi/will

::: devtools/shim/DevToolsShim.jsm:110
(Diff revision 2)
>     * - registerTheme
>     * - unregisterTheme

can be removed too

Comment 7

[Alexandre Poirot [:ochameau]]

Comment on attachment 8911809 [details]
Bug 1397452 - Remove all SDK compatiblity code from DevTools.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183236/diff/2-3/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0dfa545d2f02
Remove all SDK compatiblity code from DevTools. r=jdescottes

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0dfa545d2f02