Skip to content

[WPE] WPE Platform: add window minimization to WPEToplevel#32021

Closed
siwei-li wants to merge 1 commit intoWebKit:mainfrom
siwei-li:eng/WPE-WPE-Platform-add-window-minimization-to-WPEToplevel
Closed

[WPE] WPE Platform: add window minimization to WPEToplevel#32021
siwei-li wants to merge 1 commit intoWebKit:mainfrom
siwei-li:eng/WPE-WPE-Platform-add-window-minimization-to-WPEToplevel

Conversation

@siwei-li
Copy link
Contributor

@siwei-li siwei-li commented Aug 12, 2024

0acefe3

[WPE] WPE Platform: add window minimization to WPEToplevel
https://bugs.webkit.org/show_bug.cgi?id=276905

Reviewed by NOBODY (OOPS!).

Add toplevel minimization support to the webdriver. Wayland compositor
does not notify when the minimization actually happens, hence we only ask
the toplevel to be minimized.

* Source/WebKit/UIProcess/API/wpe/WebKitWebViewWPE.cpp:
(WindowStateEvent::WindowStateEvent):
(WindowStateEvent::~WindowStateEvent):
(WindowStateEvent::complete):
(toplevelStateChangedCallback):
(webkitWebViewMonitorWindowState):
(webkitWebViewMaximizeWindow):
(webkitWebViewMinimizeWindow):
(webkitWebViewRestoreWindow):

0acefe3

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@siwei-li siwei-li requested review from a team and zdobersek as code owners August 12, 2024 02:05
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 12, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static void toplevelStateChangedCallback(WebKitWebView* view, WPEToplevelState previousState)
static void toplevelStateChangedCallback(WPEView* view, WPEToplevelState previousState)

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this, the view we receive here is the WPEView

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
webkitWebViewMonitorWindowState(WebKitWebView* view, WindowStateEvent::Type type, CompletionHandler<void()>&& completionHandler)
monitorToplevelState(WPEView* view, TopevelStateEvent::Type type, CompletionHandler<void()>&& completionHandler)

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static const char* windowStateEventID = "wpe-toplevel-state-event";
static const char* toplevelStateEventID = "wpe-toplevel-state-event";

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
struct WindowStateEvent {
struct ToplevelStateEvent {

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use Toplevel instead of Window everywhere, since it better matches the WPE API name

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
g_signal_connect_object(view, "toplevel-state-changed", G_CALLBACK(toplevelStateChangedCallback), nullptr, G_CONNECT_AFTER);
g_signal_connect_after(view, "toplevel-state-changed", G_CALLBACK(toplevelStateChangedCallback), nullptr);

Comment on lines 137 to 140
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!wpe_toplevel_maximize(toplevel)) {
completionHandler();
return;
}
if (wpe_toplevel_maximize(toplevel)) {
return;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

And then remove this return.

Comment on lines 159 to 164
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Comment on lines 183 to 185
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (wpe_toplevel_get_state(toplevel) & WPE_TOPLEVEL_STATE_MAXIMIZED)
wpe_toplevel_unmaximize(toplevel);
return;
if ((wpe_toplevel_get_state(toplevel) & WPE_TOPLEVEL_STATE_MAXIMIZED) && wpe_toplevel_unmaximize(toplevel))
return;

https://bugs.webkit.org/show_bug.cgi?id=276905

Reviewed by NOBODY (OOPS!).

Add toplevel minimization support to the webdriver. Wayland compositor
does not notify when the minimization actually happens, hence we only ask
the toplevel to be minimized.

* Source/WebKit/UIProcess/API/wpe/WebKitWebViewWPE.cpp:
(WindowStateEvent::WindowStateEvent):
(WindowStateEvent::~WindowStateEvent):
(WindowStateEvent::complete):
(toplevelStateChangedCallback):
(webkitWebViewMonitorWindowState):
(webkitWebViewMaximizeWindow):
(webkitWebViewMinimizeWindow):
(webkitWebViewRestoreWindow):
@siwei-li siwei-li force-pushed the eng/WPE-WPE-Platform-add-window-minimization-to-WPEToplevel branch from 5e652a4 to 0acefe3 Compare August 12, 2024 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merging-blocked Applied to prevent a change from being merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants