Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Do not try to set the default URL using the remote debugger on iOS 17+ #2447

Closed
wants to merge 1 commit into from

Conversation

mykola-mokhnach
Copy link
Contributor

@mykola-mokhnach
Copy link
Contributor Author

@KazuCocoa Could you please try this on a real device?

@KazuCocoa
Copy link
Member

Hm, it looks like... opening a url via deeplink failed to load the content frequently rather than the existing one... After this deeplink behavior, commands like driver.title got stuck. I needed to kill the appium server or wait for the no Sending '_rpc_forwardSocketData:' message to app 'PID:790', page '1', target 'page-25' (id: 32): 'Runtime.evaluate' response error.

@KazuCocoa
Copy link
Member

KazuCocoa commented Aug 6, 2024

failed to load the content

This meant the content load bar in Safari didn't progress less than half. This occurred while the default page in appium.

@KazuCocoa
Copy link
Member

KazuCocoa commented Aug 6, 2024

mm, I think we should revert https://github.com/appium/WebDriverAgent/pull/929/files#diff-7debbf48ac1513ec716fc7d72a09c224118ac2d4835e8f3d0517d457ee39c9acR180-R191 as well. The default page also had the same behavior.

When I started a safari session with this and the WDA change (8.8.0) 10 times, this behavior occurred over half while 10 times had no issue with today's implementation with WDA 8.7.0

I also noticed Safari's open url behavior after deeplink looked weird. After the deeplink start and open a URL via driver.get "url", the url didn't open while the response via the remote control responded page open completed.

@mykola-mokhnach
Copy link
Contributor Author

@KazuCocoa Could you please add the failed logs?
I did not observe such behavior on Simulator

@mykola-mokhnach
Copy link
Contributor Author

Might it also be a race condition? What if we add idle check delay (fb_waitUntilStableWithTimeout) after we load the default URL in WDA using deeplink?

@KazuCocoa
Copy link
Member

Not helpful logs to see the behavior in the log level, but https://gist.github.com/KazuCocoa/37e01e713f7e51f4c257ff800b275fe0 is no response so long against /title example. This is with WDA 8.8.0 and master's case, actually not this PR itself but WDA's via deeplink change. (Other logs are already gone from my local terminal right now.) The log's initial page, https://google.com, didn't complete the page load. The safari's content load bar stopped by less than half.

@KazuCocoa
Copy link
Member

KazuCocoa commented Aug 6, 2024

Might it also be a race condition? What if we add idle check delay (fb_waitUntilStableWithTimeout) after we load the default URL in WDA using deeplink?

Hm... I couldn't say 100% sure thing but what I observed was below progress bar (blue color's one) itself had no progress so long. I needed the device restart to fix the weird behavior. Safari process kill didn't help some times.

Screenshot 2024-08-06 at 1 32 40 AM

(This image is from simulator though)

@KazuCocoa
Copy link
Member

KazuCocoa commented Aug 6, 2024

Potentially... after the deeplink command, do we need the waiting page stuff like https://github.com/appium/appium-remote-debugger/blob/414f1b56f9e9c68fab476a6d7ea2c191bc731ccf/lib/mixins/navigate.js#L148 ...?

I guess... the deeplink command itself does not help to wait for the page content load, so it just launches Safari with URL, that's it.

@KazuCocoa
Copy link
Member

KazuCocoa commented Aug 6, 2024

