Bug 90063 – [EFL] Move BatteryClientEfl from WebKit to WebCore

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Chris Dumez]

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.

[Chris Dumez]

Created attachment 149735 [details]
Patch

[Chris Dumez]

Created attachment 149736 [details]
Patch

[Gyuyoung Kim]

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 ?

[Kihong Kwon]

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.

[Gyuyoung Kim]

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

[Kihong Kwon]

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

[Kenneth Rohde Christiansen]

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.

[Simon Hausmann]

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

[Kenneth Rohde Christiansen]

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

[Simon Hausmann]

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

[Chris Dumez]

(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

[Gyuyoung Kim]

I agree to land this patch for EFL WK1 and WK2.

[Chris Dumez]

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.

[WebKit Review Bot]

Comment on attachment 149736 [details]
Patch

Clearing flags on attachment: 149736

Committed r121828: <http://trac.webkit.org/changeset/121828>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.