Bug 38867 – [Qt] Unskip fast/loader/main-document-url-for-non-http-loads.htm

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Robert Hogan]

Fix FrameLoadeClientQt.cpp to print relative filename paths correctly.

[Robert Hogan]

Created attachment 55606 [details]
Patch

[Kenneth Rohde Christiansen]

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

[Robert Hogan]

(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.

[Kenneth Rohde Christiansen]

(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?

[Robert Hogan]

(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.

[Robert Hogan]

Created attachment 56190 [details]
Patch

[Kenneth Rohde Christiansen]

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.

[Robert Hogan]

(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.

[Laszlo Gombos]

Created attachment 56419 [details]
remove some unused symbols

There was no QtWebKit release made with these symbols, it should be safe to remove them.

[WebKit Commit Bot]

Comment on attachment 56419 [details]
remove some unused symbols

Clearing flags on attachment: 56419

Committed r59836: <http://trac.webkit.org/changeset/59836>

[Shinichiro Hamaji]

I think this issue was fixed by http://trac.webkit.org/changeset/59580

Closing...