Bug 97018 – [EFL] Do not dump inspector output in DRT

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

[WebKit Review Bot]

[EFL] Do not dump inspector output in DRT
Requested by rakuco on #webkit.

[Raphael Kubo da Costa (:rakuco)]

As per <https://bugs.webkit.org/show_bug.cgi?id=96803#c11>, we are not supposed to dump inspector output in DRT/WTR. Perhaps this should also help unskip other inspector tests besides inspector/extensions/extensions-audits-api.html

[Raphael Kubo da Costa (:rakuco)]

Created attachment 165392 [details]
Patch

[Chris Dumez]

Comment on attachment 165392 [details]
Patch

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

> Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp:149
> +    evas_object_data_set(inspectorView, "ignore-console-messages", reinterpret_cast<const bool*>(true));

This cast looks fishy to me. Shouldn't it be:
bool shouldIgnore = true;
evas_object_data_set(inspectorView, "ignore-console-messages", &shouldIgnore);

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 165392 [details]
Patch

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

>> Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp:149
>> +    evas_object_data_set(inspectorView, "ignore-console-messages", reinterpret_cast<const bool*>(true));
> 
> This cast looks fishy to me. Shouldn't it be:
> bool shouldIgnore = true;
> evas_object_data_set(inspectorView, "ignore-console-messages", &shouldIgnore);

The ugliness is unfortunately unavoidable; using either my form or yours should not change the effects (we are both passing a bool* or const bool* as the last parameter for evas_object_data_set).

[Chris Dumez]

(In reply to comment #4)
> (From update of attachment 165392 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=165392&action=review
> 
> >> Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp:149
> >> +    evas_object_data_set(inspectorView, "ignore-console-messages", reinterpret_cast<const bool*>(true));
> > 
> > This cast looks fishy to me. Shouldn't it be:
> > bool shouldIgnore = true;
> > evas_object_data_set(inspectorView, "ignore-console-messages", &shouldIgnore);
> 
> The ugliness is unfortunately unavoidable; using either my form or yours should not change the effects (we are both passing a bool* or const bool* as the last parameter for evas_object_data_set).

I'm passing a pointer to a boolean. In your case you're casting a boolean into a pointer to a boolean, which I don't understand. This does not look right to me.

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 165392 [details]
Patch

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

>>>> Tools/DumpRenderTree/efl/DumpRenderTreeChrome.cpp:149
>>>> +    evas_object_data_set(inspectorView, "ignore-console-messages", reinterpret_cast<const bool*>(true));
>>> 
>>> This cast looks fishy to me. Shouldn't it be:
>>> bool shouldIgnore = true;
>>> evas_object_data_set(inspectorView, "ignore-console-messages", &shouldIgnore);
>> 
>> The ugliness is unfortunately unavoidable; using either my form or yours should not change the effects (we are both passing a bool* or const bool* as the last parameter for evas_object_data_set).
> 
> I'm passing a pointer to a boolean. In your case you're casting a boolean into a pointer to a boolean, which I don't understand. This does not look right to me.

As explained on IRC: the pointer is not dereferenced anywhere, so either your approach or mine has the exact same result; the only difference is that you have an actual variable whose address you take, which is more readable to some. I will change the code to this style to reach a common denominator.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 165398 [details]
Patch

[Raphael Kubo da Costa (:rakuco)]

Thanks. I will land this tomorrow to watch the bots; I suspect WK2 will not like running these unskipped tests.

[Chris Dumez]

(In reply to comment #8)
> Thanks. I will land this tomorrow to watch the bots; I suspect WK2 will not like running these unskipped tests.

So please test with WK2 before landing this patch.

[Raphael Kubo da Costa (:rakuco)]

Created attachment 165589 [details]
Patch for landing, skipping some tests in WK2

[WebKit Review Bot]

Comment on attachment 165589 [details]
Patch for landing, skipping some tests in WK2

Clearing flags on attachment: 165589

Committed r129494: <http://trac.webkit.org/changeset/129494>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.