Bug 104384 – Web Inspector: Pass the script url to the script-preprocessor script

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[johnjbarton]

Bug 80922 introduced a script preprocessor. Here we add an argument to the preprocessing script supplying the script url or 'name'.  That way the preprocessor script can make processing decisions based on the name.

[johnjbarton]

Created attachment 178250 [details]
Patch

[johnjbarton]

(In reply to comment #0)
... Bug 80992 introduced a script preprocessor.

[Pavel Feldman]

Comment on attachment 178250 [details]
Patch

Sorry for not landing the tests part. Lets give the bots a chance to cycle.

[Build Bot]

Comment on attachment 178250 [details]
Patch

Attachment 178250 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15186531

New failing tests:
inspector/debugger/debugger-script-preprocessor.html

[johnjbarton]

Created attachment 178585 [details]
Patch

[johnjbarton]

I don't know how to find out what error occurred on the mac EWS.

[Pavel Feldman]

(In reply to comment #6)
> I don't know how to find out what error occurred on the mac EWS.

We need to get some good expectations for the JSC ports. I can make one via making a qt build for you.

[Build Bot]

Comment on attachment 178585 [details]
Patch

Attachment 178585 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15260305

New failing tests:
inspector/debugger/debugger-script-preprocessor.html

[Pavel Feldman]

Comment on attachment 178585 [details]
Patch

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

> LayoutTests/inspector/debugger/debugger-script-preprocessor-expected.txt:5
> +inspector-test.js.js

It should be inspector-test.js (not .js.js)

> LayoutTests/inspector/debugger/debugger-script-preprocessor-expected.txt:6
> +debugger-test.js.js

ditto

> LayoutTests/inspector/debugger/debugger-script-preprocessor-expected.txt:7
> +debugger-script-preprocessor.html

This should occur only once

[johnjbarton]

(In reply to comment #9)
> (From update of attachment 178585 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=178585&action=review
> 
> > LayoutTests/inspector/debugger/debugger-script-preprocessor-expected.txt:5
> > +inspector-test.js.js
> 
> It should be inspector-test.js (not .js.js)
> 
> > LayoutTests/inspector/debugger/debugger-script-preprocessor-expected.txt:6
> > +debugger-test.js.js
> 
> ditto

Note: this part of the test was added for bug 104384, beyond bug 80992

I added the ".js" to be certain that the output resulted from the preprocessor call and thus to be certain that the test did indeed use the |name| argument. Would it be clearer if I used ".verified" or some such? 

> 
> > LayoutTests/inspector/debugger/debugger-script-preprocessor-expected.txt:7
> > +debugger-script-preprocessor.html
> 
> This should occur only once

I guess one comes from the <script> tag and one from the 
   onload="load()"
but in any case how can I prevent this second occurrence?

[johnjbarton]

(In reply to comment #8)
> New failing tests:
> inspector/debugger/debugger-script-preprocessor.html

Is it appropriate to skip this test on Mac via the LayoutTests/platform/mac/TestExpectaions ?

[Pavel Feldman]

(In reply to comment #11)
> (In reply to comment #8)
> > New failing tests:
> > inspector/debugger/debugger-script-preprocessor.html
> 
> Is it appropriate to skip this test on Mac via the LayoutTests/platform/mac/TestExpectaions ?

I think you should land jsc expectations and the main ones and chromium as specific. I.e. as in your patch. It is just that jsc expectations needed tweaking in the places I suggested.

[johnjbarton]

(In reply to comment #12)
> I think you should land jsc expectations and the main ones and chromium as specific. I.e. as in your patch. It is just that jsc expectations needed tweaking in the places I suggested.

Sorry I don't understand how to do this. I guess 'jsc' is the Safari port? What tweaks are needed, where might they go and how could I test them?

[Pavel Feldman]

(In reply to comment #13)
> (In reply to comment #12)
> > I think you should land jsc expectations and the main ones and chromium as specific. I.e. as in your patch. It is just that jsc expectations needed tweaking in the places I suggested.
> 
> Sorry I don't understand how to do this. I guess 'jsc' is the Safari port? What tweaks are needed, where might they go and how could I test them?

Comment #9 describes what needs to change. You can only test it using EWS.

[johnjbarton]

(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > I think you should land jsc expectations and the main ones and chromium as specific. I.e. as in your patch. It is just that jsc expectations needed tweaking in the places I suggested.
> > 
> > Sorry I don't understand how to do this. I guess 'jsc' is the Safari port? What tweaks are needed, where might they go and how could I test them?
> 
> Comment #9 describes what needs to change. You can only test it using EWS.

What I did not realize is that there are two different ...expected.txt files. The one in comment #9 is apparently the "jsc expectations", and it should reflect the test running without this preprocessor. I suppose then that its content can be predicted simply by disabling the preprocessor and copying the test results.

The other file, 
 LayoutTests/platform/chromium/inspector/debugger/debugger-script-preprocessor-expected.txt
 is the one that is used when testing chromium and it should contain the preprocessed results.

[johnjbarton]

Created attachment 181946 [details]
Patch

[WebKit Review Bot]

Comment on attachment 181946 [details]
Patch

Clearing flags on attachment: 181946

Committed r139405: <http://trac.webkit.org/changeset/139405>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

New test is failing on EFL port and the diff is different for each build:

--- /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-expected.txt
+++ /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-actual.txt
@@ -4,8 +4,8 @@
 Page reloaded.
 inspector-test.js
 debugger-test.js
+
 debugger-script-preprocessor.html
-
 
 Page reloaded.
 Debugger was disabled.

--- /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-expected.txt
+++ /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-actual.txt
@@ -2,9 +2,9 @@
 
 Debugger was enabled.
 Page reloaded.
-inspector-test.js
 debugger-test.js
 debugger-script-preprocessor.html
+inspector-test.js
 
 
 Page reloaded.

--- /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-expected.txt
+++ /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-actual.txt
@@ -2,11 +2,11 @@
 
 Debugger was enabled.
 Page reloaded.
+
 inspector-test.js
 debugger-test.js
+
 debugger-script-preprocessor.html
-
-
 Page reloaded.
 Debugger was disabled.

[Chris Dumez]

(In reply to comment #19)
> New test is failing on EFL port and the diff is different for each build:
> 
> --- /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-expected.txt
> +++ /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-actual.txt
> @@ -4,8 +4,8 @@
>  Page reloaded.
>  inspector-test.js
>  debugger-test.js
> +
>  debugger-script-preprocessor.html
> -
> 
>  Page reloaded.
>  Debugger was disabled.
> 
> --- /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-expected.txt
> +++ /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-actual.txt
> @@ -2,9 +2,9 @@
> 
>  Debugger was enabled.
>  Page reloaded.
> -inspector-test.js
>  debugger-test.js
>  debugger-script-preprocessor.html
> +inspector-test.js
> 
> 
>  Page reloaded.
> 
> --- /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-expected.txt
> +++ /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/layout-test-results/inspector/debugger/debugger-script-preprocessor-actual.txt
> @@ -2,11 +2,11 @@
> 
>  Debugger was enabled.
>  Page reloaded.
> +
>  inspector-test.js
>  debugger-test.js
> +
>  debugger-script-preprocessor.html
> -
> -
>  Page reloaded.
>  Debugger was disabled.

New test skipped for EFL port in http://trac.webkit.org/changeset/139564