If it worked well on the Simulators (I haven't tested well on simulators)... then (not ideal, of course, but) we might need to have separate methods for simulators and real devices until real devices get stable implementation in XCTest...? Just a note..

@mykola-mokhnach
Copy link
Contributor Author

@KazuCocoa Could you please try the same with the patch at appium/WebDriverAgent#930 ?

@KazuCocoa
Copy link
Member

Sure, I'll try it out/debug it tonight

@KazuCocoa
Copy link
Member

KazuCocoa commented Aug 7, 2024

Hm, while I set "appium:safariInitialUrl": "https://google.com" in the caps, a new session request didn't open the URL.

Then, the session went to https://github.com/appium/appium-xcuitest-driver/pull/2447/files#diff-ed3c013edbc896bd87ef62539626fcdc8930a31f3cee7ecb37438e172af2c8e0R676-R680 as below logs, but the browser kept default health check page. It looks like this unexpected behavior (the safariInitialUrl did not open) was by https://github.com/appium/WebDriverAgent/pull/929/files#diff-7debbf48ac1513ec716fc7d72a09c224118ac2d4835e8f3d0517d457ee39c9acR180-R191 . The same behavior occurred without this PR but with WDA 8.8.0.

[RemoteDebugger] Document readyState is 'complete'. The pageLoadStrategy is 'normal'
[RemoteDebugger] Selected page after 17ms
[RemoteDebugger] Unregistering from page readiness notifications
[XCUITestDriver@c417] About to set the initial Safari URL to 'https://google.com'.Use 'safariInitialUrl' capability in order to customize it
[XCUITestDriver@c417] Attempting to set url 'https://google.com'
[RemoteDebugger] Navigating to new URL: 'https://google.com'
[RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:1309', page '1', target 'page-25' (id: 30): 'Page.navigate'
[RemoteDebugger] Received data response from send (id: 30): '{}'
[RemoteDebugger] Sending to Web Inspector took 5ms
[RemoteDebugger] Pages changed for PID:1309: [{"id":1,"title":"Google","url":"https://www.google.com/","isKey":false},{"id":2,"title":"","url":"safari-web-extension://82A492C7-2582-4763-ADAF-57616AE93E1C/background/background.html","isKey":false},{"id":4,"title":"Health Check","url":"http://127.0.0.1:8100/health","isKey":false}] -> [{"id":1,"title":"Google","url":"https://www.google.com/","isKey":true},{"id":2,"title":"","url":"safari-web-extension://82A492C7-2582-4763-ADAF-57616AE93E1C/background/background.html","isKey":false},{"id":4,"title":"Health Check","url":"http://127.0.0.1:8100/health","isKey":false}]
[RemoteDebugger] Notified that an application has been updated
[RemoteDebugger] Received page change notice for app 'PID:1309' but the listing has not changed. Ignoring.
[RemoteDebugger] Timed out after 6000ms of waiting for the https://google.com page readiness. Continuing anyway
[RemoteDebugger] Sending '_rpc_forwardSocketData:' message to app 'PID:1309', page '1', target 'page-25' (id: 32): 'Console.enable'
[RemoteDebugger] Sending to Web Inspector took 2ms
[BaseDriver] The value of 'elementResponseAttributes' setting did not change. Skipping the update for it
[BaseDriver] The value of 'shouldUseCompactResponses' setting did not change. Skipping the update for it
[AppiumDriver@1d0a] New XCUITestDriver session created successfully, session b1a3b413-3e08-448b-8a59-b8ebf4b4c979 added to master session list
[AppiumDriver@1d0a] Event 'newSessionStarted' logged at 1723012065885 (23:27:45 GMT-0700 (Pacific Daylight Time))
[XCUITestDriver@c417] Cached the protocol value 'W3C' for the new session b1a3b413-3e08-448b-8a59-b8ebf4b4c979
[XCUITestDriver@c417] Responding to client with driver.createSession() result: {"capabilities":{"webStorageEnabled":false,"locationContextEnabled":false,"browserName":"safari","platform":"MAC","javascriptEnabled":true,"databaseEnabled":false,"takesScreenshot":true,"networkConnectionEnabled":false,"platformName":"ios","automationName":"xcuitest","deviceName":"iPhone 15 Plus","platformVersion":"17.4","webDriverAgentUrl":"http://192.168.5.8:8100","udid":"00008020-000E5CDA0A23002E","safariInitialUrl":"https://google.com"}}
[HTTP] <-- POST /session 200 11183 ms - 506

I also noticed the deep link method in WDA 8.8.0 opened Safari's new tab when the Safari process ended? and launched Safari with a new page? while existing method kept using the focused page every time basically.
It looks like... this "open a new tab" behavior by the deeplink behavior caused #2447 (comment) behavior and non safariInitialUrl page load behavior.

@KazuCocoa
Copy link
Member

appium/WebDriverAgent#930 itself did not change the behavior, btw.

@KazuCocoa
Copy link
Member

KazuCocoa commented Aug 7, 2024

So, starting a Safari session without any existing tabs worked well with WDA 8.8.0 and this PR (deep link method) on my local, but if Safari already has open pages, the weird behavior I met before could occur frequently at least. The number of new tabs also could keep increasing with the deeplink method as well. This weird behavior is probably because current selected page via remote-debugger is different from the newly opened tab via the deeplink behavior in WDA 8.8.0. (Current guess)

For example, when I start a new safari session with a fresh safari and end it, then start another session without removing the existing safari tab, this behavior could occur. This scenario is a common case of repeating automation sessions against one real device. We have no methods to close all existing tabs without UI interactions in a real device, so this increasing tabs behavior also could be stressful for users who run multiple automation sessions.

Maybe... we need to find a way to delete tabs without UI interactions and tweak page selection method for this behavior before making the deeplink method replacement default.

For the page selection, I haven't tried but I guess switching context to WEBVIEW after opening a url via deeplink and waiting a bit while would help....? Like below steps would help to improve this weird behavior after the deeplink. Maybe the context selection needed to be improved?

d.execute_script "mobile: deepLink", {"bundleId": "com.apple.mobilesafari", "url": "https://google.com"}
d.execute_script "mobile: getContexts"
d.set_context 'context of the url'
d.title # etc worked without this weird behavior

However, increasing a new tab here would not be nice compared to the existing behavior with WDA 8.7.0 in terms of running multiple automation sessions repeatedly though.

@mykola-mokhnach
Copy link
Contributor Author

Many thanks for checking it @KazuCocoa
I am going to revert the change for WDA and apply a different workaround to the original issue.

@mykola-mokhnach mykola-mokhnach deleted the bump_wda branch August 8, 2024 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants