Bug 61533 - Web Inspector: [Chromium] cpu-profiler-profiling layout test is flaky on Linux Debug
Summary: Web Inspector: [Chromium] cpu-profiler-profiling layout test is flaky on Linu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Mikhail Naganov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-26 07:56 PDT by Mikhail Naganov
Modified: 2011-06-15 23:35 PDT (History)
12 users (show)

See Also:


Attachments
patch (17.92 KB, patch)
2011-06-03 02:57 PDT, Mikhail Naganov
mnaganov: commit-queue-
Details | Formatted Diff | Diff
fix style (17.92 KB, patch)
2011-06-03 04:28 PDT, Mikhail Naganov
mnaganov: commit-queue-
Details | Formatted Diff | Diff
brush up changelog (18.09 KB, patch)
2011-06-03 05:44 PDT, Mikhail Naganov
mnaganov: commit-queue-
Details | Formatted Diff | Diff
fix tabs (18.20 KB, patch)
2011-06-03 05:56 PDT, Mikhail Naganov
mnaganov: commit-queue-
Details | Formatted Diff | Diff
comments addressed (18.19 KB, patch)
2011-06-03 06:15 PDT, Mikhail Naganov
yurys: review+
mnaganov: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Mikhail Naganov 2011-06-03 02:57:23 PDT
Created attachment 95880 [details]
patch
Comment 2 WebKit Review Bot 2011-06-03 03:00:21 PDT
Attachment 95880 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1

Source/WebKit2/WebProcess/WebPage/WebInspector.h:59:  The parameter name "enabled" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Mikhail Naganov 2011-06-03 04:28:49 PDT
Created attachment 95886 [details]
fix style
Comment 4 Mikhail Naganov 2011-06-03 05:44:40 PDT
Created attachment 95900 [details]
brush up changelog
Comment 5 WebKit Review Bot 2011-06-03 05:48:55 PDT
Attachment 95900 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1

Source/WebKit2/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit2/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit2/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Tools/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
Tools/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Tools/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/chromium/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/chromium/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/chromium/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:3:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
LayoutTests/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 16 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Yury Semikhatsky 2011-06-03 05:50:50 PDT
Comment on attachment 95886 [details]
fix style

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

> LayoutTests/ChangeLog:10
> +        * inspector/profiler/cpu-profiler-profiling-fast.html: Added.

I'd rather rename it to cpu-profiler-profiling-without-frontend, otherwise the names assumes that other test is too slow which means that it should be fixed.

> LayoutTests/inspector/profiler/cpu-profiler-profiling-fast.html:56
> +    for (var i = 0; i < indentLevel; ++i)

Why not pass indent space directly?
Comment 7 Mikhail Naganov 2011-06-03 05:56:39 PDT
Created attachment 95902 [details]
fix tabs
Comment 8 Mikhail Naganov 2011-06-03 06:15:15 PDT
Created attachment 95905 [details]
comments addressed
Comment 9 Mikhail Naganov 2011-06-03 06:16:53 PDT
(In reply to comment #6)
> (From update of attachment 95886 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=95886&action=review
> 
> > LayoutTests/ChangeLog:10
> > +        * inspector/profiler/cpu-profiler-profiling-fast.html: Added.
> 
> I'd rather rename it to cpu-profiler-profiling-without-frontend, otherwise the names assumes that other test is too slow which means that it should be fixed.
> 

Renamed.
Well, almost all inspector's test are slow in debug builds

> > LayoutTests/inspector/profiler/cpu-profiler-profiling-fast.html:56
> > +    for (var i = 0; i < indentLevel; ++i)
> 
> Why not pass indent space directly?

Fixed.
Comment 10 Mikhail Naganov 2011-06-03 06:30:59 PDT
Manually committed http://trac.webkit.org/changeset/88010


2011-06-03  Mikhail Naganov  <mnaganov@chromium.org>

        Reviewed by Yury Semikhatsky.

        Web Inspector: [Chromium] cpu-profiler-profiling layout test is flaky on Linux Debug.
        https://bugs.webkit.org/show_bug.cgi?id=61533

        Skip cpu-profiler-profiling in debug, implement a fast headless alternative.

        * inspector/profiler/cpu-profiler-profiling-without-inspector-expected.txt: Added.
        * inspector/profiler/cpu-profiler-profiling-without-inspector.html: Added.
        * platform/chromium/test_expectations.txt:

2011-06-03  Mikhail Naganov  <mnaganov@chromium.org>

        Reviewed by Yury Semikhatsky.

        Web Inspector: [Chromium] cpu-profiler-profiling layout test is flaky on Linux Debug.
        https://bugs.webkit.org/show_bug.cgi?id=61533

        Skip cpu-profiler-profiling in debug, implement a fast headless alternative.

        * public/WebDevToolsAgent.h:
        * src/WebDevToolsAgentImpl.cpp:
        (WebKit::WebDevToolsAgentImpl::setJavaScriptProfilingEnabled):
        * src/WebDevToolsAgentImpl.h:

2011-06-03  Mikhail Naganov  <mnaganov@chromium.org>

        Reviewed by Yury Semikhatsky.

        Web Inspector: [Chromium] cpu-profiler-profiling layout test is flaky on Linux Debug.
        https://bugs.webkit.org/show_bug.cgi?id=61533

        Skip cpu-profiler-profiling in debug, implement a fast headless alternative.

        * DumpRenderTree/chromium/DRTDevToolsAgent.cpp:
        (DRTDevToolsAgent::setJavaScriptProfilingEnabled):
        * DumpRenderTree/chromium/DRTDevToolsAgent.h:
        * DumpRenderTree/chromium/LayoutTestController.cpp:
        (LayoutTestController::LayoutTestController):
        (LayoutTestController::setJavaScriptProfilingEnabled):
        * DumpRenderTree/chromium/LayoutTestController.h:
        * DumpRenderTree/wx/LayoutTestControllerWx.cpp:
        (LayoutTestController::setJavaScriptProfilingEnabled):
        * WebKitTestRunner/InjectedBundle/LayoutTestController.cpp:
        (WTR::LayoutTestController::setJavaScriptProfilingEnabled):
        * WebKitTestRunner/InjectedBundle/LayoutTestController.h:

2011-06-03  Mikhail Naganov  <mnaganov@chromium.org>

        Reviewed by Yury Semikhatsky.

        Web Inspector: [Chromium] cpu-profiler-profiling layout test is flaky on Linux Debug.
        https://bugs.webkit.org/show_bug.cgi?id=61533

        Skip cpu-profiler-profiling in debug, implement a fast headless alternative.

        * WebProcess/InjectedBundle/API/c/WKBundleInspector.cpp:
        (WKBundleInspectorSetJavaScriptProfilingEnabled):
        * WebProcess/InjectedBundle/API/c/WKBundleInspector.h:
        * WebProcess/WebPage/WebInspector.cpp:
        (WebKit::WebInspector::setJavaScriptProfilingEnabled):
        * WebProcess/WebPage/WebInspector.h:
Comment 12 Mikhail Naganov 2011-06-10 13:06:34 PDT
We have found a bug in the VM. It had been fixed: http://code.google.com/p/v8/source/detail?r=8259
Now locally tests doesn't crash anymore. Need to wait until the next VM roll.