Closed Bug 1641737 Opened 2 years ago Closed 2 years ago

Have ADocumentChannelBridge <-> DocumentLoadListener communicate via MozPromise

Categories

(Core :: Networking, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox79 --- fixed

People

(Reporter: jya, Assigned: jya)

References

Details

(Whiteboard: [necko-triaged])

Attachments

(7 files)

This is a change we discussed with :mattwoodrow today and will play very nicely in simplifying the communication between a DocumentChannel and a DocumentLoadListener.

Right now, we have a cycle between the DocumentChannel (DC) and a DocumentLoadListener (DLL) ; the DC sends info to the DLL such as when to start (via Open) ; when to finish or disconnect.

The DLL on the other hand, needs to also know when to disconnect, and when the redirection has been completed.

The cycle in place between the two has created a fair amount of complexity, some UAF error ; and we have issues where the DC would resolve the RedirectToRealChannel promise , and then disconnect the DLL ; but due to MozPromise implementation the task telling about the resolution gets run after the DLL has actually been disconnected.

So the idea is for the DocumentChannel to call DLL::Open which would return a promise ; this promise would be resolved/rejected once the DLL is done or when it needs for the redirect to the real channel started.

The DLL would no longer hold a reference to the ADocumentChannelBridge ; removing the cycle and simplifying on what to do when we're done or there's an error.

Severity: -- → N/A
Priority: -- → P3
Whiteboard: [necko-triaged]

When chaining a MozPromise set to be dispatched via the direct task queue (or synchronous), it makes sense for the chained promise to be dispatched in the same fashion.

All MozPromises generated by the IPC bindings are set to use the direct task queue in order to prevent the then runnable to run out of order with other IPC tasks.
We want to preserve that task ordering by default.

The allows to not have to deal with MozPromise::Private directly.

We would have ended up attempting to tell the child that it just died.

The earlier changes make this unnecessary. The load will start and will automatically resume once the PDocumentChannelParent::RedirectToRealChannel promise gets resolved when a DocumentChannel claim the DocumentLoadListener.

We attempt to reduce the number of event loop iterations, which would bring us back to a similar behaviour to the pre-DocumentChannel days.

This prevent some tests to have increase intermittent failures.

While ideally, we would prefer to fix or re-write those tests properly, the extra work would block us for too long.

All those increased intermittents already have bug logged about them; so we aren't hiding dust under the rust.

Depends on D78488

Attachment #9154053 - Attachment description: Bug 1641737 - P4. Add UseDirectTaskDispatch/UseSynchronousDispatch to MozPromiseHolder. r?bholley → Bug 1641737 - P3. Add UseDirectTaskDispatch/UseSynchronousDispatch to MozPromiseHolder. r?bholley
Attachment #9154056 - Attachment description: Bug 1641737 - P5. Remove unnecessary method. r?mattwoodrow → Bug 1641737 - P4. Remove unnecessary method. r?mattwoodrow
Attachment #9154532 - Attachment description: Bug 1641737 - P6. Remove the need for an extra promise for when a DocumentChannel would connect. r?mattwoodrow → Bug 1641737 - P5. Remove the need for an extra promise for when a DocumentChannel would connect. r?mattwoodrow
Attachment #9154533 - Attachment description: Bug 1641737 - P7. Make the DLL and PPDC promises use direct task dispatch. r?mattwoodrow → Bug 1641737 - P6. Make the DLL and PPDC promises use direct task dispatch. r?mattwoodrow
Attachment #9154053 - Attachment description: Bug 1641737 - P3. Add UseDirectTaskDispatch/UseSynchronousDispatch to MozPromiseHolder. r?bholley → Bug 1641737 - P4. Add UseDirectTaskDispatch/UseSynchronousDispatch to MozPromiseHolder. r?bholley
Attachment #9154056 - Attachment description: Bug 1641737 - P4. Remove unnecessary method. r?mattwoodrow → Bug 1641737 - P5. Remove unnecessary method. r?mattwoodrow
Attachment #9154532 - Attachment description: Bug 1641737 - P5. Remove the need for an extra promise for when a DocumentChannel would connect. r?mattwoodrow → Bug 1641737 - P6. Remove the need for an extra promise for when a DocumentChannel would connect. r?mattwoodrow
Attachment #9154533 - Attachment description: Bug 1641737 - P6. Make the DLL and PPDC promises use direct task dispatch. r?mattwoodrow → Bug 1641737 - P7. Make the DLL and PPDC promises use direct task dispatch. r?mattwoodrow
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0296d0f6c1d1
P1. Set dispatch type to chained promises. r=bholley
https://hg.mozilla.org/integration/autoland/rev/28bca5c5b8b4
P2. Use MozPromise with DocumentLoadListener and remove cycle. r=mattwoodrow.
https://hg.mozilla.org/integration/autoland/rev/94d47578009c
P3. Streamline cleanup so it's all done in the same place. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/e3bf9a45db6a
P4. Add UseDirectTaskDispatch/UseSynchronousDispatch to MozPromiseHolder. r=bholley
https://hg.mozilla.org/integration/autoland/rev/abd9cd77f3cb
P5. Remove unnecessary method. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/32fba417ebd0
P6. Remove the need for an extra promise for when a DocumentChannel would connect. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/7f7b98339065
P7. Make the DLL and PPDC promises use direct task dispatch. r=mattwoodrow

so we are now blocked on bug 1644009, which will remove the dependency on AbstractThread to use direct task dispatching.

Depends on: 1644009
Flags: needinfo?(jyavenard)
Attachment #9154018 - Attachment description: Bug 1641737 - P1. Set dispatch type to chained promises. r?bholley → Bug 1641737 - P1. Set dispatch type to chained promises. r=bholley
Attachment #9154019 - Attachment description: Bug 1641737 - P2. Use MozPromise with DocumentLoadListener and remove cycle. r?mattwoodrow. → Bug 1641737 - P2. Use MozPromise with DocumentLoadListener and remove cycle. r=mattwoodrow.
Attachment #9154020 - Attachment description: Bug 1641737 - P3. Streamline cleanup so it's all done in the same place. r?mattwoodrow → Bug 1641737 - P3. Streamline cleanup so it's all done in the same place. r=mattwoodrow
Attachment #9154053 - Attachment description: Bug 1641737 - P4. Add UseDirectTaskDispatch/UseSynchronousDispatch to MozPromiseHolder. r?bholley → Bug 1641737 - P4. Add UseDirectTaskDispatch/UseSynchronousDispatch to MozPromiseHolder. r=bholley
Attachment #9154056 - Attachment description: Bug 1641737 - P5. Remove unnecessary method. r?mattwoodrow → Bug 1641737 - P5. Remove unnecessary method. r=mattwoodrow
Attachment #9154532 - Attachment description: Bug 1641737 - P6. Remove the need for an extra promise for when a DocumentChannel would connect. r?mattwoodrow → Bug 1641737 - P6. Remove the need for an extra promise for when a DocumentChannel would connect. r=mattwoodrow
Attachment #9154533 - Attachment description: Bug 1641737 - P7. Make the DLL and PPDC promises use direct task dispatch. r?mattwoodrow → Bug 1641737 - P7. Make the DLL and PPDC promises use direct task dispatch. r=mattwoodrow
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/02f45f660107
P1. Set dispatch type to chained promises. r=bholley
https://hg.mozilla.org/integration/autoland/rev/81ddcb21f057
P2. Use MozPromise with DocumentLoadListener and remove cycle. r=mattwoodrow.
https://hg.mozilla.org/integration/autoland/rev/f1364c78caa7
P3. Streamline cleanup so it's all done in the same place. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/f6c685e4b841
P4. Add UseDirectTaskDispatch/UseSynchronousDispatch to MozPromiseHolder. r=bholley
https://hg.mozilla.org/integration/autoland/rev/3998aeb0935c
P5. Remove unnecessary method. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/bcbaea8b572e
P6. Remove the need for an extra promise for when a DocumentChannel would connect. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/3922996e51fa
P7. Make the DLL and PPDC promises use direct task dispatch. r=mattwoodrow
Regressions: 1656039
You need to log in before you can comment on or make changes to this bug.