e.g. `WI.FrameResourceManager` should really be `WI.NetworkManager`.
Created attachment 351343 [details] Patch
Comment on attachment 351343 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351343&action=review Can you put up a new patch after adding the following to your git config: [diff] renames = copies I think it would eliminate the full content of files in the diff that were just renamed and instead include an actual diff. Also when it is applied it should perform an svn copy of the files and carry blame history forwards more appropriately. > Source/WebInspectorUI/ChangeLog:20 > + * UserInterface/Controllers/DatabaseManager.js: Copied from Source/WebInspectorUI/UserInterface/Controllers/IssueManager.js. Heh, the ChangeLog says this was copied from IssueManager.js I think probably StorageManager.js? > Source/WebInspectorUI/UserInterface/Base/Main.js:120 > this.issueManager = new WI.IssueManager; This one can probably just fold into ConsoleManager. Heck, ConsoleManager is the only thing that adds issues to IssueManager. > Source/WebInspectorUI/UserInterface/Base/Main.js:400 > + this._dashboards = { > + default: new WI.DefaultDashboard, > + debugger: new WI.DebuggerDashboard, > + }; How about we just make this: this._defaultDashboard = ...; this._debuggerDashboard = ...; Instead of a `dashboards` object. > Source/WebInspectorUI/UserInterface/Controllers/ConsoleManager.js:155 > + PreviousMessageRepeatCountUpdated: "console-manager-previous-message-repeat-count-updated" Style: Trailing comma > Source/WebInspectorUI/UserInterface/Controllers/DOMStorageManager.js:185 > + Cleared: "dom-storage-manager-cleared" Nit: Add the trailing comma. to these so if we add a new event it will be a simple diff. > Source/WebInspectorUI/UserInterface/Controllers/IndexedDBManager.js:189 > + Cleared: "indexed-db-manager-cleared" Style: Trailing comma > Source/WebInspectorUI/UserInterface/Test/Test.js:-53 > - this.storageManager = new WI.StorageManager; Do we not need IndexedDB or Database in tests?! Please run: $ run-webkit-tests --no-retry-failures --no-sample-on-timeout --time-out-ms=5000 --force inspector And see if there are failures on tests that might be getting skipped by bots.
Comment on attachment 351343 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351343&action=review >> Source/WebInspectorUI/UserInterface/Base/Main.js:400 >> + }; > > How about we just make this: > > this._defaultDashboard = ...; > this._debuggerDashboard = ...; > > Instead of a `dashboards` object. I prefer the idea of separating like objects into containers, similar to <https://webkit.org/b/190165>. The top-level `WI` is already "polluted" enough. >> Source/WebInspectorUI/UserInterface/Test/Test.js:-53 >> - this.storageManager = new WI.StorageManager; > > Do we not need IndexedDB or Database in tests?! > > Please run: > > $ run-webkit-tests --no-retry-failures --no-sample-on-timeout --time-out-ms=5000 --force inspector > > And see if there are failures on tests that might be getting skipped by bots. Our IndexedDB tests seem to directly call IndexedDBAgent.
Created attachment 351435 [details] Patch
Comment on attachment 351435 [details] Patch Clearing flags on attachment: 351435 Committed r236766: <https://trac.webkit.org/changeset/236766>
All reviewed patches have been landed. Closing bug.
<rdar://problem/44955406>