Bug 97018 - [EFL] Do not dump inspector output in DRT
Summary: [EFL] Do not dump inspector output in DRT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Raphael Kubo da Costa (:rakuco)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-18 07:51 PDT by WebKit Review Bot
Modified: 2012-09-25 05:56 PDT (History)
5 users (show)

See Also:


Attachments
Patch (4.90 KB, patch)
2012-09-24 08:53 PDT, Raphael Kubo da Costa (:rakuco)
no flags Details | Formatted Diff | Diff
Patch (4.90 KB, patch)
2012-09-24 09:41 PDT, Raphael Kubo da Costa (:rakuco)
no flags Details | Formatted Diff | Diff
Patch for landing, skipping some tests in WK2 (5.89 KB, patch)
2012-09-25 05:35 PDT, Raphael Kubo da Costa (:rakuco)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description WebKit Review Bot 2012-09-18 07:51:21 PDT
[EFL] Do not dump inspector output in DRT
Requested by rakuco on #webkit.
Comment 1 Raphael Kubo da Costa (:rakuco) 2012-09-18 07:53:16 PDT
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
Comment 2 Raphael Kubo da Costa (:rakuco) 2012-09-24 08:53:03 PDT
Created attachment 165392 [details]
Patch
Comment 3 Chris Dumez 2012-09-24 09:07:16 PDT
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);
Comment 4 Raphael Kubo da Costa (:rakuco) 2012-09-24 09:13:57 PDT
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).
Comment 5 Chris Dumez 2012-09-24 09:20:02 PDT
(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.
Comment 6 Raphael Kubo da Costa (:rakuco) 2012-09-24 09:38:05 PDT
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.
Comment 7 Raphael Kubo da Costa (:rakuco) 2012-09-24 09:41:00 PDT
Created attachment 165398 [details]
Patch
Comment 8 Raphael Kubo da Costa (:rakuco) 2012-09-24 09:54:34 PDT
Thanks. I will land this tomorrow to watch the bots; I suspect WK2 will not like running these unskipped tests.
Comment 9 Chris Dumez 2012-09-24 09:55:59 PDT
(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.
Comment 10 Raphael Kubo da Costa (:rakuco) 2012-09-25 05:35:24 PDT
Created attachment 165589 [details]
Patch for landing, skipping some tests in WK2
Comment 11 WebKit Review Bot 2012-09-25 05:56:36 PDT
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>
Comment 12 WebKit Review Bot 2012-09-25 05:56:40 PDT
All reviewed patches have been landed.  Closing bug.