1531642 - The two throttle dropdown menus contradict each other

Status: NEW

(DevTools :: Responsive Design Mode, defect, P3)

Description

[Matthew Davis]

Attachment: throttle.png

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

1. Load any page
2. Right click on any element
3. click "inspect element" (or if you know a quicker way to get to the dev tools, use that)
4. click on responsive design mode
5. click on the network tab
6. Click on "No throttling" in the network tab.
7. Select "Regular 2G"

Actual results:

The throttling dropdown in the responsive mode toolbar still says "No Throttling".
If you then change that dropdown menu, the other dropdown menu doesn't change.

See the printscreen

Expected results:

The throttling dropdown in the responsive mode toolbar should say "Regular 2G"

Comment 1

[Razvan Caliman [:rcaliman]]

The NetworkThrottling menu component and its Redux action/reducer were extracted from Netmonitor to be reused in the Responsive Design Mode (aka RDM). Theoretically, dispatching the shared action should keep the component in sync. However, the action is actually dispatched to two separate stores, one for RDM and one for Netmonitor. Because of this, they are out of sync.

A one-way sync could have been established by leveraging Netmonitor's API, requested via the toolbox.getNemonitorAPI(), and listening for the network throttling change event. Additionally, the RDM could directly call this API to set the throttling instead of calling the corresponding methods on EmulationFront. However, that doesn't seem possible. The RDM is not connected to the Toolbox, therefore it can't get a reference to the Netmonitor API.

The EmulationFront/EmulationActor seems to be the shared bit between the two panels. One way I see to solve this bug is to extend that to fire an event whenever the throttling pref is set, listen to that event in both the Netmonitor and RDM clients, then dispatch the action to each of their Redux stores to sync the NetworkThrottlingMenu components.

Of course, if there's a practical way to connect the RDM to the Toolbox (I'm not sure if that's feasible or desired), the issue could be solved by using the Netmonitor API as described above.

Pinging Gabriel and Honza who have more insight into the two panels to sanity-check the proposals above. I'm open to other ideas too.

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

