This bug is for the backend of the storage tool. This bug will cover the first iteration of the actors. Attaching patch with: - The main storage actor - Three child actors : cookies, local storage and session storage. For version 1, we are targeting read only. So the patch: - Allows you to query and get all hosts and cookies/local storage items/session storage items - All child iframes/windows etc are also listed. - Gives live update on any change in the above three. The detailed protocol definition is present at https://etherpad.mozilla.org/storage-inspector-protocol Things to do: - Lot of duplicate code - On some sites, local(session)storage checks for the dom-storage2-changed event are not working properly because of how it is implemented right now. - The update event should be batched. - Add comments. - Cookies created via Set-Cookie header from network calls are not accessible. There is no way to link these cookies to the current web page as the domain of those cookies is different and platform does not link them to the current page domains. - Make sure the actor works in chrome mode. - Make sure the actor works across products (android, FxOS etc) Here is a little script to see the actor in action : https://gist.github.com/scrapmac/8714857 (Run this on a webpage, like facebook etc)

Things to do: - Lot of duplicate code - Add comments. - Cookies created via Set-Cookie header from network calls are not accessible. There is no way to link these cookies to the current web page as the domain of those cookies is different and platform does not link them to the current page domains. - Make sure the actor works in chrome mode. - Make sure the actor works across products (android, FxOS etc)

