Summary: | Share FrameLoaderClient::transitionToCommittedForNewPage between Win/Gtk+/Qt | ||
---|---|---|---|
Product: | WebKit | Reporter: | Holger Freyther <zecke> |
Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | Keywords: | Gtk, Qt |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | PC | ||
OS: | OS X 10.5 | ||
Attachments: |
Description
Holger Freyther
2008-11-03 14:47:19 PST
Created attachment 24868 [details]
Define the Color::transparent in the cpp file to fix link errors
Link fix required to use Color::transparent from within WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp. Some how my gcc only inlines that or such, any comment to find the real cause to resolve my link error would be welcome.
Created attachment 24869 [details]
Move recursively setting a background color and toggling transparency to the FrameView
Move the code from WebFrame to WebCore::FrameView to be able to use from the Gtk+ and the Qt build.
Created attachment 24870 [details]
Share FrameLoaderClient::transitionToCommittedForNewPage between Win/Gtk+/Qt
Create a FrameLoaderClient.cpp and provide a helper function to implement transitionToCommittedForNewPage in a cross port way. The mac port can not be switched to this implementation but this copy reduction is helping both the Qt and Gtk+ port.
Comment on attachment 24868 [details]
Define the Color::transparent in the cpp file to fix link errors
This change isn't right.
We do need to define these in the .cpp file; it was an error to have definitions in the header only.
But the values should stay in the .h file. This is how C++ static const data members work. I'll land a corrected version.
(In reply to comment #1) > Define the Color::transparent in the cpp file to fix link errors Landed in http://trac.webkit.org/changeset/38242 Comment on attachment 24868 [details]
Define the Color::transparent in the cpp file to fix link errors
Clearing review as Darin landed a patch which is using static const as of the various C++ FAQs.
The Share FrameLoaderClient::tra... patch has two build failures on Qt. One in qwebpage.cpp (calling a removed method) one in FrameLoaderClientQt.cpp (not declaring a variable name). I would fix it before landing the patch, the spirit/substance is still the same. Comment on attachment 24869 [details] Move recursively setting a background color and toggling transparency to the FrameView > +++ b/WebCore/page/FrameView.h > @@ -101,6 +101,7 @@ public: > > Color baseBackgroundColor() const; > void setBaseBackgroundColor(Color); > + void updateBackgroundRecursively(const Color&, bool); My feeling is that the second argument should have a name. But that's a minor detail. Otherwise the patch looks good. Comment on attachment 24870 [details] Share FrameLoaderClient::transitionToCommittedForNewPage between Win/Gtk+/Qt Nice cleanup! > + FrameView* frameView; > + if (isMainFrame) { > + frameView = new FrameView(frame, size); > + } else Minor coding style issue with the braces there. r+ and please fix the braces when landing :) I think it's a little sad that we now have code (even if a static function) in an interface class, which FrameLoaderClient is. Comment on attachment 24869 [details]
Move recursively setting a background color and toggling transparency to the FrameView
Clear review, it got landed.
Comment on attachment 24870 [details]
Share FrameLoaderClient::transitionToCommittedForNewPage between Win/Gtk+/Qt
Clear review.
Created attachment 25236 [details]
Kill FrameLoaderClient.cpp and move the code to Frame
As suggested by Maciej, move the code to Frame. I picked the name createView.
Landed in r40435. |