Implement ewk_context_prefetch_dns() API using Injected Bundle.
Created attachment 255632 [details] Patch
Comment on attachment 255632 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255632&action=review > Source/WebKit2/UIProcess/API/efl/ewk_context.h:532 > +EAPI void ewk_context_prefetch_dns(Ewk_Context *context, const char *hostname); Where is a test for this new API ?
Comment on attachment 255632 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255632&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_context.h:532 >> +EAPI void ewk_context_prefetch_dns(Ewk_Context *context, const char *hostname); > > Where is a test for this new API ? Unfortunately, "WEBCORE_EXPORT void prefetchDNS(const String& hostname)" API doesn't provide it's return value or callback function to check its results. So it looks difficult to make test case for this new API. (same as Gtk Port) But I think we can confirm its behavior though layout test(LayoutTests/http/tests/misc/dns-prefetch-control.html)
Comment on attachment 255632 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255632&action=review > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:632 > +void ewk_context_prefetch_dns(Ewk_Context* context, const char* hostname) I think "eek_context_dns_prefetch" is more close to EFL naming style. > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:637 > + WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("Bundle.PrefetchDNS")); hmm...why do you hardcode *messageName* with "Bundle.PrefetchDNS" ? I guess that this isn't correct implementation. > Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:78 > + if (messageName == String("PrefetchDNS")) { Use String::fromUTF8(). > Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:81 > + } It would be good if we add ASSERT_NOT_REACHED().
Comment on attachment 255632 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255632&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:632 >> +void ewk_context_prefetch_dns(Ewk_Context* context, const char* hostname) > > I think "eek_context_dns_prefetch" is more close to EFL naming style. Done >> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:637 >> + WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("Bundle.PrefetchDNS")); > > hmm...why do you hardcode *messageName* with "Bundle.PrefetchDNS" ? I guess that this isn't correct implementation. I agreed with you that it looks not good. I think Injected Bundle doesn't not provide other way to define / share its messageName more than String. As we know WKContextPostMessageToInjectedBundle() API only allows string as messageName. So all of current implementation that are using Injected Bundle IPC looks same as this. ex) WebKitTestRunner, webkit_web_context_prefetch_dns() For the reference, I designed two IPC messageName prefix as following for easily distinguish its target modlue. Bundle.MethodName: Request from UI Process to Injected Bundle module. WebKit.MethodName: Request from Injected Bundle module to UI Process. >> Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:78 >> + if (messageName == String("PrefetchDNS")) { > > Use String::fromUTF8(). Done >> Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:81 >> + } > > It would be good if we add ASSERT_NOT_REACHED(). Done
Created attachment 255689 [details] Patch
Comment on attachment 255689 [details] Patch Attachment 255689 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4827336549597184 New failing tests: svg/W3C-SVG-1.1-SE/color-prop-05-t.svg
Created attachment 255690 [details] Archive of layout-test-results from ews106 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
(In reply to comment #3) > Comment on attachment 255632 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=255632&action=review > > >> Source/WebKit2/UIProcess/API/efl/ewk_context.h:532 > >> +EAPI void ewk_context_prefetch_dns(Ewk_Context *context, const char *hostname); > > > > Where is a test for this new API ? > > Unfortunately, "WEBCORE_EXPORT void prefetchDNS(const String& hostname)" API > doesn't provide it's return value or callback function to check its results. > So it looks difficult to make test case for this new API. (same as Gtk Port) > But I think we can confirm its behavior though layout > test(LayoutTests/http/tests/misc/dns-prefetch-control.html) Please mention this test(LayoutTests/http/tests/misc/dns-prefetch-control.html) in ChangeLog. I hope to verify this patch can pass the test.
Created attachment 276307 [details] Patch
(In reply to comment #9) > (In reply to comment #3) > > Comment on attachment 255632 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=255632&action=review > > > > >> Source/WebKit2/UIProcess/API/efl/ewk_context.h:532 > > >> +EAPI void ewk_context_prefetch_dns(Ewk_Context *context, const char *hostname); > > > > > > Where is a test for this new API ? > > > > Unfortunately, "WEBCORE_EXPORT void prefetchDNS(const String& hostname)" API > > doesn't provide it's return value or callback function to check its results. > > So it looks difficult to make test case for this new API. (same as Gtk Port) > > But I think we can confirm its behavior though layout > > test(LayoutTests/http/tests/misc/dns-prefetch-control.html) > > Please mention this > test(LayoutTests/http/tests/misc/dns-prefetch-control.html) in ChangeLog. I > hope to verify this patch can pass the test. Done.
Comment on attachment 276307 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=276307&action=review > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:600 > + WKRetainPtr<WKStringRef> messageName(AdoptWK, WKStringCreateWithUTF8CString("Bundle.PrefetchDNS")); Why doesn't this use adoptWK() like the line below? > Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:78 > + if (messageName == String::fromUTF8("PrefetchDNS")) { messageName == "PrefetchDNS" would be more efficient. You don't need to construct a String. > Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:79 > + String hostname = toImpl(static_cast<WKStringRef>(messageBody))->string(); I believe there is a toWTFString() for this purpose. > Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:114 > CString name = toImpl(messageName)->string().utf8(); If you'd use a String here, it'd make the code below a bit nicer: String name = toWTFString(messageName); > Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:117 > + if (eina_str_has_prefix(name.data(), "Bundle.")) { name.startsWith("Bundle.") > Source/WebKit2/WebProcess/InjectedBundle/API/efl/ewk_extension.cpp:118 > + self->globalMessageReceiver(String(name.data() + strlen("Bundle.")), messageBody); name.substring(strlen("Bundle."))
Comment on attachment 276307 [details] Patch EFL port was removed. This patch is no longer needed in trunk.
Closing this bug because the EFL port has been removed from trunk. If you feel this bug applies to a different upstream WebKit port and was closed in error, please either update the title and reopen the bug, or leave a comment to request this.