We should move the BatteryClientEfl class from WebKit/efl/WebCoreSupport to WebCore/platform/efl to that it can be reused for WebKit2. This will avoid code duplication.
Created attachment 149735 [details] Patch
Created attachment 149736 [details] Patch
If there is no interaction with user(or application), I think we can move this from WebKit layer to WebCore. But, if there is need to interaction with user, we can't move it. CC'ing Kihong. kihong, how do you think about this ?
I am not sure about this. Although we don't need interaction with user(browser or email application) for this because we used edbus library, I don't know how other ports implement this. IMHO, if we want to apply this, we need to arrange implementation with other ports. Some ports are already written batteryClientXXX in the WebKit, and I am not sure about their needs exactly.
(In reply to comment #4) > I am not sure about this. > Although we don't need interaction with user(browser or email application) for this because we used edbus library, I don't know how other ports implement this. > > IMHO, if we want to apply this, we need to arrange implementation with other ports. > Some ports are already written batteryClientXXX in the WebKit, and I am not sure about their needs exactly. If EFL port doesn't interact with user, I think we can move this from WebKit to WebCore layer. Other ports will decide this issue according to its port implementation.
(In reply to comment #5) > (In reply to comment #4) > > I am not sure about this. > > Although we don't need interaction with user(browser or email application) for this because we used edbus library, I don't know how other ports implement this. > > > > IMHO, if we want to apply this, we need to arrange implementation with other ports. > > Some ports are already written batteryClientXXX in the WebKit, and I am not sure about their needs exactly. > > If EFL port doesn't interact with user, I think we can move this from WebKit to WebCore layer. Other ports will decide this issue according to its port implementation. If we use EFL on the other OS, maybe we need to re-move this under WebKit. But, if you don't mind about this, I don't have a problem with this.
Comment on attachment 149736 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=149736&action=review > Source/WebCore/ChangeLog:9 > + Move BatteryClientEfl class from WebKit to WebCore > + so that it can be reused in WebKit2. The clients are really supposed to be implemented on the API side (thus WebKit) so in a way it is sad to move it to WebCore for sharing and not to a Source/WebKitShared directory or similar. Anyway we did the same for Qt. I am just wondering whether it is the time to discuss this and figuring out the right way of doing this; sharing WebKit API side code across API ports.
(In reply to comment #7) > (From update of attachment 149736 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=149736&action=review > > > Source/WebCore/ChangeLog:9 > > + Move BatteryClientEfl class from WebKit to WebCore > > + so that it can be reused in WebKit2. > > The clients are really supposed to be implemented on the API side (thus WebKit) so in a way it is sad to move it to WebCore for sharing and not to a Source/WebKitShared directory or similar. Anyway we did the same for Qt. I am just wondering whether it is the time to discuss this and figuring out the right way of doing this; sharing WebKit API side code across API ports. I'm not actually sure why APIs like these go through the WebKit layer when it's clearly something that's delivered by the underlying platform. That said, for WebKit2 there appears often to be a need to delegate things over to the UIProcess to retain the sandbox protection, so for that WebKit seems like a more fitting place.
> > The clients are really supposed to be implemented on the API side (thus WebKit) so in a way it is sad to move it to WebCore for sharing and not to a Source/WebKitShared directory or similar. Anyway we did the same for Qt. I am just wondering whether it is the time to discuss this and figuring out the right way of doing this; sharing WebKit API side code across API ports. > > I'm not actually sure why APIs like these go through the WebKit layer when it's clearly something that's delivered by the underlying platform. > > That said, for WebKit2 there appears often to be a need to delegate things over to the UIProcess to retain the sandbox protection, so for that WebKit seems like a more fitting place. Client are meant to be implemented as part of the API layer (WebKit/). For some ports this might not make sense if they don't allow embedders any API to modify this, but it does make sense in the case of WebKit2. So basically what we need is a shared (wk1 + wk2) WebCoreSupport. I am OK with moving this to WebCore for now though.
(In reply to comment #9) > So basically what we need is a shared (wk1 + wk2) WebCoreSupport. I am OK with moving this to WebCore for now though. I really like that.
(In reply to comment #10) > (In reply to comment #9) > > So basically what we need is a shared (wk1 + wk2) WebCoreSupport. I am OK with moving this to WebCore for now though. > > I really like that. +1
I agree to land this patch for EFL WK1 and WK2.
Can we land this patch first? If we decide to do refactoring, we can move it again later. In the meantime, this allows us to reuse the code in WK2.
Comment on attachment 149736 [details] Patch Clearing flags on attachment: 149736 Committed r121828: <http://trac.webkit.org/changeset/121828>
All reviewed patches have been landed. Closing bug.