Bug 128779 – [EFL][WK1] Do not include cairo header in the public headers

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Ryuan Choi]

Although ewebkit used cairo internally, it's not good for EFL developers to use it.

[Ryuan Choi]

Created attachment 224150 [details]
Patch

[WebKit Commit Bot]

Attachment 224150 [details] did not pass style-queue:


ERROR: Tools/EWebLauncher/main.c:208:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/EWebLauncher/main.c:209:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 224150 [details]
Patch

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

> Source/WebKit/efl/ewk/ewk_view.h:2499
> +EAPI Evas_Object *ewk_view_screenshot_contents_get(const Evas_Object *o, const Eina_Rectangle *area, float scale);

I will be pleased to land this patch with an unit test for this new APIs

[Ryuan Choi]

Created attachment 224158 [details]
Patch

[Ryuan Choi]

(In reply to comment #3)
> (From update of attachment 224150 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=224150&action=review
> 
> > Source/WebKit/efl/ewk/ewk_view.h:2499
> > +EAPI Evas_Object *ewk_view_screenshot_contents_get(const Evas_Object *o, const Eina_Rectangle *area, float scale);
> 
> I will be pleased to land this patch with an unit test for this new APIs

I agree,
But, in order to test this API fully, we should check the pixels of image objects.
And it will be difficult little bit because current framework for webkit1/efl are poor.

Or, do you think that it is enough to check image size using scale?

I considered little bit, but i didn't because this API will be used for pixel tests and we can check the functionality using EWebLauncher.

[Gyuyoung Kim]

(In reply to comment #5)
> (In reply to comment #3)
> > (From update of attachment 224150 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=224150&action=review
> > 
> > > Source/WebKit/efl/ewk/ewk_view.h:2499
> > > +EAPI Evas_Object *ewk_view_screenshot_contents_get(const Evas_Object *o, const Eina_Rectangle *area, float scale);
> > 
> > I will be pleased to land this patch with an unit test for this new APIs
> 
> I agree,
> But, in order to test this API fully, we should check the pixels of image objects.
> And it will be difficult little bit because current framework for webkit1/efl are poor.

I think we can leave a comment. For instance, "FIXME: EFL WK1 has still poor DRT to test this API."
 
> Or, do you think that it is enough to check image size using scale?

It looks we need to check it by using the pixel test. However, it is difficult now, we need to check the image size or image is not null at least.

[Ryuan Choi]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #3)
> > > (From update of attachment 224150 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=224150&action=review
> > > 
> > > > Source/WebKit/efl/ewk/ewk_view.h:2499
> > > > +EAPI Evas_Object *ewk_view_screenshot_contents_get(const Evas_Object *o, const Eina_Rectangle *area, float scale);
> > > 
> > > I will be pleased to land this patch with an unit test for this new APIs
> > 
> > I agree,
> > But, in order to test this API fully, we should check the pixels of image objects.
> > And it will be difficult little bit because current framework for webkit1/efl are poor.
> 
> I think we can leave a comment. For instance, "FIXME: EFL WK1 has still poor DRT to test this API."
> 
> > Or, do you think that it is enough to check image size using scale?
> 
> It looks we need to check it by using the pixel test. However, it is difficult now, we need to check the image size or image is not null at least.

OK, I will

[Ryuan Choi]

Created attachment 224163 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 224163 [details]
Patch

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

> Tools/EWebLauncher/main.c:462
> +            static Evas_Object *screenshot = NULL;

Any reason to define *static* ? Besides could you let me know what functionality do you want to do here ? It seems you just test ewk_view_screenshot_contents_get() API here.

[Ryuan Choi]

Created attachment 224285 [details]
Patch

[Ryuan Choi]

Comment on attachment 224163 [details]
Patch

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

>> Tools/EWebLauncher/main.c:462
>> +            static Evas_Object *screenshot = NULL;
> 
> Any reason to define *static* ? Besides could you let me know what functionality do you want to do here ? It seems you just test ewk_view_screenshot_contents_get() API here.

Yes, it is for visual testing.

I removed

[Gyuyoung Kim]

Comment on attachment 224285 [details]
Patch

LGTM. r=me.

[WebKit Commit Bot]

Comment on attachment 224285 [details]
Patch

Clearing flags on attachment: 224285

Committed r164168: <http://trac.webkit.org/changeset/164168>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.