1227139 - Webconsole is broken for addons and browser content toolbox - this.window.performance is undefined

Status: RESOLVED FIXED

(DevTools :: about:debugging, defect)

Description

[Alexandre Poirot [:ochameau]]

Because of the following exception:
getCachedMessages: TypeError: this.window.performance is undefined\nStack: WCA_onGetCachedMessages@resource://gre/modules/commonjs/toolkit/loader.js -> file:///mnt/desktop/gecko/devtools//server/actors/webconsole.js:741

Comment 1

[Alexandre Poirot [:ochameau]]

It seems to be also broken for the browser content toolbox.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: patch v1

Comment 3

[Brian Grinstead [:bgrins]]

Comment on attachment 8691396 [details] [diff] [review]
patch v1

Review of attachment 8691396 [details] [diff] [review]:
-----------------------------------------------------------------

Please update commit message to describe the fix - maybe 'Fix the webconsole for addon and browser content toolbox by handling a missing performance property on this.window'.

::: devtools/server/actors/webconsole.js
@@ +736,5 @@
>            if (!this.consoleAPIListener) {
>              break;
>            }
>  
> +          let requestStartTime = this.window && this.window.performance ?

So I checked into this for Browser Content Toolbox and it seems that this.window is coming from this._consoleScope on the ChildProcessActor which is a Sandbox.  I'm assuming that's the same setup when debugging addons.

Can you add a comment above this condition explaining that this.window can be a Sandbox in Addon or Browser Content Toolbox?

Comment 4

[Brian Grinstead [:bgrins]]

[Tracking Requested - why for this release]: This is a regression from Bug 1222617 and should go to aurora along with that bug if/when it gets uplifted

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: patch v2

Feel free to review the comment I added.

Comment 6

[Alexandre Poirot [:ochameau]]

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

Comment 7

[Alexandre Poirot [:ochameau]]

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

Comment 8

[Alexandre Poirot [:ochameau]]

https://hg.mozilla.org/integration/fx-team/rev/f516533b96a46764f288d336415e5a8aa3468e21
Bug 1227139 - Fix the webconsole for addon and browser content toolbox by handling a missing performance property on this.window. r=bgrins

Comment 9

[Brian Grinstead [:bgrins]]

Comment on attachment 8691425 [details] [diff] [review]
patch v2

Approval Request Comment
[Feature/regressing bug #]: 1222617
[User impact if declined]: Browser Content Toolbox / Addon Toolbox console won't retrieve cached messages (chrome debugging only)
[Describe test coverage new/current, TreeHerder]: No new coverage.. These special toolbox processes don't have any test coverage AFAIK
[Risks and why]: Limited risk. This is only changing behavior for these special chrome debugging cases, and it's a trivial change that adds an extra check on a property before accessing it.
[String/UUID change made/needed]:

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f516533b96a4

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked since it's a regression that occurred in FF44.

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8691425 [details] [diff] [review]
patch v2

This patch fixes a regression and has been on nightly for a few days, seems safe to uplift to Aurora44.

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5b1d1495f58e

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/5b1d1495f58e