[Webkit-unassigned] [Bug 66902] Provide option to disable Mac OS 10.7 application resume when using {debug, run}-{safari, minibrowser, test-runner, test-webkit-api}, and run-webkit-app

[bugzilla-daemon at webkit.org]

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


David Kilzer (ddkilzer) <ddkilzer at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #115993|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #14 from David Kilzer (ddkilzer) <ddkilzer at webkit.org>  2011-11-28 12:32:14 PST ---
(From update of attachment 115993)
View in context: https://bugs.webkit.org/attachment.cgi?id=115993&action=review

r=me

> Tools/ChangeLog:4
> +        Provide option to disable Mac OS 10.7 application resume when using
> +        {debug, run}-{safari, minibrowser, test-runner, test-webkit-api}, and run-webkit-app

Nit:  Should mention gdb-safari here as well.

> Tools/ChangeLog:18
> +        * Scripts/debug-minibrowser: Call printHelpAndExitForRunAndDebugWebKitAppIfNeeded()
> +        to print a help message and exit(3) if the command line argument --help was given.

Nit: Don't you mean 'exit(1)' not 'exit(3)' here?

> Tools/Scripts/webkitdirs.pm:59
>         &productDir
> +       &printHelpAndExitForRunAndDebugWebKitAppIfNeeded

The new method should appear before &prodcutDir to keep the list alphabetized.

> Tools/Scripts/webkitdirs.pm:2062
> +    push @args, ("-ApplePersistenceIgnoreState", "YES") if isLion() && checkForArgumentAndRemoveFromArrayRef("--no-saved-state", \@args);

Nit: This means that "--saved-state" is not a valid option, but that's probably okay for now.

It's rather unfortunate that we're doing our own argument parsing in these Perl scripts instead of using GetOpt::Long or GetOpt::Std, but it's hard to distribute the argument checking the way scripts (and library) methods are currently written.  It's also hard to get a list of valid options for a given script because of this same issue.  In the future, we may want to think about how to unify this argument parsing and help printing.  (I'm sure this problem has been solved before as well; maybe we can find a solution from another open source Perl project.)  Just something to think about--it shouldn't block this patch.

> Tools/Scripts/webkitdirs.pm:2094
> -    exec { $gdbPath } $gdbPath, @architectureFlags, $appPath or die;
> +    exec { $gdbPath } $gdbPath, @architectureFlags, $appPath, argumentsForRunAndDebugMacWebKitApp() or die;

Nit: This changes the list of arguments passed to gdb to include a post-processed @ARGV.  That's probably okay, but could break some one relying on the old behavior of ignoring "unknown" arguments.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.