Summary: | [Qt] Unskip fast/loader/main-document-url-for-non-http-loads.htm | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robert Hogan <robert> | ||||||||
Component: | Tools / Tests | Assignee: | Robert Hogan <robert> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, hamaji, kenneth, laszlo.gombos | ||||||||
Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Robert Hogan
2010-05-10 14:29:43 PDT
Created attachment 55606 [details]
Patch
Comment on attachment 55606 [details]
Patch
WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:100
+ void QWEBKIT_EXPORT qt_dump_resource_load_callbacks_path(const QString& path)
That is not how we name private methods, plus shouldn't this be in the DRTSupport class you created?
WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:102
+ dumpResourceLoadCallbacksPath = path + "/";
Is this right on all platforms? Maybe add a comment
(In reply to comment #2) > (From update of attachment 55606 [details]) > WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:100 > + void QWEBKIT_EXPORT qt_dump_resource_load_callbacks_path(const QString& path) > That is not how we name private methods, plus shouldn't this be in the DRTSupport class you created? It's in the style of qt_dump_resource_load_callbacks() so let me know what it should be and I'll rename all of them as appropriate. FrameLoaderClientQt doesn't use the DRTSupport class at all, maybe it should. I'll look into it. > > WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:102 > + dumpResourceLoadCallbacksPath = path + "/"; > Is this right on all platforms? Maybe add a comment Good point, will fix/comment. (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 55606 [details] [details]) > > WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:100 > > + void QWEBKIT_EXPORT qt_dump_resource_load_callbacks_path(const QString& path) > > That is not how we name private methods, plus shouldn't this be in the DRTSupport class you created? > > It's in the style of qt_dump_resource_load_callbacks() so let me know what it should be and I'll rename all of them as appropriate. > > FrameLoaderClientQt doesn't use the DRTSupport class at all, maybe it should. I'll look into it. qt_dumpResourceLoadCallbacksPath(...) But isn't this for DRT usage only? (In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (From update of attachment 55606 [details] [details] [details]) > > > WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:100 > > > + void QWEBKIT_EXPORT qt_dump_resource_load_callbacks_path(const QString& path) > > > That is not how we name private methods, plus shouldn't this be in the DRTSupport class you created? > > > > It's in the style of qt_dump_resource_load_callbacks() so let me know what it should be and I'll rename all of them as appropriate. > > > > FrameLoaderClientQt doesn't use the DRTSupport class at all, maybe it should. I'll look into it. > > qt_dumpResourceLoadCallbacksPath(...) > > But isn't this for DRT usage only? Yes it is. All the resource callbacks in frameloaderclientqt sit in the DRT on other ports. I didn't move them into DRTSupport because I encountered a hitch I didn't know how to get around and can't remember what it was now. I'll try it again and if it's possible move them all over with this patch. Created attachment 56190 [details]
Patch
Comment on attachment 56190 [details]
Patch
I would probably have suggested you to do this as two separate patches, but just remember that next time.
Great change though :-) Thanks for doing this!
Before you commit, please talk to Laszlo to see if someone depends on these functions that you removed, though I guess not.
(In reply to comment #7) > (From update of attachment 56190 [details]) > I would probably have suggested you to do this as two separate patches, but just remember that next time. > > Great change though :-) Thanks for doing this! > > Before you commit, please talk to Laszlo to see if someone depends on these functions that you removed, though I guess not. I've retained the old style calls at the end of DumpRenderTreeSupportQt so it shouldn't discombobulate(!) anyone using the 'private' API in their clients. Created attachment 56419 [details]
remove some unused symbols
There was no QtWebKit release made with these symbols, it should be safe to remove them.
Comment on attachment 56419 [details] remove some unused symbols Clearing flags on attachment: 56419 Committed r59836: <http://trac.webkit.org/changeset/59836> I think this issue was fixed by http://trac.webkit.org/changeset/59580 Closing... |