Add code for authentication from the DevTools iframe to enable access to web socket connections for VM Service and DTD #5680
Conversation
…o enable access to web socket connections for VM Service and DTD.
| protected get currentSession(): DartDebugSessionInformation | undefined { | ||
| // Get the first debug session if present. Should we get the current session instead? | ||
| // Just using the first one works with opening devTools, even when debugging a subsequent session. | ||
| return debugSessions && debugSessions.length ? debugSessions[0] : undefined; | ||
| } |
There was a problem hiding this comment.
I think we should be able to pass through the same VM Service URL that is going on the queryString so we don't have to worry about the possibility of getting the wrong one here. It might need a little refactoring though (so that where we currently just pass around a URL, we pass around something like a { viewUrl: string, vmServiceUrl?: string}). I will take look and if it works push changes for you to review.
|
I've pushed some changes. It ended up being more than I'd planned because once I made the set of auto-requiring URLs an array, I needed to create the iframes dynamically, but I think this will make it easier if we need to add more in future (I also expanded it to ensure we we support this for all of the different iframes, whether it's DevTools sidebar, DevTools editor tabs, Property Editor, etc.). I still need to do some proper testing on FS as so far my testing was all locally with the flags to trigger this hard-coded. |
|
/gemini review |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@keertip I've done some testing of this PR with my tweaks both on Firebase Studio and locally, and I think everything is working as expected. I was unable to trigger any errors on Firebase Studio. I have not yet tried removing the startup delay - I suspect we don't need it, but I think we should try to land this as-is first, and then do some testing of that separately. It would be good to also have you re-test since I think you were triggering the issue more than I was. If you don't want to build locally, there's a preview build from this PR branch here ( https://drive.google.com/drive/u/0/folders/1PM66L6kN_OVzS9RXgJ0ceWtr46gT_yob |
|
@DanTup , changes look good! Will retest it today to confirm that it works. |
|
Retested, and confirmed that it works as expected! Thanks for shepherding the change. |
…o enable access to web socket connections for VM Service and DTD.