Comment on attachment 8368027 [details] [diff] [review] actor Yes! This looks excellent. I have not done a serious review, but it looks like the right way to do it. For the next round, please make sure to add comments (many, I'm dumb), and some tests. I don't think we want to work on the frontend here, but it'd be nice if you could share some scratchpad code to test these actors. --- >+types.addDictType("storeUpdateObject", { >+ changed: "nullable:json", >+ deleted: "nullable:json", >+ added: "nullable:json" >+}); "storetransactionobject" maybe. >+ >+// Cookies actor and front >+ >+ >+let CookiesActor = protocol.ActorClass({ >+ typeName: "cookies", >+ >+ get conn() { >+ return this.storageActor.conn; >+ }, >+ >+ get hosts() { >+ let hosts = new Set(); >+ for (let {location} of this.storageActor.windows) { this.windows >+ if (changed) { >+ events.emit(this, "window-" + event.type + "ed", window); I died a little.

(In reply to Girish Sharma [:Optimizer] from comment #1) > - Cookies created via Set-Cookie header from network calls are not > accessible. There is no way to link these cookies to the current web page as > the domain of those cookies is different and platform does not link them to > the current page domains. Can you explain that? > - Make sure the actor works in chrome mode. Do we really care?

(In reply to Paul Rouget [:paul] from comment #3) > (In reply to Girish Sharma [:Optimizer] from comment #1) > > - Cookies created via Set-Cookie header from network calls are not > > accessible. There is no way to link these cookies to the current web page as > > the domain of those cookies is different and platform does not link them to > > the current page domains. > > Can you explain that? Using the gist that I posted in comment 0 to list out all the cookies. Go to nytimes.com and list out all cookies for host "nytimes.com" . Compare the list with what Chrome devtools show you under the host "nytimes.com" . In Chrome, you will see many other cookies from sites like "doubleclick.net" , "imrworldwide.com" . These cookies were created when images were loaded from these sites as a part of "Set-Cookie" response header. > > - Make sure the actor works in chrome mode. > > Do we really care? Browser Toolbox won't need this tool ?

(In reply to Girish Sharma [:Optimizer] from comment #4) > (In reply to Paul Rouget [:paul] from comment #3) > > (In reply to Girish Sharma [:Optimizer] from comment #1) > > > - Cookies created via Set-Cookie header from network calls are not > > > accessible. There is no way to link these cookies to the current web page as > > > the domain of those cookies is different and platform does not link them to > > > the current page domains. > > > > Can you explain that? > > Using the gist that I posted in comment 0 to list out all the cookies. > > Go to nytimes.com and list out all cookies for host "nytimes.com" . Compare > the list with what Chrome devtools show you under the host "nytimes.com" . > In Chrome, you will see many other cookies from sites like "doubleclick.net" > , "imrworldwide.com" . These cookies were created when images were loaded > from these sites as a part of "Set-Cookie" response header. How can we fix that? Maybe you want to ask on dev-platform?

(In reply to Paul Rouget [:paul] from comment #5) > (In reply to Girish Sharma [:Optimizer] from comment #4) > > (In reply to Paul Rouget [:paul] from comment #3) > > > (In reply to Girish Sharma [:Optimizer] from comment #1) > > > > - Cookies created via Set-Cookie header from network calls are not > > > > accessible. There is no way to link these cookies to the current web page as > > > > the domain of those cookies is different and platform does not link them to > > > > the current page domains. > > > > > > Can you explain that? > > > > Using the gist that I posted in comment 0 to list out all the cookies. > > > > Go to nytimes.com and list out all cookies for host "nytimes.com" . Compare > > the list with what Chrome devtools show you under the host "nytimes.com" . > > In Chrome, you will see many other cookies from sites like "doubleclick.net" > > , "imrworldwide.com" . These cookies were created when images were loaded > > from these sites as a part of "Set-Cookie" response header. > > How can we fix that? Maybe you want to ask on dev-platform? filed bug 970246

Things to do: - Figure out why the unload event on ChromeEventHandler for google.co.in does not fire. - Add a property in "dom-storage2-change" notification to tell the type of storage as window.localStorage == event.storageArea does not work if window was obtained from "load" event of ChromeEventHandler of the docshell. - Make sure the actor works in chrome mode. - Make sure the actor works across products (android, FxOS etc)

This is more or less ready for review. I have filed bugs for what I found were platform issues. There are a couple of more known issues too, which I will try to figure out and fix in the time being, or open new bugs if they are too big of issues. Things to do: - Figure out why the unload event on ChromeEventHandler for google.co.in does not fire. - Handle edge cases , like when local storage is not accessible. etc. - Cookies start getting added before the window is completely loaded and thus have a reference to the hostname. - Make sure the actor works in chrome mode. - Make sure the actor works across products (android, FxOS etc)

As a note: would it be possible to create a panel/window that will show in an styled way all current window storage? Including flash cookies, that would help have an overview of what's currently going on in a web page.

(In reply to sys.sgx from comment #9) > As a note: would it be possible to create a panel/window that will show in > an styled way all current window storage? Including flash cookies, that > would help have an overview of what's currently going on in a web page. you mean to see (all the cookies + key-value pairs of *storage + indexed db entries + what not ) at once ? Not just being heavy on the protocol layer, it would be too much of an information imo. Maybe we can have a summary of everything, like "10 cookies in 2 frames and 5 local storage keys in 3 frames" ? About flash cookies - I will have to see if platform has APIs to get them. But are they being used that much ?

Yes, that's pretty much the point; have something panel-like that will allow for easy overview of what is currently on memory for this window (not full details). About the flash cookies, it's about privacy and being able to modify which ones are stored in the browser.

Fixed a couple of issues where localstroage is inaccessible and hostname is not present. Also fixed the way the "cookie list" command pairs up cookies with current page. I still have to replace the docshell load and unload with something else as it has many issues that I am discovering one by one. this is the thread in dev-platform for the reference. - https://groups.google.com/forum/#!topic/mozilla.dev.platform/9qmcvjg_0G4

Comment on attachment 8375025 [details] [diff] [review] patch v0.2 Review of attachment 8375025 [details] [diff] [review]: ----------------------------------------------------------------- I like it so far. This is more of a f+ than an r+ because we've got tests etc still to do. ::: toolkit/devtools/server/actors/storage.js @@ +109,5 @@ > + * The topic which this actor listens to via Notification Observers. > + * @param {string} storeObjectType > + * The RetVal type of the store object of this actor. > + */ > +StorageActors.defaults = function(typeName, observationTopic, storeObjectType) { It feels there are lots of levels to this. I'm having to jump up and down the code a lot to work out what's going on. Can't we simplify things? @@ +291,5 @@ > + * Actor. > + * > + * @See StorageActors.defaults() > + * @param {object} options > + * Options required by StorageActors.defaults method. Please define them, at least by reference to the defaults function

(In reply to Joe Walker [:jwalker] from comment #13) > Comment on attachment 8375025 [details] [diff] [review] > patch v0.2 > > Review of attachment 8375025 [details] [diff] [review]: > ----------------------------------------------------------------- > > I like it so far. > > This is more of a f+ than an r+ because we've got tests etc still to do. > > ::: toolkit/devtools/server/actors/storage.js > @@ +109,5 @@ > > + * The topic which this actor listens to via Notification Observers. > > + * @param {string} storeObjectType > > + * The RetVal type of the store object of this actor. > > + */ > > +StorageActors.defaults = function(typeName, observationTopic, storeObjectType) { > > It feels there are lots of levels to this. I'm having to jump up and down > the code a lot to work out what's going on. > Can't we simplify things? It was previously simplified, but with lots and lots of repetition and thus, chances of error. Even I don't like this much code reusage , but now, adding a new actor is dead easy and small. Do you have some suggestions on a good tradeoff ? And yes, tests are next on my list :)

Added tests to check proper listing of cookies, local storage and session storage across iframes and main page. try : https://tbpl.mozilla.org/?tree=Try&rev=124f1fc1718f

[...contd.] And also switched the new suggested way of tracking addition and deletion of windows (and not load and unload of windows). This new approach handles BFCached windows and chrome to content transitions perfectly well. Also incorporated changes based on the patch from bug 970246 so that the cookies set vie SET-COOKIE (read comment 4 for more info) are also listed in the inspector. But only when the page load is happening while the tool is open.

No need to QI nsIDocShellTreeItem (nsIDocShell now inherits from it). + // Notifications that help us keep track of newly added windows and windows + // that got removed + Services.obs.addObserver(this, "content-document-global-created", false); + Services.obs.addObserver(this, "dom-window-destroyed", false); Great! + window.top == this.window) { This won't work in case of apps and mozbrowsers. Don't trust `.top` and `.parents`. Use docshell methods.

new try push : https://tbpl.mozilla.org/?tree=Try&rev=f30a097c91ee

- Using LayoutHelpers docshell involving methods instead of window.top. - Fixed the xpcshell tests. - Explicitly calling DebuggerServer.destroy() so that any left over actors are removed - Some other console error fixes. try : https://tbpl.mozilla.org/?tree=Try&rev=f34411b130a9

It would be worth attaching some screenshots so as to view how it might look like and provide some feedback for the UI.

(In reply to sys.sgx from comment #20) > It would be worth attaching some screenshots so as to view how it might look > like and provide some feedback for the UI. This bug is not for UI, bug 970517 is.

There was a discrepency of expires time unit in nsCookie and the http-on-response-set-cookie notification. Fixed it. Also fixed the cookie parsing logic in http-on-response-set-cookie case where somethings there will be no space b/w ; and next key, like Expires. I also changed the packet structure from hosts:[ { host: "...", stores: [...], }, { ... }] to hosts: { <host1>: [...], // host: stores ... } because it was shorter and it let me manage the batched updates in an easier and faster manner. The batched update has some issues like when sometimes, the a partiular cookie will get added and deleted (or modified) in the same batch and then frontend would not know what happened the last. So now the backend properly handles batch operation by clubbing this kind of situations. This also reduced the packet size a lot. At this point, I would like to get some feedback on what all other properties of cookies are required from the web developer perspective ? Size of cookies can be done on the front end itself. The remaining available properties are: - isDomain - isSecure - policy - status - creationTime - expiry - isHttpOnly - isSession - lastAccessed - rawHost

Actually, having all of them would be nice. By adding a way to view these information for each entry it would provide more info to the dev.

Comment on attachment 8379843 [details] [diff] [review] patch v0.5 Review of attachment 8379843 [details] [diff] [review]: ----------------------------------------------------------------- Looks good so far, one trivial comment. What do you see as outstanding in this file? ::: toolkit/devtools/server/actors/storage.js @@ +583,5 @@ > + * This method exists as both Local Storage and Session Storage have almost > + * identical actors. > + */ > +function getObjectForLocalOrSessionStorage(type) { > +return { Could we indent this properly?

(In reply to Joe Walker [:jwalker] from comment #24) > Comment on attachment 8379843 [details] [diff] [review] > patch v0.5 > > Review of attachment 8379843 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks good so far, one trivial comment. > What do you see as outstanding in this file? I am planning to add one more test to test dynamic frame additions and batch updates, should not be too much though. Also, your opinion on the last question in comment 22 would be helpful :) > ::: toolkit/devtools/server/actors/storage.js > @@ +583,5 @@ > > + * This method exists as both Local Storage and Session Storage have almost > > + * identical actors. > > + */ > > +function getObjectForLocalOrSessionStorage(type) { > > +return { > > Could we indent this properly? I thought that it would be a wasted indentation, but I have nothing against it too :) Other that this, do I assume it has an r+ overall ?

(In reply to Girish Sharma [:Optimizer] from comment #22) > ... > > At this point, I would like to get some feedback on what all other > properties of cookies are required from the web developer perspective ? > > Size of cookies can be done on the front end itself. The remaining available > properties are: > > - isDomain > - isSecure > - isHttpOnly > - isSession > - expiry I think these are all required from a web developer perspective > - creationTime > - lastAccessed I'd say these could be useful, and we should include them if it's easy. > - rawHost This is derivable from the host, isn't it? > - policy > - status I have no idea what these are. I can guess that status is derivable from the expiry date?

(In reply to Joe Walker [:jwalker] from comment #26) > (In reply to Girish Sharma [:Optimizer] from comment #22) > > ... > > > > At this point, I would like to get some feedback on what all other > > properties of cookies are required from the web developer perspective ? > > > > Size of cookies can be done on the front end itself. The remaining available > > properties are: > > > > - isDomain > > - isSecure > > - isHttpOnly > > - isSession > > - expiry > > I think these are all required from a web developer perspective isSession can be derived from expires, same goes for expiry. I really don't know why nsiCookie2 has these properties > > - creationTime > > - lastAccessed > > I'd say these could be useful, and we should include them if it's easy. These are directly available on the nsiCookie2 interface of the cookie object. So yes, easy. > > - rawHost > > This is derivable from the host, isn't it? > > > - policy > > - status > > I have no idea what these are. I can guess that status is derivable from the > expiry date? yeah, and even the mdn page for these gives 404. So I will add: isDomain isSecure isHttpOnly creationTime lastAccessed But I will make their columns by default hidden (except for I guess lastAccessed time) as otherwise, it would be too much clutter

(In reply to Girish Sharma [:Optimizer] from comment #25) > > Could we indent this properly? > > I thought that it would be a wasted indentation, but I have nothing against > it too :) Personally, I think the cost of the "what's going on here?" is greater than the cost to the right hand margin, but I'm not militant about it :) > Other that this, do I assume it has an r+ overall ? I'd like to spend a bit more time on storage.js, and have another play, but we're nearly there. Hopefully, I'll get this done today.

(In reply to Girish Sharma [:Optimizer] from comment #27) > (In reply to Joe Walker [:jwalker] from comment #26) > > (In reply to Girish Sharma [:Optimizer] from comment #22) > > > ... > > > > > > Size of cookies can be done on the front end itself. The remaining available > > > properties are: > > > > > > - isDomain > > > - isSecure > > > - isHttpOnly > > > - isSession > > > - expiry > > > > I think these are all required from a web developer perspective > > isSession can be derived from expires, same goes for expiry. I really don't > know why nsiCookie2 has these properties isSession is a thing that web developers talk about though. "It's a session cookie" so I think it's worth having that.

(In reply to Joe Walker [:jwalker] from comment #29) > (In reply to Girish Sharma [:Optimizer] from comment #27) > > (In reply to Joe Walker [:jwalker] from comment #26) > > > (In reply to Girish Sharma [:Optimizer] from comment #22) > > > > ... > > > > > > > > Size of cookies can be done on the front end itself. The remaining available > > > > properties are: > > > > > > > > - isDomain > > > > - isSecure > > > > - isHttpOnly > > > > - isSession > > > > - expiry > > > > > > I think these are all required from a web developer perspective > > > > isSession can be derived from expires, same goes for expiry. I really don't > > know why nsiCookie2 has these properties > > isSession is a thing that web developers talk about though. "It's a session > cookie" so I think it's worth having that. That can be inferred via expires == 0. Which the frontend is also showing. If you see. This is how firebug and Chrome also display session cookies. Having another column which true only for the cases where the Expires column read "session cookie" seems redundant.

err. Just reliased I missed out an STR step. I meant: "If you see the storage inspector video https://www.youtube.com/watch?feature=player_detailpage&v=evyaSydRqFk#t=84 , you will see the Expires tab saying "Session" for many cookies."

More thoughts from playing with the UI: * We still have unresizable columns. Given that names and values have lengths that vary a lot, I don't think we can get away with this (I know this is an issue with the front end patch, but I'm obviously trying them out together) * We've got data both in the table and in the pop-up side-bar. I don't think it needs to be in both places. Perhaps we should just have name|value in the main table, and all the other values in the side-bar. Or we should get rid of the side bar and have everything in the table. Probably UX should have some input here. * We need an icon * I'm not a fan of our 'tree' implementation, but I can live with it.

(In reply to Girish Sharma [:Optimizer] from comment #30) > (In reply to Joe Walker [:jwalker] from comment #29) > > (In reply to Girish Sharma [:Optimizer] from comment #27) > > > (In reply to Joe Walker [:jwalker] from comment #26) > > > > (In reply to Girish Sharma [:Optimizer] from comment #22) > > > > > ... > > > > > > > > > > Size of cookies can be done on the front end itself. The remaining available > > > > > properties are: > > > > > > > > > > - isDomain > > > > > - isSecure > > > > > - isHttpOnly > > > > > - isSession > > > > > - expiry > > > > > > > > I think these are all required from a web developer perspective > > > > > > isSession can be derived from expires, same goes for expiry. I really don't > > > know why nsiCookie2 has these properties > > > > isSession is a thing that web developers talk about though. "It's a session > > cookie" so I think it's worth having that. > > That can be inferred via expires == 0. Which the frontend is also showing. > If you see. > > This is how firebug and Chrome also display session cookies. > > Having another column which true only for the cases where the Expires column > read "session cookie" seems redundant. I know it can be inferred. My point is that 'session' is a concept that has meaning to a web developer. They might ask the question 'is it a session cookie' and we should make that easy to answer. Perhaps having a value of 'session' in the expires column would make sense.

(In reply to Girish Sharma [:Optimizer] from comment #31) > err. Just reliased I missed out an STR step. > > I meant: > > "If you see the storage inspector video > https://www.youtube.com/watch?feature=player_detailpage&v=evyaSydRqFk#t=84 , > you will see the Expires tab saying "Session" for many cookies." Mid-air collisions. I agree.

(In reply to Joe Walker [:jwalker] from comment #32) > More thoughts from playing with the UI: > > * We still have unresizable columns. Given that names and values have > lengths that vary a lot, I don't think we can get away with this (I know > this is an issue with the front end patch, but I'm obviously trying them out > together) But they *are* resizeable . Aren't they ? (Or are you suggesting to remove the resizing functionality from the columns) > * We've got data both in the table and in the pop-up side-bar. I don't think > it needs to be in both places. Perhaps we should just have name|value in the > main table, and all the other values in the side-bar. Or we should get rid > of the side bar and have everything in the table. Probably UX should have > some input here. The sidebar is helpful in many ways : - Even if some columns are hidden, sidebar will show all properties anyways. - Sidebar shows parsed value of cookies/localstorage items. If the value is of JSON type, or a key-separated array, or a key-value pair, it is shown as an object below the normal object. - In future, I want to make the table rows multi-selectable (to perform various actions together). Then the sidebar will show all the selected items together. > * We need an icon Yes please :) > * I'm not a fan of our 'tree' implementation, but I can live with it. I agree. Moreover, this tree implementation will not work for Indexed DB. I think I will switch to itchpad's tree once indexed db part is done.

(In reply to Girish Sharma [:Optimizer] from comment #35) > This is all really about the front end, so I replied in bug 970517 comment 26.

(In reply to Joe Walker [:jwalker] from comment #28) > > Other that this, do I assume it has an r+ overall ? > > I'd like to spend a bit more time on storage.js, and have another play, but > we're nearly there. Hopefully, I'll get this done today. I've had another look, and I'd still like to see it again with better comments. FWIW a big consumer of source comments is the original reviewer, so I think in general it's a good idea to get those in place before asking for an r? As far as landing plans go - I'm OK with landing this preffed off (and invisible using the options panel), in fact I think we should do this to prevent bitrot.

(In reply to Joe Walker [:jwalker] from comment #37) > (In reply to Joe Walker [:jwalker] from comment #28) > > > Other that this, do I assume it has an r+ overall ? > > > > I'd like to spend a bit more time on storage.js, and have another play, but > > we're nearly there. Hopefully, I'll get this done today. > > I've had another look, and I'd still like to see it again with better > comments. FWIW a big consumer of source comments is the original reviewer, > so I think in general it's a good idea to get those in place before asking > for an r? Can you point out where all is a need for more comments. I was pretty thorough in putting comments in this patch wherever possible.. > As far as landing plans go - I'm OK with landing this preffed off (and > invisible using the options panel), in fact I think we should do this to > prevent bitrot. The actors can go as is. The frontend will be disable via a killSwitch pref so that its not visible even in the options panel.

Comment on attachment 8379843 [details] [diff] [review] patch v0.5 Review of attachment 8379843 [details] [diff] [review]: ----------------------------------------------------------------- There's code here that looks like it's never been executed, so I'm renewing my call for more tests. ::: toolkit/devtools/server/actors/storage.js @@ +37,5 @@ > +let storageTypePool = new Map(); > + > +/** > + * Gets an accumulated list of all storage actors registered to be used to > + * create a RetVal * create a RetVal, for example in StorageActor.listStores ? Otherwise it's just a random disconnected function @@ +484,5 @@ > + } > + return cookies; > + }, > + > + observe: function(subject, topic, action) { Could you document these parameters? @@ +568,5 @@ > + this.storageActor.update("cleared", "cookies"); > + break; > + > + case "reload": > + this.storageActor.update("relaod", "cookies"); "reloaded"? @@ +709,5 @@ > + return this.childWindowPool; > + }, > + > + /** > + * List of solicit event notifications that the server can send to the client. 'solicit'? Can we just delete that word? @@ +711,5 @@ > + > + /** > + * List of solicit event notifications that the server can send to the client. > + * > + * - storage-update : When any store object in any storage type changes. stores-update? @@ +805,5 @@ > + return null; > + } > + let window = docShell.contentViewer.DOMDocument.defaultView; > + // TODO - figure out this use case. Some times, a normal url-ed window also > + // has href as about:blank when it is unloaded. TODO: Probably needs a bug and a BUG XXXXXX marker @@ +837,5 @@ > + } > + // else if (topic == "dom-window-destroyed" && > + // this.childWindowPool.delete(window)) { > + // events.emit(this, "window-destroyed", window); > + // } Commented out code - why? @@ +921,5 @@ > + // throw new Error(); > + // } > + // catch (ex) { > + // console.log(action, storeType, data, this.boundUpdate, ex.stack); > + // } This shouldn't be here

- Addressed all review comments. - Figured out some edge cases and other TODO comments' solutions. Big thanks to smaug for helping me out with the window-observer. - Added additional information for cookies - Adding more tests now. will attach a patch tomorrow.

Added tests for test whether: - reloading/addition/deletion of windows/iframes works properly - storage changes [cookies/localStorage/sessionStorage changes] are reflected properly. On going try: https://tbpl.mozilla.org/?tree=Try&rev=9a2a8b3192ae

(I am looking into the intermittents happening on debug machines. Please feel free to review the current patch and I will keep this bug posted with newer trys attempting to fix the intermittents)

Comment on attachment 8388235 [details] [diff] [review] patch v1.0 Review of attachment 8388235 [details] [diff] [review]: ----------------------------------------------------------------- Noticed something that could be important about ports for cookies... r+ with these 3 things fixed, but I'd like to know the conclusion to the storage/cookie/port issue. ::: toolkit/devtools/server/actors/storage.js @@ +645,5 @@ > + } > + return location.protocol + "//" + location.hostname; > + }, > + > + populateStoresForHost: function(host, window) { What's the host param for? It doesn't look used? @@ +695,5 @@ > + > + /** > + * Given a url, correctly determine its protocol + hostname part. > + */ > + getHostFromURL: function(url) { This doesn't actually get the host. How about 'getSchemeAndHost(...)' ? Also I think the logic here is wrong isn't it? For cookies the port isn't important, but localStorage and sessionStorage follow the origin rules properly so the port is important.

(In reply to Joe Walker [:jwalker] from comment #43) > Comment on attachment 8388235 [details] [diff] [review] > patch v1.0 > > Review of attachment 8388235 [details] [diff] [review]: > ----------------------------------------------------------------- > > Noticed something that could be important about ports for cookies... > r+ with these 3 things fixed, but I'd like to know the conclusion to the > storage/cookie/port issue. Thanks :) > ::: toolkit/devtools/server/actors/storage.js > @@ +645,5 @@ > > + } > > + return location.protocol + "//" + location.hostname; > > + }, > > + > > + populateStoresForHost: function(host, window) { > > What's the host param for? It doesn't look used? You are right, I should actually be using it instead of calculating the host again. > @@ +695,5 @@ > > + > > + /** > > + * Given a url, correctly determine its protocol + hostname part. > > + */ > > + getHostFromURL: function(url) { > > This doesn't actually get the host. How about 'getSchemeAndHost(...)' ? Yeah, its not just host, but scheme. Will rename. > Also I think the logic here is wrong isn't it? For cookies the port isn't > important, but localStorage and sessionStorage follow the origin rules > properly so the port is important. Correct again. Will add port (and hide the port if its 80) alongside the protocol + hostname part .

trying to fix the errors happening in slow machines : https://tbpl.mozilla.org/?tree=Try&rev=fc434c319538

latest try is green : https://tbpl.mozilla.org/?tree=Try&rev=922d4fb32037 Joe, should I go ahead and land this or should we wait for the merge ?

(In reply to Girish Sharma [:Optimizer] from comment #46) > Joe, should I go ahead and land this or should we wait for the merge ? Several oranges there, but if you're sure they're not related, then go ahead and land.

Yup, they are mostly existing intermittents, and some of them also have been fixed already in the tip. I will rebase the patch on latest fx-team tip and again pass it through full try before landing.

addressed comments from last review. final try : https://tbpl.mozilla.org/?tree=Try&rev=9395fa796e26

and for some reason, neither bugzilla interdiff nor http://benjamin.smedbergs.us/interdiff/ are working for the small interdiff b/w v1 and v1.1 . The only thing close that I can get is : http://diffchecker.com/ctx6gifq

landed in fx-team with minor info() call string changes in one test - https://hg.mozilla.org/integration/fx-team/rev/0dd61eada6c9

mass component move . filter on #MassComponentMoveStorageInspector