1296589 - TypeError: this._locationStore is null when navigating from about:newtab with DevTools open

Status: RESOLVED FIXED

(DevTools :: General, defect, P2)

Description

[Jarda Snajdr [:jsnajdr]]

Steps to reproduce:
1. Open about:newtab
2. Open DevTools on the about:newtab page
3. Navigate to another page in the same tab (e.g., by clicking on one of the tiles offered by about:newtab)

Result:
TypeError: this._locationStore is null: SourceMapService.prototype.reset@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/source-map-service.js:44:3
SourceMapService.prototype.destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/source-map-service.js:49:3
Toolbox.prototype.destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/toolbox.js:2060:7
emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:191:13
TabTarget.prototype.destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:567:5
TabTarget.prototype._setupRemoteListeners/this._onTabDetached@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:481:9
eventSource/aProto.emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:132:9
DebuggerClient.prototype.onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1007:7
LocalDebuggerTransport.prototype.send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:570:13
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14
exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14

Comment 1

[Brian Grinstead [:bgrins]]

Tentatively blocking on the source maps in console bug since I'm assuming that's when this started

Comment 2

[Jaideep Bhoosreddy [:jbhoosreddy]]

Apparently, I'm unable to assign myself to this bug, but I can take this.

Comment 3

[Jaideep Bhoosreddy [:jbhoosreddy]]

Attachment: 1296589.patch

Added guards to reset and unsubscribe methods for the source map service to prevent access to the store, if it is not initialized yet.

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8783066 [details] [diff] [review]
1296589.patch

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

Looks good -- I wonder if we should just clear the map upon destroy rather than having these checks and nullifying the maps.

Comment 5

[Jarda Snajdr [:jsnajdr]]

There might be a better way to fix this. The error stack trace suggests that the sourceMapService.destroy() method is called on already destroyed (or unitialized) instance. Jaideep's patch adds guards in this method, effectively hiding the error instead of really fixing it. Jaideep, maybe you could investigate what is going on and why the incorrect calls happen? This would lead to a fix that takes more effort, but ultimately leads to cleaner and better code.

Comment 6

[Jaideep Bhoosreddy [:jbhoosreddy]]

(In reply to Jarda Snajdr [:jsnajdr] from comment #5)
> There might be a better way to fix this. The error stack trace suggests that
> the sourceMapService.destroy() method is called on already destroyed (or
> unitialized) instance. Jaideep's patch adds guards in this method,
> effectively hiding the error instead of really fixing it. Jaideep, maybe you
> could investigate what is going on and why the incorrect calls happen? This
> would lead to a fix that takes more effort, but ultimately leads to cleaner
> and better code.

Jordan and I discussed about this before, and this is one of the things I have to investigate, in one of my follow up bugs. I believe I already have a follow up bug filed for the issue of handling toolbox events, where I was planning on looking into this more deeply, as I go through my notes when I go back. Personally, I would like land this patch so that this error does not add noise for others trying out the feature. But if a little delay is okay, I can resolve this in this bug itself. I am planning to tackle the root cause of the issue though.

(In reply to Jordan Santell [:jsantell] [@jsantell] (Away from Bugzilla for awhile) from comment #4)
> Comment on attachment 8783066 [details] [diff] [review]
> 1296589.patch
> 
> Review of attachment 8783066 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good -- I wonder if we should just clear the map upon destroy rather
> than having these checks and nullifying the maps.

I still have to try it out but I feel like you are right about not nullifying the Map objects. I looked into the toolbox events which the source map service listens to, and I think I understand why we were seeing that behavior. I will look into it and submit another patch if it works better.

Comment 7

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

(In reply to Jaideep Bhoosreddy [:jbhoosreddy] from comment #2)
> Apparently, I'm unable to assign myself to this bug, but I can take this.

(I have given your new Bugzilla account permission to make changes.)

Comment 8

[Jaideep Bhoosreddy [:jbhoosreddy]]

Attachment: 1296589.patch [1.0]

Removed listener in the Source Map Service for the toolbox "close" event, as it is handled by the toolbox itself, which explicitly calls the destroy method for this.

Comment 9

[Jaideep Bhoosreddy [:jbhoosreddy]]

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

Comment 10

[Jaideep Bhoosreddy [:jbhoosreddy]]

Attachment: 1296589.patch [1.0]

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/cb2e635542cf
Add guard to prevent accessing location store before it is initialized. r=jsantell

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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

Comment 13

[Kazi Nuzhat Tasnem]

I have reproduced this bug with Firefox nightly 51.0a1(build id:20160819030226)on
windows 7(64 bit)

I have verified this bug as fixed with Firefox beta 51.0b8(build id:20161215085501)
User Agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0

[bugday-20161214]