(In reply to Razvan Caliman [:rcaliman] from comment #1)

The EmulationFront/EmulationActor seems to be the shared bit between the two panels. One way I see to solve this bug is to extend that to fire an event whenever the throttling pref is set, listen to that event in both the Netmonitor and RDM clients, then dispatch the action to each of their Redux stores to sync the NetworkThrottlingMenu components.

This sounds like the way to go.

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Gabriel [:gl] (ΦωΦ) from comment #2)

(In reply to Razvan Caliman [:rcaliman] from comment #1)

The EmulationFront/EmulationActor seems to be the shared bit between the two panels. One way I see to solve this bug is to extend that to fire an event whenever the throttling pref is set, listen to that event in both the Netmonitor and RDM clients, then dispatch the action to each of their Redux stores to sync the NetworkThrottlingMenu components.

This sounds like the way to go.
Sounds promising to me too.

Honza

Comment 4

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1531642 - Introduce a singleton to share Network Throttling Menu state across RDM and Netmonitor

Context: The RDM and Netmonitor live in separate DebuggerClient instances with their own instances of Emulation Actor and Front. They share a React component with a menu for network throttling, but it's attached to different Redux stores so its state is not reflected in both RDM and Netmonitor panels at the same time.

This patch introduces a singleton (NetworkThrottlingObserver) to which both instances of the throttling menu pass their state and listen to changes. The React component essentially listens to changes coming from itself and from other instances of it.

When the component's state is different from the incoming network throttling status, it triggers the change handler as if the user selected that option from the menu. If the state is the same as the incoming status, nothing happens. This avoids going into an infinite loop.

This follows a similar approach used on the server for Track Changes with TrackChangeEmitter.

Comment 5

[Razvan Caliman [:rcaliman]]

The solution proposed in Comment #1 is not feasible because the RDM and Toolbox holding the Netmonitor live in separate worlds with their own instances of Emulation actors and fronts. They are not shared instances and therefore cannot be used to pass state between RDM and Netmonitor.

The solution attached with Comment #4 works but has drawbacks: it synchronizes state between all instances of open RDM and Netmonitor - this means throttling other tabs with DevTools open. Clearly, that's not desired either. Users must be able to isolate network throttling to specific tabs.

Comment 6

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1531642 - (Part 1) Make EmulationActor instances communicate network throttling changes to each other. r=ochameau

Context: RDM and Netmonitor are connected to separate DebuggerClients and have distinct instances of EmulationActor/Front which cannot communicate directly with each other. This means their network throttling menus are out of sync.

This patch introduces a singleton, NetworkThrottlingObserver, towards which all instances of EmulationActor log when they change network throttling and to which all instances listen for changes (this means they react to their own changes, like an echo).

The checks in the setNetworkThrottling() are fixed so that an EmulationActor instance doesn't go into an infinite loop by reacting to its own changes.

An event is fired when throttling changes. This is exposed to the consumers on the client via EmulationFront which enables RDM and Netmonitor to sync their network throttling menus.

Comment 7

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1531642 - (Part 2) Expose network throttling profiles and a method to match a profile. r=mtigley

Depends on D26124

This patch changes the default export of the module from a list of profiles to separate exports:

• a list of profiles
• a method used to indentify a profile based on throttling constraits

This will be used in Part 3 and Part 4 of this series to match profiles based on events coming from the server.

Comment 8

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1531642 - (Part 3) Make RDM listen for network throttling change events from EmulationFront to sync with Netmonitor.

Depends on D26127

RDM and Netmonitor are connected to separate DebuggerClients and have distinct instances of EmulationActor/Front which cannot communicate directly with each other. This means their network throttling menus are out of sync.

The changes in Part 1 of this series make it so the EmulationActor instances react to changes from one another. When throttling is changed in any one EmulationActor, an event is sent via the EmulationFront to the consumer client with the latest throttling data (coming from itself or from the other instance). This enables RDM and Netmonitor clients to sync their network throttling menus.

This patch introduces changes to the RDM manager class which interfaces with the EmulationFront so it handles "network-throttling-change" events. It uses these events to call the Redux action with the selected throttling profile which updates the menu UI. Since EmulationActors emit even on their own changes (like an echo), we guard against triggering a change with the same throttling profile to avoid needless updates (the EmulationActor has its own guards to prevent going into infinite loops).

Comment 9

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1531642 - (Part 4) Make Netmonitor listen for network throttling change events from EmulationFront to sync with RDM. r=honza

Depends on D26128
RDM and Netmonitor are connected to separate DebuggerClients and have distinct instances of EmulationActor/Front which cannot communicate directly with each other. This means their network throttling menus are out of sync.

The changes in Part 1 of this series make it so the EmulationActor instances react to changes from one another. When throttling is changed in any one EmulationActor, an event is sent via the EmulationFront to the consumer client with the latest throttling data (coming from itself or from the other instance). This enables RDM and Netmonitor clients to sync their network throttling menus.

This patch updates the firefox-connector used by the NetmonitorAPI to listen to "network-throttling-change" events coming from the EmulationFront as a result of changes to throttling. It handles this event by attempting to match the profile according to the network throttling constraints provided with the event. It then fires an event with this matched profile id which is listened to by the Toolbar which dispatches the Redux action to update the selected item in the network throttling menu.

Because "network-throttling-change" can fire as a result of changes from our own EmulationActor (like an echo) or from RDM's EmulationActor, we guard against dispatching the Redux action with the same profile to prevent needless updates. (Throttling middleware reacting to that Redux action would cause another trip to the server in case of an echo) The EmulationActor has its own guards to prevent it from going into an infinite loop.

Comment 11

[Razvan Caliman [:rcaliman]]

Attachment: sharedfront

Depends on D26129