Bug 76935

<rdar://problem/10747585>

See 'setUpWindowsCrashLogSaving' in the ORWT Perl script to see what needs to be done here. Note: Windows 8 takes long strides to make this even more impossible. We might need to require that test bots have the registry key permissions changes to permit regular users to change this particular value.

Created attachment 203370 [details] Patch

To encourage Windows 7 and Windows 8 to allow the registry keys to be modified, you need to do the following: 1. Since we are running in 32-bit mode, the actual key to be modified is "HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\Windows NT\CurrentVersion\AeDebug" This is the location we are touching using the "regtool --wow32" command. 2. Adjust the security settings as follows: (a) Launch regedit as an Administrator. (b) Navigate to "HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\Windows NT\CurrentVersion\AeDebug". (c) Right-click on this node, and select 'Permissions' (d) Make sure that the "Users" group is given Full Control over this key. This permission adjustment is needed to make sure that Windows will allow the script to flip the desired debugger from the ntsd.exe program back to whatever the user wanted originally.

Comment on attachment 203370 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203370&action=review Looks okay but needs to use more webkitpy abstractions to be consistent. > Tools/Scripts/webkitpy/port/win.py:141 > + possible_paths = [self._filesystem.join(os.environ['PROGRAMFILES'], "Windows Kits", "8.0", "Debuggers", "x64", "ntsd.exe"), > + self._filesystem.join(os.environ['PROGRAMFILES'], "Windows Kits", "8.0", "Debuggers", "x86", "ntsd.exe"), > + self._filesystem.join(os.environ['PROGRAMFILES'], "Debugging Tools for Windows (x86)", "ntsd.exe"), > + self._filesystem.join(os.environ['ProgramW6432'], "Debugging Tools for Windows (x64)", "ntsd.exe"), > + self._filesystem.join(os.environ['SYSTEMROOT'], "system32", "ntsd.exe")] Nit: wrong indentation. self. should appear exactly 4 spaces to the right of possible. > Tools/Scripts/webkitpy/port/win.py:157 > + with open(command_file, "w") as text_file: > + text_file.write(".logopen /t \"%s\\CrashLog%s.txt\"\n" % (cygpath(self.results_directory()), self.CRASH_LOG_PREFIX)) > + text_file.write(".srcpath \"%s\"\n" % cygpath(os.environ['WEBKIT_SOURCE'])) > + text_file.write("!analyze -vv\n") > + text_file.write("~*kpn\n") > + text_file.write("q\n") Please use self._filesystem.write_text_file. cygpath(os.environ['WEBKIT_SOURCE']) should be read off of WebKitFinder.webkit_base. > Tools/Scripts/webkitpy/port/win.py:162 > + value = subprocess.Popen(["regtool", "--wow32", "get", registry_key], bufsize=1024, stdout=subprocess.PIPE).communicate()[0] Use webkitpy.common.system.executive. > Tools/Scripts/webkitpy/port/win.py:174 > + rc = subprocess.call(["regtool", "--wow32", "set", "-s", str(registry_key), str(value)]) > + if rc == 2: > + rc = subprocess.call(["regtool", "--wow32", "add", "-s", str(registry_key)]) > + if rc == 0: > + rc = subprocess.call(["regtool", "--wow32", "set", "-s", str(registry_key), str(value)]) > + if rc: > + _log.warn("Error setting key: %s to value %s. Error=%ld." % (key, value, rc)) > + return False Ditto. > Tools/Scripts/webkitpy/port/win.py:190 > + ntsd_path = self._ntsd_location() > + if None == ntsd_path: Do: not ntsd_path. > Tools/Scripts/webkitpy/port/win.py:199 > + command_file = self.create_debugger_command_file() > + if None == command_file: Ditto. > Tools/Scripts/webkitpy/port/win.py:201 > + debugger_options = "\"" + cygpath(ntsd_path) + "\" -p %ld -e %ld -g -lines -cf \"" + cygpath(command_file) + "\"" Why not '"%s" -p %ld -e %ld -g -lines -cf "%s"' % (cygpath(ntsd_path), cygpath(command_file))?

Comment on attachment 203370 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203370&action=review >> Tools/Scripts/webkitpy/port/win.py:141 >> + self._filesystem.join(os.environ['SYSTEMROOT'], "system32", "ntsd.exe")] > > Nit: wrong indentation. self. should appear exactly 4 spaces to the right of possible. Done. >> Tools/Scripts/webkitpy/port/win.py:157 >> + text_file.write("q\n") > > Please use self._filesystem.write_text_file. cygpath(os.environ['WEBKIT_SOURCE']) should be read off of WebKitFinder.webkit_base. Great! I didn't know about WebKitFinder. That's very useful. >> Tools/Scripts/webkitpy/port/win.py:162 >> + value = subprocess.Popen(["regtool", "--wow32", "get", registry_key], bufsize=1024, stdout=subprocess.PIPE).communicate()[0] > > Use webkitpy.common.system.executive. Done >> Tools/Scripts/webkitpy/port/win.py:174 >> + return False > > Ditto. Done >> Tools/Scripts/webkitpy/port/win.py:190 >> + if None == ntsd_path: > > Do: not ntsd_path. Done >> Tools/Scripts/webkitpy/port/win.py:199 >> + if None == command_file: > > Ditto. Done >> Tools/Scripts/webkitpy/port/win.py:201 >> + debugger_options = "\"" + cygpath(ntsd_path) + "\" -p %ld -e %ld -g -lines -cf \"" + cygpath(command_file) + "\"" > > Why not '"%s" -p %ld -e %ld -g -lines -cf "%s"' % (cygpath(ntsd_path), cygpath(command_file))? Yes, that's much cleaner. I had to use the new string format syntax to avoid having to escape all of the other format specifiers (e.g., the '%ld' tokens) that are meant to be passed to the external utility.

Created attachment 203403 [details] Patch

Comment on attachment 203403 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203403&action=review > Tools/Scripts/webkitpy/port/win.py:33 > +import subprocess We shouldn't be importing this. > Tools/Scripts/webkitpy/port/win.py:35 > +import tempfile We shouldn't include this. > Tools/Scripts/webkitpy/port/win.py:150 > + debugger_temp_directory = tempfile.mkdtemp() Use filesystem.mkdtemp

Committed r151004: <http://trac.webkit.org/changeset/151004>