Summary: | [GTK] TestInspectorServer unit test is timing out | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cgarcia, commit-queue, gustavo, mrobinson, obzhirov | ||||||
Priority: | P2 | Keywords: | Gtk | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Zan Dobersek
2012-12-30 01:44:45 PST
Going check this one. Probably started to fail after GTK inspector server has been re-factored to Soup inspector server. It does time out in debug mode - managed to reproduce it. Will debug tomorrow Well, found out the problem finally - webinspector resources are not installed in default folder. Going to fix it now. Created attachment 206654 [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 206654 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206654&action=review r- because we should move copying of files to the build system > Source/WebKit2/UIProcess/API/gtk/tests/InspectorTestServer.cpp:48 > + // Overwrite WEBKIT_INSPECTOR_SERVER_PATH variable to point to inspector resources folder. > + const gchar* inspectorResourcesPath = g_getenv("WEBKIT_INSPECTOR_PATH"); > + g_setenv("WEBKIT_INSPECTOR_SERVER_PATH", inspectorResourcesPath, TRUE); It makes sense to have this variable being set here, but when you say overwrite, do you mean the variable is already set but set to the wrong path? > Source/WebKit2/UIProcess/API/gtk/tests/InspectorTestServer.cpp:55 > + // Copy inspectorPageIndex.html to inspector resources folder > + GOwnPtr<char> sourcePath(g_build_filename(WEBKIT_SRC_DIR, "Source", "WebKit2", "UIProcess", "InspectorServer", "front-end", "inspectorPageIndex.html", NULL)); > + GRefPtr<GFile> source = adoptGRef(g_file_new_for_path(sourcePath.get())); > + GOwnPtr<char> destinationPath(g_build_filename(inspectorResourcesPath, "inspectorPageIndex.html", NULL)); > + GRefPtr<GFile> destination = adoptGRef(g_file_new_for_path(destinationPath.get())); > + g_file_copy(source.get(), destination.get(), G_FILE_COPY_OVERWRITE, NULL, NULL, NULL, NULL); We should do this in the build system like we do for that other bits, instead. (In reply to comment #6) > (From update of attachment 206654 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=206654&action=review > > r- because we should move copying of files to the build system > > > Source/WebKit2/UIProcess/API/gtk/tests/InspectorTestServer.cpp:48 > > + // Overwrite WEBKIT_INSPECTOR_SERVER_PATH variable to point to inspector resources folder. > > + const gchar* inspectorResourcesPath = g_getenv("WEBKIT_INSPECTOR_PATH"); > > + g_setenv("WEBKIT_INSPECTOR_SERVER_PATH", inspectorResourcesPath, TRUE); > > It makes sense to have this variable being set here, but when you say overwrite, do you mean the variable is already set but set to the wrong path? No, it shouldn’t be set. But just in case if there is "leftover" after another test. It feels safer for me to overwrite it. > > > Source/WebKit2/UIProcess/API/gtk/tests/InspectorTestServer.cpp:55 > > + // Copy inspectorPageIndex.html to inspector resources folder > > + GOwnPtr<char> sourcePath(g_build_filename(WEBKIT_SRC_DIR, "Source", "WebKit2", "UIProcess", "InspectorServer", "front-end", "inspectorPageIndex.html", NULL)); > > + GRefPtr<GFile> source = adoptGRef(g_file_new_for_path(sourcePath.get())); > > + GOwnPtr<char> destinationPath(g_build_filename(inspectorResourcesPath, "inspectorPageIndex.html", NULL)); > > + GRefPtr<GFile> destination = adoptGRef(g_file_new_for_path(destinationPath.get())); > > + g_file_copy(source.get(), destination.get(), G_FILE_COPY_OVERWRITE, NULL, NULL, NULL, NULL); > > We should do this in the build system like we do for that other bits, instead. Yep, I will update GNUmakefile.am for WebKit2 to copy the file. run-gtk-tests sets WEBKIT_INSPECTOR_PATH (In reply to comment #8) > run-gtk-tests sets WEBKIT_INSPECTOR_PATH Yes, and then I use it to set WEBKIT_INSPECTOR_SERVER_PATH inside the test. Or you mean I should set WEBKIT_INSPECTOR_SERVER_PATH in run-gtk-tests as well? (In reply to comment #9) > (In reply to comment #8) > > run-gtk-tests sets WEBKIT_INSPECTOR_PATH > Yes, and then I use it to set WEBKIT_INSPECTOR_SERVER_PATH inside the test. Or you mean I should set WEBKIT_INSPECTOR_SERVER_PATH in run-gtk-tests as well? Ah, SERVER_PATH, I see, sorry, I read too fast. I think it's fine to set it in the test then. Created attachment 206787 [details]
Patch
Comment on attachment 206787 [details]
Patch
OK
Comment on attachment 206787 [details] Patch Clearing flags on attachment: 206787 Committed r153085: <http://trac.webkit.org/changeset/153085> All reviewed patches have been landed. Closing bug. |