RESOLVED FIXED 30256
~96 regression tests fail when using QuickTime 7.6 (they pass with QuickTime 7.3)
https://bugs.webkit.org/show_bug.cgi?id=30256
Summary ~96 regression tests fail when using QuickTime 7.6 (they pass with QuickTime ...
Brian Weinstein
Reported 2009-10-09 11:20:37 PDT
When the Windows buildbots were updated to Quicktime 7.6, ~100 media tests started failing. These were media tests and other tests that depended on media attributes (some of the fast/js tests were failing because they couldn't find an Audio object, for example). When researching, I noticed the Quicktime SDK is only at version 7.3, which could have something to do with it.
Attachments
Proposed patch (2.96 KB, patch)
2009-12-03 09:01 PST, Eric Carlson
no flags
Patch, without tabs this time. (2.97 KB, patch)
2009-12-03 09:08 PST, Eric Carlson
aroben: review+
Simplified patch. (3.02 KB, patch)
2009-12-03 13:41 PST, Eric Carlson
aroben: review+
Brian Weinstein
Comment 1 2009-10-09 11:23:28 PDT
Eric Carlson
Comment 2 2009-12-03 09:01:45 PST
Created attachment 44247 [details] Proposed patch
WebKit Review Bot
Comment 3 2009-12-03 09:03:47 PST
Attachment 44247 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKitTools/DumpRenderTree/win/DumpRenderTree.cpp:216: Tab found; better to use spaces [whitespace/tab] [1] Total errors found: 1
Eric Carlson
Comment 4 2009-12-03 09:08:58 PST
Created attachment 44248 [details] Patch, without tabs this time.
WebKit Review Bot
Comment 5 2009-12-03 09:14:16 PST
style-queue ran check-webkit-style on attachment 44248 [details] without any errors.
Adam Roben (:aroben)
Comment 6 2009-12-03 09:25:31 PST
Comment on attachment 44248 [details] Patch, without tabs this time. > +static void addQTDirToPATH() > +{ > + static LPCWSTR pathEnvironmentVariable = L"PATH"; > + static LPCWSTR quickTimeKeyName = L"Software\\Apple Computer, Inc.\\QuickTime"; > + static LPCWSTR quickTimeSysDir = L"QTSysDir"; > + static bool initialized; > + > + if (initialized) > + return; > + initialized = true; > + > + // Get the QuickTime dll directory from the registry. The key can be in either HKLM or HKCU. > + HKEY hKey; > + if (RegOpenKeyEx(HKEY_LOCAL_MACHINE, quickTimeKeyName, 0, KEY_READ, &hKey)) { > + if (RegOpenKeyEx(HKEY_CURRENT_USER, quickTimeKeyName, 0, KEY_READ, &hKey)) > + return; > + } I think (throughout this function) it would be better to compare with ERROR_SUCCESS. > + DWORD type; > + DWORD bufferSize; > + LONG err = RegQueryValueEx(hKey, quickTimeSysDir, 0, &type, 0, &bufferSize); > + if (err || !bufferSize || type != REG_SZ) { > + RegCloseKey(hKey); > + return; > + } > + > + Vector<TCHAR> qtDir(bufferSize); > + err = RegQueryValueEx(hKey, quickTimeSysDir, 0, 0, (LPBYTE)qtDir.data(), &bufferSize); > + RegCloseKey(hKey); > + if (err || !bufferSize) > + return; Can we use SHGetValue instead? That would be a lot simpler. > + > + // Read the current PATH > + DWORD pathSize = GetEnvironmentVariable(pathEnvironmentVariable, 0, 0); > + Vector<TCHAR> oldPath(pathSize); > + if (!GetEnvironmentVariable(pathEnvironmentVariable, oldPath.data(), oldPath.size())) > + return; > + > + // And add the QuickTime dll. > + wstring newPath; > + newPath.append(qtDir.data(), qtDir.size() - 1); > + newPath.append(L";"); > + newPath.append(oldPath.data(), oldPath.size()); > + SetEnvironmentVariable(pathEnvironmentVariable, newPath.c_str()); > +} There's a strange mix of TCHAR and WCHAR here. I'd just always use WCHAR and call the *W versions of the APIs (e.g., SetEnvironmentVariableW). > @@ -263,6 +309,8 @@ static void initialize() > for (int i = 0; i < ARRAYSIZE(fontsToInstall); ++i) > textRenderer->registerPrivateFont(wstring(resourcesPath + fontsToInstall[i]).c_str()); > > + addQTDirToPATH(); I think it's worth adding a comment explaining why this is necessary. r=me, but you should consider these comments.
Eric Carlson
Comment 7 2009-12-03 13:41:47 PST
Created attachment 44267 [details] Simplified patch.
WebKit Review Bot
Comment 8 2009-12-03 13:42:29 PST
style-queue ran check-webkit-style on attachment 44267 [details] without any errors.
Adam Roben (:aroben)
Comment 9 2009-12-03 13:47:23 PST
Comment on attachment 44267 [details] Simplified patch. > + // Read the current PATH. > + DWORD pathSize = GetEnvironmentVariableW(pathEnvironmentVariable, 0, 0); > + Vector<TCHAR> oldPath(pathSize); > + if (!GetEnvironmentVariable(pathEnvironmentVariable, oldPath.data(), oldPath.size())) > + return; That should be Vector<WCHAR>. r=me
Note You need to log in before you can comment on or make changes to this bug.