Always restoring through sidebars causes the inspector to lose track of the previously-opened sidebar. The current architecture of save/restore from cookie also makes it awkward to restore view state that is independent of the represented object (i.e., scroll positions or subview selections).
<rdar://problem/15185630>
Created attachment 213774 [details] v1
Comment on attachment 213774 [details] v1 View in context: https://bugs.webkit.org/attachment.cgi?id=213774&action=review Looks good. Just fix up the layering violations. > Source/WebInspectorUI/UserInterface/ApplicationCacheManager.js:115 > + return (matchOnTypeAlone && this._applicationCacheObjects.length) ? this._applicationCacheObjects[0] : null; No need for the ( ). > Source/WebInspectorUI/UserInterface/ContentBrowser.js:78 > + // These listeners are for events that could resolve a pending content view cookie. > + WebInspector.applicationCacheManager.addEventListener(WebInspector.ApplicationCacheManager.Event.FrameManifestAdded, this._resolveAndShowPendingContentViewCookie, this); > + WebInspector.frameResourceManager.addEventListener(WebInspector.FrameResourceManager.Event.MainFrameDidChange, this._resolveAndShowPendingContentViewCookie, this); > + WebInspector.storageManager.addEventListener(WebInspector.StorageManager.Event.DatabaseWasAdded, this._resolveAndShowPendingContentViewCookie, this); > + WebInspector.storageManager.addEventListener(WebInspector.StorageManager.Event.CookieStorageObjectWasAdded, this._resolveAndShowPendingContentViewCookie, this); > + WebInspector.storageManager.addEventListener(WebInspector.StorageManager.Event.DOMStorageObjectWasAdded, this._resolveAndShowPendingContentViewCookie, this); This is a layering violation. Moving this to Main.js and possibly calling into a public function on ContentBrowser for generic logic would be best. > Source/WebInspectorUI/UserInterface/ContentBrowser.js:577 > + _resolveAndShowPendingContentViewCookie: function(matchOnTypeAlone) > + { > + var cookie = this._pendingContentViewCookie; > + if (!cookie) > + return false; > + > + var representedObject = null; > + > + if (cookie.type === WebInspector.ContentView.ResourceContentViewCookieType) > + representedObject = WebInspector.frameResourceManager.objectForCookie(cookie); > + > + if (cookie.type === WebInspector.ContentView.TimelinesContentViewCookieType) > + representedObject = WebInspector.timelineManager.objectForCookie(cookie); > + > + if (cookie.type === WebInspector.ContentView.CookieStorageContentViewCookieType || cookie.type === WebInspector.ContentView.DatabaseContentViewCookieType || cookie.type === WebInspector.ContentView.DatabaseTableContentViewCookieType || cookie.type === WebInspector.ContentView.DOMStorageContentViewCookieType) > + representedObject = WebInspector.storageManager.objectForCookie(this._pendingContentViewCookie, matchOnTypeAlone); > + > + if (cookie.type === WebInspector.ContentView.ApplicationCacheContentViewCookieType) > + representedObject = WebInspector.applicationCacheManager.objectForCookie(this._pendingContentViewCookie, matchOnTypeAlone); > + > + if (!representedObject) > + return false; > + > + // If we reached this point, then we should be able to create and/or display a content view based on the cookie. > + delete this._pendingContentViewCookie; > + if (this._lastAttemptCookieCheckingTimeout) > + clearTimeout(this._lastAttemptCookieCheckingTimeout); > + > + // Delay this work because other listeners of the originating event might not have fired yet. > + // So displaying the content view before those listeners do their work might cause the > + // dependent view states (navigation sidebar tree elements, path components) to be wrong. > + function delayedWork() > + { > + this.showContentViewForRepresentedObject(representedObject, cookie); > + } > + setTimeout(delayedWork.bind(this), 0); > + return true; > + }, This should also move to Main.js since it touches many high level classes that ContentBrowser should not know about. > Source/WebInspectorUI/UserInterface/ContentView.js:105 > +WebInspector.ContentView.ApplicationCacheContentViewCookieType = "application-cache"; > +WebInspector.ContentView.CookieStorageContentViewCookieType = "cookie-storage"; > +WebInspector.ContentView.DatabaseContentViewCookieType = "database"; > +WebInspector.ContentView.DatabaseTableContentViewCookieType = "database-table"; > +WebInspector.ContentView.DOMStorageContentViewCookieType = "dom-storage"; > +WebInspector.ContentView.TimelinesContentViewCookieType = "timeline"; > +WebInspector.ContentView.ResourceContentViewCookieType = "resource"; // includes Frame too. This is also a layering violation. Main.js would be best. Also this format would be better and more like an enum: WebInspector.ContentVIewCookieType = { ApplicationCache: "application-cache", ... };
Created attachment 213897 [details] v2
Comment on attachment 213897 [details] v2 View in context: https://bugs.webkit.org/attachment.cgi?id=213897&action=review Looks good. Just some style nits. > Source/WebInspectorUI/UserInterface/FrameResourceManager.js:86 > + var representedObject = (cookie.url) ? this.resourceForURL(cookie.url) : this.mainFrame; No need for ( ). > Source/WebInspectorUI/UserInterface/StorageManager.js:159 > + return (matchOnTypeAlone && array.length) ? array[0] : null; No need for ( ). > Source/WebInspectorUI/UserInterface/StorageManager.js:179 > + if (cookie.type === WebInspector.ContentViewCookieType.DOMStorage) > + return findMatchingObjectInArray(this._domStorageObjects, function(object) { > + return object.host === cookie.host && object.isLocalStorage() === cookie.isLocalStorage; > + }); This needs { } since it is multi-line. > Source/WebInspectorUI/UserInterface/StorageManager.js:184 > + if (cookie.type === WebInspector.ContentViewCookieType.Database) > + return findMatchingObjectInArray(this._databaseObjects, function(object) { > + return object.host === cookie.host && object.name === cookie.name; > + }); Ditto. > Source/WebInspectorUI/UserInterface/StorageManager.js:192 > + if (cookie.type === WebInspector.ContentViewCookieType.DatabaseTable) > + return findMatchingObjectInArray(this._databaseObjects, function(object) { > + return object.host === cookie.host && object.database === cookie.name; > + }); Ditto.
Created attachment 213935 [details] v2.1 (style nits)
Comment on attachment 213935 [details] v2.1 (style nits) Clearing flags on attachment: 213935 Committed r157269: <http://trac.webkit.org/changeset/157269>
All reviewed patches have been landed. Closing bug.