webdriver: Move NewWebView, FocusWebView, GetWindowSize, and SetWindowSize to servoshell#37555
Conversation
longvatrong111
changed the title
longvatrong111
force-pushed
the
webdriver-new-webview
branch
2 times, most recently
from
4a917e7 to
53ca995
Compare
longvatrong111
force-pushed
the
webdriver-new-webview
branch
from
53ca995 to
bf424ca
Compare
mrobinson
left a comment
mrobinson
left a comment
There was a problem hiding this comment.
Looks good. I just have a few minor suggestions:
mrobinson
changed the title
NewWebView, FocusWebView, GetWindowSize, and SetWindowSize to servoshell
longvatrong111
force-pushed
the
webdriver-new-webview
branch
from
c253ab5 to
6fef44c
Compare
Signed-off-by: batu_hoang <[email protected]>
Signed-off-by: batu_hoang <[email protected]>
Signed-off-by: batu_hoang <[email protected]>
Signed-off-by: batu_hoang <[email protected]>
Signed-off-by: batu_hoang <[email protected]>
|
Test result of #37669, which is a follow up to this PR: |
mrobinson
left a comment
mrobinson
left a comment
There was a problem hiding this comment.
Looks great! Just one request:
ports/servoshell/desktop/app.rs
Outdated
| } | ||
| }, | ||
| webdriver_msg @ WebDriverCommandMsg::GetViewportSize(..) => { | ||
| running_state.forward_webdriver_command(webdriver_msg); |
There was a problem hiding this comment.
You should be able to retrieve the size of the viewport here without forwarding the message. The viewport of a WebView is window.rendering_context().size2d(). I think it makes sense to implement this here without forwarding or just waiting to handle this in a followup.
There was a problem hiding this comment.
The more I know. It would be faster if we put this change in the follow up, there are some.
There was a problem hiding this comment.
Okay. Sounds good. For now, just don't move this message to the embedder yet. I can review the change very quickly.
Signed-off-by: batu_hoang <[email protected]>
longvatrong111
force-pushed
the
webdriver-new-webview
branch
from
6fef44c to
ed8ae58
Compare
3 participants
Follow up to: #36714
Moving webdriver context commands to be handled in embedder:
WebdriverCommandMsg::NewWebViewWebdriverCommandMsg::FocusWebViewcc: @xiaochengh