Summary: | [gtk] Implement FrameLoaderClientGdk::createFrame | ||
---|---|---|---|
Product: | WebKit | Reporter: | Holger Freyther <zecke> |
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | aroben, christian, hyatt |
Priority: | P2 | Keywords: | Gtk |
Version: | 523.x (Safari 3) | ||
Hardware: | Mac | ||
OS: | OS X 10.4 | ||
Bug Depends on: | 14857 | ||
Bug Blocks: | 14725 | ||
Attachments: |
Description
Holger Freyther
2007-07-23 11:53:42 PDT
Add the Gtk keyword, even to the curl bugs. Created attachment 15731 [details]
Implement ScrollView similiar to win/qt
Mostly follow the Windows design (which works great):
The most controversal change is with Widget::setScrollViewControl. PlatformScrollbar uses a GtkWidget. In order to draw it correctly we need to have a proper GtkAllocation set on the widget. We set one when PlatformScrollbar::setRect gets called.
Now PlatformScrollbar is used by RenderFrame and ScrollView. In the case of ScrollView scrolling should not change the position of the ScrollBar. In Win/Qt this is achieved by translating the context before painting. As the PlatformScrollbar in Gtk+ is a GtkWidget I would like to avoid changing the GtkAllocation on every paint and treat it more like WebCore/plugins/win and handle it like/similiar geometryChanged to position it.
This leads to the controversial change in Widget to select which coordinate translation to use. Other solutions considered where to subclass PlatformScrollbar for ScrollView to change the placing behaviour.
This is just a current diff of the work in progress implementation and comments would be appreciated.
CCing Hyatt so he can look at the PlatformScrollBar changes Created attachment 15748 [details]
Implement ScrollView similiar to win/qt
It has the same controversal change as the previous attachment. This patch changes the following things:
-It should apply against ToT
-It handles getting embedded into a GtkScrolledWindow nicely by using GtkAdjustment
Known issues:
-On destructing it seems I get ref counting of the PlatformScrollbar wrong.
-We might create and destroy PlatformScrollbar too often (same as on Windows and not introduced by this patch)
Created attachment 15754 [details]
Implement ScrollView similiar to win/qt
Compared to the previous patch:
-Make it compile on case-sensitive filesystems (PlatformScrollbar.h vs. PlatformScrollBar.h)
-Add (again) refcounting to PlatformScrollbar and ScrollView
-Implement forall and unrealize in WebKitGtkPage to make issues on exit go away.
-Fixup some comments.
The biggest part of the patch is copy and paste from the windows port. The controversal change is in Widget to use one of the two possible IntRect conversion methods. I think this change is fine, other approaches would be to move this code to PlatformScrollbar or to subclass PlatformScrollbar and use that subclss in ScrollView.
I think this is commit-worthy.
Created attachment 15755 [details]
Implement ScrollView similiar to win/qt
Compared to the previous patch:
-Make it compile on case-sensitive filesystems (PlatformScrollbar.h vs. PlatformScrollBar.h)
-Add (again) refcounting to PlatformScrollbar and ScrollView
-Implement forall and unrealize in WebKitGtkPage to make issues on exit go away.
-Fixup some comments.
The biggest part of the patch is copy and paste from the windows port. The controversal change is in Widget to use one of the two possible IntRect conversion methods. I think this change is fine, other approaches would be to move this code to PlatformScrollbar or to subclass PlatformScrollbar and use that subclss in ScrollView.
I think this is commit-worthy.
Created attachment 15756 [details]
Implement ScrollView similiar to win/qt
Compared to the previous patch:
-Make it compile on case-sensitive filesystems (PlatformScrollbar.h vs. PlatformScrollBar.h)
-Add (again) refcounting to PlatformScrollbar and ScrollView
-Implement forall and unrealize in WebKitGtkPage to make issues on exit go away.
-Fixup some comments.
The biggest part of the patch is copy and paste from the windows port. The controversal change is in Widget to use one of the two possible IntRect conversion methods. I think this change is fine, other approaches would be to move this code to PlatformScrollbar or to subclass PlatformScrollbar and use that subclss in ScrollView.
I think this is commit-worthy.
Comment on attachment 15754 [details]
Implement ScrollView similiar to win/qt
Duplicate attachment.
Comment on attachment 15755 [details]
Implement ScrollView similiar to win/qt
Duplicate attachment
I have one report that the Xserver hogs memory when scrolling. But I have no information beyond this. Created attachment 15784 [details]
Implement ScrollView similiar to win/qt to support createFrame
Changes due the Xserver leakage report:
The z-order painting patch introduced thig bug. The gdk_window_end_painting call was removed. As Xan pointed out in another bug report this window is double buffered any so we can remove the begin_paint call as well.
Interdiff:
+ gdk_region_get_clipbox(event->region, &clip);
-+ gdk_window_begin_paint_region(event->window, event->region);
I tested the patch 15755 which caused the X server to eat memory when scrolling. I can also confirm that patch 15784 is fine now. Comment on attachment 15784 [details] Implement ScrollView similiar to win/qt to support createFrame Clearing review flag as the patches in bug 14857 should be reviewed instead. Thanks to the reviewing effort of Niko and especially Adam this method and everything needed to implement it is in ToT. There is one known issue/race with GtkLayout and the positioning of ScrollViewScrollbar which will be addressed in another patch. |