Summary: | [GTK] Reduce IPC traffic due to view state changes | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | berto, bugs-noreply, commit-queue, gustavo, mcatanzaro, mrobinson | ||||
Priority: | P2 | Keywords: | Gtk | ||||
Version: | WebKit Local Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Carlos Garcia Campos
2016-02-01 09:13:35 PST
Created attachment 270390 [details]
Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API Comment on attachment 270390 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270390&action=review Nice patch, although I think it doesn't really work as is (see my comment about the mask) > Source/WebKit2/ChangeLog:27 > + WebPageProxy::viewStateDidChange() and resret the flags that need resret -> reset > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:316 > + g_signal_connect(priv->toplevelOnScreenWindow, "window-state-event", G_CALLBACK(toplevelWindowStateEvent), webViewBase); According to the docs you should enable GDK_STRUCTURE_MASK something that you are not doing. You should also remove the GDK_VISIBILITY_NOTIFY_MASK. > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:1385 > + unsigned flagsToUpdate = ViewState::IsFocused; We could use ViewState::Flags here for consistency with the rest of the code (even though it's an unsigned anyway). (In reply to comment #3) > Comment on attachment 270390 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=270390&action=review > > Nice patch, although I think it doesn't really work as is (see my comment > about the mask) Thanks for the review! > > Source/WebKit2/ChangeLog:27 > > + WebPageProxy::viewStateDidChange() and resret the flags that need > > resret -> reset Oops. > > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:316 > > + g_signal_connect(priv->toplevelOnScreenWindow, "window-state-event", G_CALLBACK(toplevelWindowStateEvent), webViewBase); > > According to the docs you should enable GDK_STRUCTURE_MASK something that > you are not doing. You should also remove the GDK_VISIBILITY_NOTIFY_MASK. But then it says "GDK will enable this mask automatically for all new windows." Also note that this is not a signal of the web view, but the toplevel window, and GtkWindos adds the GDK_STRUCTURE_MASK unconditionally in realize method. > > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:1385 > > + unsigned flagsToUpdate = ViewState::IsFocused; > > We could use ViewState::Flags here for consistency with the rest of the code > (even though it's an unsigned anyway). OK. Committed r196062: <http://trac.webkit.org/changeset/196062> |