[webkit-reviews] review canceled: [Bug 43791] Web Inspector: replace hand written InspectorBackendStub.js by generated one : [Attachment 64348] [patch] fourth iteration. Gtk tests were disabled. Qt tests were disabled. Cr-linux was fixed.

[bugzilla-daemon at webkit.org]

Ilya Tikhonovsky <loislo at chromium.org> has canceled Ilya Tikhonovsky
<loislo at chromium.org>'s request for review:
Bug 43791: Web Inspector: replace hand written InspectorBackendStub.js by
generated one
https://bugs.webkit.org/show_bug.cgi?id=43791

Attachment 64348: [patch] fourth iteration. Gtk tests were disabled. Qt tests
were disabled. Cr-linux was fixed.
https://bugs.webkit.org/attachment.cgi?id=64348&action=review

------- Additional Comments from Ilya Tikhonovsky <loislo at chromium.org>

(In reply to comment #9)
> (From update of attachment 64348 [details])
> > +++ b/LayoutTests/platform/qt/Skipped
> > +# [Qt] InspectorBackendStub.js was removed from the sources and replaced
with generated one.
> > +inspector/
> 
> Could use a bug, I see you made one for Gtk.

https://bugs.webkit.org/show_bug.cgi?id=44042


> > +++ b/WebCore/ChangeLog
> > +2010-08-10  Ilya Tikhonovsky  <loislo at chromium.org>
> > +
> > +	     Reviewed by NOBODY (OOPS!).
> > +
> > +	     Web Inspector: replace hand written InspectorBackendStub.js by
generated one.
> > +	     https://bugs.webkit.org/show_bug.cgi?id=43791
> > +
> > +	     * GNUmakefile.am:
> > +	     * WebCore.gypi:
> > +	     * WebCore.xcodeproj/project.pbxproj:
> > +	     * combine-javascript-resources:
> > +	     * inspector/CodeGeneratorInspector.pm:
> > +	     * inspector/front-end/InspectorBackendStub.js: Removed.
> > +
> 
> Could mention that you're adding a new, optional, command
> line switch to `combine-javascript-resources` to accomplish this.
> I think it would be helpful!

done

> > +++ b/WebCore/WebCore.gypi
> > -		 'inspector/front-end/InspectorBackendStub.js',
> 
> Should this file be removed from WebKit.qrc as well?
> 
>   WebCore/inspector/front-end/WebKit.qrc

done 



> > +++ b/WebCore/WebCore.xcodeproj/project.pbxproj
> > @@ -20306,7 +20306,7 @@
> 
> The following (reformatted) still exists in the build phase:
> 
> >   # Don't do anything for Debug builds, so the Inspector is easier to
debug.
> >   if [[ ${CONFIGURATION:=Debug} == \"Debug\" ]]; then
> >	exit
> >   fi
> 
> If I understand this correctly, a release build would run the
combine-javascript
> script, but not a debug build. Does the generated javascript file get copied
over
> to the build directory elsewhere so this works in debug builds?

done


> > +++ b/WebKit/gtk/WebCoreSupport/InspectorClientGtk.cpp
> > +	 // FixMe: it is not working. It should be not current folder but
subsolder of $(WEBKITOUTPUTDIR)/Programs.
> 
> NIT: Grammar: "It should not be the current directory, but a subdirectory of
$(WEBKITOUTPUTDIR)/Programs."

done