[webkit-reviews] review granted: [Bug 68499] Extract common gdb code into its own function; Remove script gdb-safari : [Attachment 110321] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 14 11:34:37 PDT 2011
David Kilzer (ddkilzer) <ddkilzer at webkit.org> has granted Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 68499: Extract common gdb code into its own function; Remove script
gdb-safari
https://bugs.webkit.org/show_bug.cgi?id=68499
Attachment 110321: Patch
https://bugs.webkit.org/attachment.cgi?id=110321&action=review
------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=110321&action=review
r=me
> Tools/Scripts/webkitdirs.pm:2067
> + return system { $vcBuildPath } $vcBuildPath, "/debugexe",
"\"$safariPath\"", @ARGV;
This returns a different value on success (e.g., when system() returns 0) than
runSafari() does, but I think that's okay since it's wrapped by exitStatus() in
its new use.
> Tools/Scripts/webkitdirs.pm:2084
> + return system { $webKitLauncherPath } $webKitLauncherPath, @ARGV;
This change causes the method to return 0 instead 1 when system() returns 0 on
Windows. This also seems fine since runSafari() is only used (from a quick
grep) in the run-safari script, and that is also wrapped by exitStatus().
> Tools/Scripts/webkitdirs.pm:2102
> + debugMacWebKitApp(File::Spec->catfile(productDir(),
"MiniBrowser.app", "Contents", "MacOS", "MiniBrowser")); # This call never
returns.
Nit: Might be nice to think of a name for debugMacWebKitApp() that removes the
need for the comment. Like execMacWebKitAppForDebugging()?
Not necessary to land the patch, though.
> Tools/Scripts/webkitdirs.pm:2127
> + debugMacWebKitApp(File::Spec->catfile(productDir(),
"WebKitTestRunner")); # This call never returns.
Ditto.
More information about the webkit-reviews
mailing list