[WIN] Use BString in favour of BSTR to improve memory management
Created attachment 156394 [details] Patch
Comment on attachment 156394 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156394&action=review > Source/WebCore/platform/win/BString.cpp:130 > - if (m_bstr) > - SysFreeString(m_bstr); > + clear(); Is SysFreeString null-safe? The previous code didn't seem to think so.
Comment on attachment 156394 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156394&action=review >> Source/WebCore/platform/win/BString.cpp:130 >> + clear(); > > Is SysFreeString null-safe? The previous code didn't seem to think so. Yes it is. The previous code did only on a few places.
Comment on attachment 156394 [details] Patch Clearing flags on attachment: 156394 Committed r128809: <http://trac.webkit.org/changeset/128809>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by 96958
Created attachment 164621 [details] Patch
Comment on attachment 164621 [details] Patch Rejecting attachment 164621 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/13898489
Created attachment 164707 [details] Patch
Comment on attachment 164707 [details] Patch Clearing flags on attachment: 164707 Committed r128988: <http://trac.webkit.org/changeset/128988>
Hello, this patch is causing an assertion failure on Windows. All I'm doing is launching WinLauncher. I get an assertion failure ASSERTION FAILED !m_bstr. Here's the stack trace. WebKit.dll!WebCore::BString::operator&() Line 61 + 0x3a bytes C++ > WebKit.dll!WebView::notifyPreferencesChanged(IWebNotification * notification=0x005fefb8) Line 4591 + 0x16 bytes C++ WebKit.dll!WebView::onNotify(IWebNotification * notification=0x005fefb8) Line 4549 + 0xf bytes C++ WebKit.dll!WebNotificationCenter::postNotificationInternal(IWebNotification * notification=0x005fefb8, wchar_t * notificationName=0x004fb9bc, IUnknown * anObject=0x005be918) Line 131 + 0x14 bytes C++ WebKit.dll!WebNotificationCenter::postNotificationName(wchar_t * notificationName=0x004fb9bc, IUnknown * anObject=0x005be918, IPropertyBag * userInfo=0x00000000) Line 188 C++ WebKit.dll!WebPreferences::postPreferencesChangesNotification() Line 144 + 0x1c bytes C++ WebKit.dll!WebView::initWithFrame(tagRECT frame={...}, wchar_t * frameName=0x00000000, wchar_t * groupName=0x00000000) Line 2726 C++ WinLauncher.dll!dllLauncherEntryPoint(HINSTANCE__ * __formal=0x00120000, HINSTANCE__ * __formal=0x00120000, HINSTANCE__ * __formal=0x00120000, int nCmdShow=1) Line 368 + 0x35 bytes C++ WinLauncher.exe!wWinMain(HINSTANCE__ * hInstance=0x00120000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x004a4afe, int nCmdShow=1) Line 208 + 0x18 bytes C++ WinLauncher.exe!__tmainCRTStartup() Line 589 + 0x1c bytes C kernel32.dll!@BaseThreadInitThunk@12() + 0x12 bytes ntdll.dll!___RtlUserThreadStart@8() + 0x27 bytes ntdll.dll!__RtlUserThreadStart@8() + 0x1b bytes Do you mind taking a look, it's preventing my from debugging things... Thanks!
(In reply to comment #12) > Hello, this patch is causing an assertion failure on Windows. All I'm doing is launching WinLauncher. > > I get an assertion failure ASSERTION FAILED !m_bstr. > > Here's the stack trace. > > > WebKit.dll!WebCore::BString::operator&() Line 61 + 0x3a bytes C++ > > WebKit.dll!WebView::notifyPreferencesChanged(IWebNotification * notification=0x005fefb8) Line 4591 + 0x16 bytes C++ > WebKit.dll!WebView::onNotify(IWebNotification * notification=0x005fefb8) Line 4549 + 0xf bytes C++ > WebKit.dll!WebNotificationCenter::postNotificationInternal(IWebNotification * notification=0x005fefb8, wchar_t * notificationName=0x004fb9bc, IUnknown * anObject=0x005be918) Line 131 + 0x14 bytes C++ > WebKit.dll!WebNotificationCenter::postNotificationName(wchar_t * notificationName=0x004fb9bc, IUnknown * anObject=0x005be918, IPropertyBag * userInfo=0x00000000) Line 188 C++ > WebKit.dll!WebPreferences::postPreferencesChangesNotification() Line 144 + 0x1c bytes C++ > WebKit.dll!WebView::initWithFrame(tagRECT frame={...}, wchar_t * frameName=0x00000000, wchar_t * groupName=0x00000000) Line 2726 C++ > WinLauncher.dll!dllLauncherEntryPoint(HINSTANCE__ * __formal=0x00120000, HINSTANCE__ * __formal=0x00120000, HINSTANCE__ * __formal=0x00120000, int nCmdShow=1) Line 368 + 0x35 bytes C++ > WinLauncher.exe!wWinMain(HINSTANCE__ * hInstance=0x00120000, HINSTANCE__ * hPrevInstance=0x00000000, wchar_t * lpstrCmdLine=0x004a4afe, int nCmdShow=1) Line 208 + 0x18 bytes C++ > WinLauncher.exe!__tmainCRTStartup() Line 589 + 0x1c bytes C > kernel32.dll!@BaseThreadInitThunk@12() + 0x12 bytes > ntdll.dll!___RtlUserThreadStart@8() + 0x27 bytes > ntdll.dll!__RtlUserThreadStart@8() + 0x1b bytes > > Do you mind taking a look, it's preventing my from debugging things... > Thanks! In notifyPreferencesChanged, it looks like the bstr gets set on settings->setCursiveFontFamily(toAtomicString(str));. Later a reference to said bstr gets passed into hr = preferences->defaultTextEncodingName(&str); For some reason the &operator for bstrings is overriden to assert(!b_str). I'm not sure why though...
Committed r129598: <http://trac.webkit.org/changeset/129598>