1721379 - browser_touch_device.js fails with server side target switching enabled

Status: RESOLVED FIXED

(DevTools :: Responsive Design Mode, defect)

Description

[Alexandre Poirot [:ochameau]]

$ ./mach mochitest --headless devtools/client/responsive/test/browser/browser_touch_device.js --setpref devtools.target-switching.server.enabled=true

There is many issues with this test.

• waitForViewportLoad may benefit from listening to DOCUMENT_EVENT dom-complete in order to better wait for all the updates. But I'm not longer sure it is important?
• BrowsingContextTargetActor.updateTargetConfiguration is being called when the related document has just started loading. But this method will try to reload the document in order to ensure applying the new configuration. This reload introduce infinite loop. When this method is called on document instantiation, we shouldn't need nor try to reload. The platform flags should be set early enough so that it has the right impact on the document to be loaded.
• We still call setMetaViewportOverride/clearMetaViewportOverride on each target. We end up calling these methods on already destroyed target. This introduce exception which make the test fail. We may benefit from moving this to target configuration. Otherwise we should at least avoid the exceptions.
• RDM ui.js tries to restore the actor state on each new target from here. We try to call some target actor on each new target and also call target configuration again. Calling target configuration isn't necessary. Target configuration itself should handle passing the data to the new target from the server side. So we should be able to remove that and only update the configuration when RDM opens, and later only when one particular config changes. I'm not sure that causes this test to fail, but I witness exceptions related to that.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1721379 - [devtools] Fix browser_touch_device.js with server target switching

• waitForViewportLoad may benefit from listening to dom-complete, but I'm no longer sure it is important? (old fix I had in my queue)
• isDocumentCreation is to avoid reloading from BrowsingContextTargetActor.updateTargetConfiguration, when we apply the setting whe the target get created from DOMWindowCreated. i.e. when the document just start loading, we don't need and should not reload!
• setMetaViewportOverride and clearMetaViewportOverride are failing. We may call them on previous, already destroyed target... This should probably be migrated to become a target configuration.
• last but not least, but I'm not 100% sure it is mandatory. The restoreActorState/applyConfigurations change. We don't need to apply target configuration on each new target. The watcher should automatically reapply the config on the new top level target.

This seem to also fix browser_toolbox_rule_view_reload.js

Comment 2

[Alexandre Poirot [:ochameau]]

See also bug 1721398 which faces similar issue, but instead of having trouble with target creation, it has issues with target destruction.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1721379 - [devtools] Don't reload page when re-applying target configuration when the target just got created. r=ochameau.

This makes browser_touch_device.js and browser_toolbox_rule_view_reload.js pass
with server side target switching enabled.

Depends on D120456

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/759bc741488b
[devtools] Don't reload page when re-applying target configuration when the target just got created. r=ochameau.

Comment 5

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/759bc741488b