SSIA
Created attachment 184222 [details] patch
Comment on attachment 184222 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=184222&action=review > Source/WebKit2/UIProcess/efl/RequestManagerClientEfl.cpp:72 > - ASSERT(context); > + ASSERT(m_soupRequestManager); where is this change explained?
Comment on attachment 184222 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=184222&action=review >> Source/WebKit2/UIProcess/efl/RequestManagerClientEfl.cpp:72 >> + ASSERT(m_soupRequestManager); > > where is this change explained? ASSERT(context); was erroneous here, as context is used already before this line
Comment on attachment 184222 [details] patch LGTM.
Comment on attachment 184222 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=184222&action=review Looks like a step in the right direction. Some comments: > Source/WebKit2/UIProcess/efl/ContextHistoryClientEfl.cpp:109 > -ContextHistoryClientEfl::ContextHistoryClientEfl(PassRefPtr<WebContext> context) > +ContextHistoryClientEfl::ContextHistoryClientEfl(WKContextRef context) I see you include DownloadProxy.h in this file, is that intentional? > Source/WebKit2/UIProcess/efl/ContextHistoryClientEfl.h:29 > +#include "WKRetainPtr.h" I would use #include <WebKit2/WKRetainPtr.h> to simplify the include paths. Then order the #includes. > Source/WebKit2/UIProcess/efl/DownloadManagerEfl.h:29 > +#include "WKRetainPtr.h" Ditto. > Source/WebKit2/UIProcess/efl/RequestManagerClientEfl.cpp:70 > -RequestManagerClientEfl::RequestManagerClientEfl(EwkContext* context) > - : m_soupRequestManager(WKContextGetSoupRequestManager(toAPI(context->webContext().get()))) > +RequestManagerClientEfl::RequestManagerClientEfl(WKContextRef context) > + : m_soupRequestManager(WKContextGetSoupRequestManager(context)) This class uses WebSoupRequestManagerProxy directly. It looks like it could easily switch to the C API.
Comment on attachment 184222 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=184222&action=review >> Source/WebKit2/UIProcess/efl/ContextHistoryClientEfl.cpp:109 >> +ContextHistoryClientEfl::ContextHistoryClientEfl(WKContextRef context) > > I see you include DownloadProxy.h in this file, is that intentional? apparently not :) thanks for the catch >> Source/WebKit2/UIProcess/efl/ContextHistoryClientEfl.h:29 >> +#include "WKRetainPtr.h" > > I would use #include <WebKit2/WKRetainPtr.h> to simplify the include paths. Then order the #includes. sure it is better >> Source/WebKit2/UIProcess/efl/RequestManagerClientEfl.cpp:70 >> + : m_soupRequestManager(WKContextGetSoupRequestManager(context)) > > This class uses WebSoupRequestManagerProxy directly. It looks like it could easily switch to the C API. mm.. I guess it's in place already 'WKRetainPtr<WKSoupRequestManagerRef> m_soupRequestManager'
Created attachment 185419 [details] patch v2 Took comments from Benjamin into consideration.
> mm.. I guess it's in place already 'WKRetainPtr<WKSoupRequestManagerRef> m_soupRequestManager' Here: toImpl(m_soupRequestManager.get())->registerURIScheme(scheme);
(In reply to comment #8) > > mm.. I guess it's in place already 'WKRetainPtr<WKSoupRequestManagerRef> m_soupRequestManager' > > Here: > toImpl(m_soupRequestManager.get())->registerURIScheme(scheme); indeed.. I haven't noticed :( Thanks for pointing out
Created attachment 185498 [details] patch v3 Removed remaining WK2 internal usage in RequestManagerClientEfl::registerURLSchemeHandler.
Comment on attachment 185498 [details] patch v3 Looks good. Just remove #include "WebSoupRequestManagerProxy.h" from RequestManagerClientEfl.cpp.
Committed r141389: <http://trac.webkit.org/changeset/141389>