Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions ios/DocuSignManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,16 @@ internal final class DocuSignManager: NSObject {
}
_ = pendingResolved // silence unused-warning; kept for future telemetry

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code in the function you are already touching. var pendingResolved on 529 is assigned and never read, and this line exists only to silence the warning. Drop both.


// DSMManager APIs (clearAllWebCookies, logout) must run on the main thread.
// Expo async functions are dispatched on AsyncFunctionQueue (non-main), so
// we must hop to main before touching any DSMManager API.
guard Thread.isMainThread else {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guard sits after the state mutation, so the stateQueue.sync block above runs twice: once on the caller's thread, once on the main-queue re-entry. If a presentCaptiveSigning lands in between, the second pass cancels that fresh session.

Hop to main at the top of the function. Better still, put the guard inside clearWebCookiesAsync. That is the only place touching DSMManager.clearAllWebCookies() and WKWebsiteDataStore off-main, so one guard covers every caller now and later. DSMManager.logout() below was already safe: it runs from clearWebCookiesAsync's completion, which is dispatched on main.

DispatchQueue.main.async { [weak self] in
self?.endSigningSession(completion: completion)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

completion is dropped when self is nil and the JS promise never settles. The equivalent hop in reset() handles it: guard let self = self else { completion(); return }. Latent today because the manager is a singleton, but let's keep the two paths consistent.

}
return
}

clearWebCookiesAsync { [weak self] in
guard let self = self else {
completion()
Expand Down Expand Up @@ -571,8 +581,10 @@ internal final class DocuSignManager: NSObject {
/// Safe to call when the SDK was never initialized: returns immediately
/// without touching DSMManager.
func reset(completion: @escaping () -> Void) {
var hadPending = false
stateQueue.sync {
if let pending = pendingCompletion {
hadPending = true
let outcome = SigningOutcome(
status: "cancelled",
envelopeId: currentEnvelopeId ?? "",
Expand All @@ -585,6 +597,23 @@ internal final class DocuSignManager: NSObject {
}
}

// Force-dismiss the captive signing modal if one is on screen. reset() is
// called on session teardown (e.g. END_CONSULTATION) which can arrive while
// the DocuSign WebView is still presented; the SDK only dismisses its own UI
// on user Finish/Cancel, so we dismiss the presented modal here. Gated on
// hadPending so an unrelated modal is never torn down.
if hadPending {
DispatchQueue.main.async {
let root = UIApplication.shared.connectedScenes
.compactMap { $0 as? UIWindowScene }
.flatMap { $0.windows }
.first(where: { $0.isKeyWindow })?.rootViewController
if root?.presentedViewController != nil {
root?.dismiss(animated: true)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dismissal is not sequenced with the teardown or with the promise.

There is no completion handler here, so reset() runs straight into clearWebCookiesAsync. That wipes the whole WKWebsiteDataStore while the DocuSign WKWebView is still on screen mid-animation, and the JS promise resolves before the modal is gone. Call reset() and then immediately start a new signing: topmostViewController() returns the dismissing controller, and you get "already presenting" or a presentation on a detached VC. Sequence it as dismiss, then wipe, then resolve.

Small thing on the same lines: if let root, root.presentedViewController != nil avoids optional-chaining root twice.

}
}
}
Comment on lines +605 to +615

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dismisses every modal above the root view controller, including ones the host app owns.

presentCaptiveSigning and presentCaptiveSigningWithUrl both present from Self.topmostViewController(), which walks the presentation chain. If the host app already has a modal on screen (signing started from a sheet, or from a modally presented screen), the DocuSign controller is presented by that modal. Dismissing the key window's root controller then tears down the app's modal along with DocuSign's. The hadPending gate does not prevent that, contrary to the comment above it.

The SDK hands you the controller you actually want. Both presentCaptiveSigning completion closures receive it and discard it: { [weak self] (_: UIViewController?, error: Error?) in. Store it in a private weak var presentedSigningVC: UIViewController?, dismiss through presentedSigningVC?.presentingViewController?.dismiss(animated:completion:), and clear it in resolvePending. A weak ref is nil when nothing is presented, so hadPending drops out, and so does the key window walk on 607-610, which duplicates the default argument of topmostViewController().


let needsTeardown = stateQueue.sync { _isInitialized || _hasLoggedIn }
guard needsTeardown else {
completion()
Expand Down