Bug 89413 – [EFL][WK2] Add ewk_view_reload_bypass_cache API.

• Component: WebKit2
• Status: RESOLVED FIXED

[Hyerim Bae]

Add ewk_view_reload_bypass_cache API.

[Hyerim Bae]

Created attachment 148520 [details]
Add ewk_view_reload_bypass_cache API

[Chris Dumez]

Comment on attachment 148520 [details]
Add ewk_view_reload_bypass_cache API

LGTM. Note that this function was called "ewk_view_reload_full()" in WK1. Maybe we could try to match the WK1 API as much as possible to simplify porting apps?

[Hyerim Bae]

Yes, but this API is name like below in gtk port.

webkit_web_view_reload_bypass_cache

and I also think this is more clear.

[Chris Dumez]

(In reply to comment #3)
> Yes, but this API is name like below in gtk port.
> 
> webkit_web_view_reload_bypass_cache
> 
> and I also think this is more clear.

Yes, I saw that gtk is using this name and I agree that it is a better name. I personally don't mind either way but I thought I would mention it since it may be a good idea to stay compatible with WK1 API unless there is a good reason not to.

[Ryuan Choi]

(In reply to comment #4)
> (In reply to comment #3)
> > Yes, but this API is name like below in gtk port.
> > 
> > webkit_web_view_reload_bypass_cache
> > 
> > and I also think this is more clear.
> 
> Yes, I saw that gtk is using this name and I agree that it is a better name. I personally don't mind either way but I thought I would mention it since it may be a good idea to stay compatible with WK1 API unless there is a good reason not to.

I agree with chris's opinion.

As another option,
we can think about introducing webkit_web_view_reload_bypass_cache and deprecating ewk_view_load_full API.

Personally, naming itself is not important to me.
But, I believe that compatiblity looks more important.

[Hyerim Bae]

Created attachment 148704 [details]
Rename ewk_view_reload_full.

[Ryuan Choi]

Comment on attachment 148704 [details]
Rename ewk_view_reload_full.

View in context: https://bugs.webkit.org/attachment.cgi?id=148704&action=review

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:190
> + * @return @c EINA_TRUE on success or @c EINA_FALSE otherwise
> + */
> +EAPI Eina_Bool ewk_view_reload_full(Evas_Object *o);

It return EINA_TRUE irrespective of load.
So, this comment can be ambiguous.

[Hyerim Bae]

(In reply to comment #7)
> (From update of attachment 148704 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148704&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_view.h:190
> > + * @return @c EINA_TRUE on success or @c EINA_FALSE otherwise
> > + */
> > +EAPI Eina_Bool ewk_view_reload_full(Evas_Object *o);
> 
> It return EINA_TRUE irrespective of load.
> So, this comment can be ambiguous.

Yes, but other previous API are all the same.

[Gyuyoung Kim]

Comment on attachment 148704 [details]
Rename ewk_view_reload_full.

View in context: https://bugs.webkit.org/attachment.cgi?id=148704&action=review

I think we don't need to use same API name with other ports. It is good to keep same API name with EFL WK1 from API compatibility of view.

> Source/WebKit2/ChangeLog:6
> +        Add API, which is for reloading documents without cache.

Generally, we place patch description below "Reviewed by NOBODY"

>>> Source/WebKit2/UIProcess/API/efl/ewk_view.h:190
>>> +EAPI Eina_Bool ewk_view_reload_full(Evas_Object *o);
>> 
>> It return EINA_TRUE irrespective of load.
>> So, this comment can be ambiguous.
> 
> Yes, but other previous API are all the same.

I think we need to file a new bug for this issue. Here is not proper bug for this argument.

[Hyerim Bae]

(In reply to comment #9)
> (From update of attachment 148704 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148704&action=review
> 
> I think we don't need to use same API name with other ports. It is good to keep same API name with EFL WK1 from API compatibility of view.
> 
> > Source/WebKit2/ChangeLog:6
> > +        Add API, which is for reloading documents without cache.
> 
> Generally, we place patch description below "Reviewed by NOBODY"
> 
> >>> Source/WebKit2/UIProcess/API/efl/ewk_view.h:190
> >>> +EAPI Eina_Bool ewk_view_reload_full(Evas_Object *o);
> >> 
> >> It return EINA_TRUE irrespective of load.
> >> So, this comment can be ambiguous.
> > 
> > Yes, but other previous API are all the same.
> 
> I think we need to file a new bug for this issue. Here is not proper bug for this argument.

Yes, I also agree with you. It seems that new bug is needed.

[Hyerim Bae]

Created attachment 149152 [details]
Add ewk_view_reload_full API.

[Chris Dumez]

Comment on attachment 149152 [details]
Add ewk_view_reload_full API.

LGTM.

[Ryuan Choi]

(In reply to comment #11)
> Created an attachment (id=149152) [details]
> Add ewk_view_reload_full API.

LGTM for now.

[Kenneth Rohde Christiansen]

Comment on attachment 149152 [details]
Add ewk_view_reload_full API.

View in context: https://bugs.webkit.org/attachment.cgi?id=149152&action=review

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:540
> +Eina_Bool ewk_view_reload_full(Evas_Object* ewkView)

I dont find the name very descriptive. What is Qt and other APIś using? _bypass_cache?

[Hyerim Bae]

(In reply to comment #14)
> (From update of attachment 149152 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=149152&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:540
> > +Eina_Bool ewk_view_reload_full(Evas_Object* ewkView)
> 
> I dont find the name very descriptive. What is Qt and other APIś using? _bypass_cache?

Yes, they are using _bypass_cache.
But, Ryuan, Gyuyoung, Chris recommend to use ewk_view_reload_full for compatibility of WK1. Plz refer above their comments.

[Kenneth Rohde Christiansen]

(In reply to comment #15)
> (In reply to comment #14)
> > (From update of attachment 149152 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=149152&action=review
> > 
> > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:540
> > > +Eina_Bool ewk_view_reload_full(Evas_Object* ewkView)
> > 
> > I dont find the name very descriptive. What is Qt and other APIś using? _bypass_cache?
> 
> Yes, they are using _bypass_cache.
> But, Ryuan, Gyuyoung, Chris recommend to use ewk_view_reload_full for compatibility of WK1. Plz refer above their comments.

I think you should make a future API focusing on how people use the web today and not focus on keeping legacy. The API is pretty simple anyways.

[Ryuan Choi]

(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #14)
> > > (From update of attachment 149152 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=149152&action=review
> > > 
> > > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:540
> > > > +Eina_Bool ewk_view_reload_full(Evas_Object* ewkView)
> > > 
> > > I dont find the name very descriptive. What is Qt and other APIś using? _bypass_cache?
> > 
> > Yes, they are using _bypass_cache.
> > But, Ryuan, Gyuyoung, Chris recommend to use ewk_view_reload_full for compatibility of WK1. Plz refer above their comments.
> 
> I think you should make a future API focusing on how people use the web today and not focus on keeping legacy. The API is pretty simple anyways.

Well, each ports use their own name for this functionality.
but they seem to keep the same name for both WK and WK2.

For example, mac port use reload and reloadFromOrigin in both WK1 and WK2.
Gtk port use webkit_web_view_reload and webkit_web_view_reload_bypass_cache in both WK1 and WK2.

WK/Efl has used ewk_view_reload_full.

By the way, I agree that this name is not good.
Does we need to change ewk_view_reload_full in WK/Efl ?
(by deprecating current and adding new one)

[Chris Dumez]

Ryuan, since we apparently all agree that the current name is not good/expressive and since Kenneth would prefer we change the name. Why not simply rename the method for WK2?

It is not like we have a "strong" requirement to stay compatible with WK1 API and I'm sure we break compatibility in many places anyway.

The original name used by Hyerim Bae (same one as for GTK) was good IMHO (sorry Hyerim for making you change it :P).

[Hyerim Bae]

Created attachment 150886 [details]
Modify the API name to ewk_view_reload_bypass_cache.

Modify the API name to ewk_view_reload_bypass_cache.

[Chris Dumez]

Comment on attachment 150886 [details]
Modify the API name to ewk_view_reload_bypass_cache.

LGTM. Thanks.

[WebKit Review Bot]

Comment on attachment 150886 [details]
Modify the API name to ewk_view_reload_bypass_cache.

Clearing flags on attachment: 150886

Committed r121892: <http://trac.webkit.org/changeset/121892>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.