Bug 190160 – Web Inspector: rename frontend managers to be more consistent with backend agents

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Devin Rousso]

e.g. `WI.FrameResourceManager` should really be `WI.NetworkManager`.

[Devin Rousso]

Created attachment 351343 [details]
Patch

[Joseph Pecoraro]

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.

[Devin Rousso]

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.

[Devin Rousso]

Created attachment 351435 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 351435 [details]
Patch

Clearing flags on attachment: 351435

Committed r236766: <https://trac.webkit.org/changeset/236766>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44955406>