1349509 - DevTools addon should reload from source/addon on addon install

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Alexandre Poirot [:ochameau]]

For now, the addon only loads addon resources when you press the Ctrl+Alt+R.
But in a world where Firefox ships without devtools, the addon should ensure setting up devtools on install.
It also sounds correct today, to update the devtools as soon as the addon is installed.

Comment 1

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9da9a5d6886c

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1349509 - Reload devtools when installing the addon.

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

Comment 3

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=30d7222db5a30df3d159ebd06f01fe97ee077361

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8850033 [details]
Bug 1349509 - Reload devtools when installing the addon.

https://reviewboard.mozilla.org/r/122780/#review125152

Seems reasonable to me!

On install via about:debugging, I got several errors:

```
1490226463904	addons.manager	WARN	AddonListener threw exception when calling onInstalled: TypeError: defer is not a function (resource://devtools/shared/deprecated-sync-thenables.js:38:22) JS Stack trace: [e-mail]:38:22 < [e-mail]:71:5 < [e-mail]:93:5 < [e-mail]:1693:11 < [e-mail]:3160:5 < this.XPIProvider.installAddonFromLocation<@XPIProvider.jsm:4141:5 < [e-mail]:319:42 < [e-mail]:922:23 < [e-mail]:806:7 < scheduleWalkerLoop/<@Promise-backend.js:742:11

"notify event 'closed' threw an exception: TypeError: isPromise is not a function
Stack: resolve@resource://devtools/shared/deprecated-sync-thenables.js:71:18
l@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:68:7
eventSource/proto.emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:130:9
onClosed@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1098:5
close@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:668:11
close@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:664:9
close@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:664:9
cleanup@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:371:7
detachClients@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:395:9
close@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:404:5
destroy@chrome://devtools/content/aboutdebugging/initializer.js:56:5
@chrome://devtools/content/aboutdebugging/initializer.js:66:3
Line: 71, column: 18"  ThreadSafeDevToolsUtils.js:80

TypeError: rejected is not a function
Stack trace:
reject@resource://devtools/shared/deprecated-sync-thenables.js:80:24
listenerJson@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:731:9
emitOnObject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:112:9
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:89:38
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1322:29
reject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1145:7
purgeRequests/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1166:47
purgeRequests@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1166:5
onClosed@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1100:5
close@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:668:11
close@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:664:9
close@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:664:9
cleanup@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:371:7
detachClients@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:395:9
close@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:404:5
destroy@chrome://devtools/content/aboutdebugging/initializer.js:56:5
@chrome://devtools/content/aboutdebugging/initializer.js:66:3
  core.js:106
```

I guess it's to be expected though, since about:debugging is using files you are replacing!

Comment 5

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=acf45b25c377569f53fdebdbcf40641f3de9846c

Comment 6

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d68707b56b8682c2ed1b80575f3c799a2910240d

Comment 7

[Alexandre Poirot [:ochameau]]

Comment on attachment 8850033 [details]
Bug 1349509 - Reload devtools when installing the addon.

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8b525d91101b
Reload devtools when installing the addon. r=jryans

Comment 9

[Alexandre Poirot [:ochameau]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) (on PTO until Mar. 28) from comment #4)
> Comment on attachment 8850033 [details]
> Bug 1349509 - Reload devtools when installing the addon.
> 
> I guess it's to be expected though, since about:debugging is using files you
> are replacing!

This is related to the addon worflow/reload step.
Something doesn't get cleaned up correctly and ends up throwing.
This may be related to Cu.unload and not waiting enough before starting devtools again.

I would like to look seriously into that once we are on github.
In the meantime I consider that we still have to restart firefox to check for a modification against devtools safely.

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/8b525d91101b