Merging Network Extension

[mliao95]

This PR adds the network tool to the devtools extension! The majority of this change was moving over the various patches and tests unique to the Network tool.

Here are some other notable changes from in this PR:

• Created "applyAppendTabPatch" as the one tabbing patch. This targets the 'appendTab' function which later funnels into the showTab/selectTab functions that were previously being patched.
• Simplified tests by making variable names consistent and separating tests that were grouped together (some tests has result1, result2, result3...)
• Changed .tabbed-pane-right-toolbar patch to be visibility: hidden over display: none since it was hiding the network search close button and could not reveal when its parent had display: none
• Removed applyInspectorCommonCssHeaderContentsPatch since it was hiding the header toolbar. Depending on how we want to expose the Network pane, we can re-add this code

The original network extension can be found here: https://github.com/microsoft/vscode-edge-devtools-network

[vidorteg]

Is failing the tests when ran locally.

[mliao95]

Is failing the tests when ran locally.

Interesting. It is passing on mine. Are you targeting 82.0.423.0?

[vidorteg]

nit: remove extra space

[vidorteg]

Is failing the tests when ran locally.

Interesting. It is passing on mine. Are you targeting 82.0.423.0?

Never mind my environment is broken as they are behaving the same with master. I'll fix it and reopen if needed.

[vidorteg]

Left some comments

[vidorteg]

what are the tradeoffs of moving away from "selectTab". Would this prevents some tabs to be initialized and not only selected like previously?

[mliao95]

From my understanding these are how the tabs are generated:

Every tab/tool has a view associated with it, and when the ViewManager creates a tabbed section, it calls ViewManager.appendApplicable items to populate the tabs in the tabbedSection. By filtering the tab ids at this level, the tabbed headers won't and cannot add unused tabs in their tab array property.

The previous patches were _showTabElement and _selectTab.
_showTabElement rendered the tabs within the tab header. We can't render a tab that isn't added to the tab array, so we no longer need to filter here.

_selectTab previously would return early if a tab that wasn't on the exception list was clicked making it a noop. This is what would cause tabs to be unselectable. Since we are only appending tabs that we want, we do not need a check to make certain tabs unselectable.

[vidorteg]

it still sounds to me that we need to completely understand the effect of this change. As an example:

inspector_main\InspectorMain.js, during SourcesPanelIndicator constructor:
self.UI.inspectorView.setPanelIcon('sources', icon);

in SetTabbedPane.js setTabIcon is defined as:
setTabIcon(id, icon) {
const tab = this._tabsById.get(id);
tab._setIcon(icon);
this._updateTabElements();
}

Will throw an exception as the id is not registered in the tab array . I tested this theory on an environment with the PR changes applied and got the same error:

[mliao95]

I just restored the applySelectTabPatch and applyShowTabElement functions and removed the appendTab patch. In general, I don't love that there are so many hard coded tab ID references in the source code, but I guess we have to work around that in our patches.

[vidorteg]

let's start adding some comments on each patch that briefly describes what exactly they are patching.(2-5 lines), otherwise the review (and any subsequent fixes) become time consuming. I'll create a similar PR for the existing patches.

[mliao95]

Good idea. Adding comments.

[vidorteg]

This function is becoming crowded, let's use an "atomic" approach: individual functions for individual patches, that way we can also add a description to know exactly what is it patching.

[mliao95]

Splitting this up into General CSS changes, Network CSS changes and context menu changes.

[vidorteg]

Nice!, thanks for tackling this out 👍