Bug 84576 – [EFL] [DRT] LayoutTestController needs implementation of setJavaScriptProfilingEnabled

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

[Sudarsana Nagineni (babu)]

LayoutTestController needs implementation of setJavaScriptProfilingEnabled in order to unskip tests under fast/profiler.

[Sudarsana Nagineni (babu)]

Created attachment 138329 [details]
Patch

Implement setJavaScriptProfilingEnabled

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 138329 [details]
Patch

Looks good, thanks.

[Sudarsana Nagineni (babu)]

Created attachment 138799 [details]
patch

rebased

[Sudarsana Nagineni (babu)]

Created attachment 138803 [details]
patch

[Martin Robinson]

Comment on attachment 138803 [details]
patch

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

> Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:451
> +    setDeveloperExtrasEnabled(flag);

Should disabling JavaScript profiling disable all developer extras? That seems a bit odd. What if there another developer extra that's enabled and profiling is turned off?

[Martin Robinson]

(In reply to comment #5)
> (From update of attachment 138803 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138803&action=review
> 
> > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:451
> > +    setDeveloperExtrasEnabled(flag);
> 
> Should disabling JavaScript profiling disable all developer extras? That seems a bit odd. What if there another developer extra that's enabled and profiling is turned off?

I guess this isn't an issue?

[Raphael Kubo da Costa (:rakuco)]

Comment on attachment 138803 [details]
patch

cq-'ing while mrobinson's question isn't answered.

[Sudarsana Nagineni (babu)]

(In reply to comment #5)
> (From update of attachment 138803 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138803&action=review
> 
> > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:451
> > +    setDeveloperExtrasEnabled(flag);
> 
> Should disabling JavaScript profiling disable all developer extras? That seems a bit odd. What if there another developer extra that's enabled and profiling is turned off?

setDeveloperExtrasEnabled() controls only whether the Web Inspector should be enabled or not, and this setting is enabled by default on the test startup. So, changing JS profiling setting changes only the Web Inspector behaviour for the test.

[Martin Robinson]

(In reply to comment #8)

> setDeveloperExtrasEnabled() controls only whether the Web Inspector should be enabled or not, and this setting is enabled by default on the test startup. So, changing JS profiling setting changes only the Web Inspector behaviour for the test.

It's probably safer to turn it off if you are enabling the profiler, but not turn it off if you are disabling it.

[Sudarsana Nagineni (babu)]

Created attachment 139016 [details]
Patch

taken feedback into consideration and updated patch.

[WebKit Review Bot]

Comment on attachment 139016 [details]
Patch

Clearing flags on attachment: 139016

Committed r115332: <http://trac.webkit.org/changeset/115332>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.