From 77e71bd9232d0160af6b8a148a8542b2210e069e Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Wed, 18 Dec 2013 10:20:15 +0900 Subject: [PATCH 01/31] Bug 947981 - broken --with-system-icu build. r=glandium --- configure.in | 1 + js/src/config/Makefile.in | 1 - js/src/configure.in | 1 + 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/configure.in b/configure.in index dca0c6f0df9..ce361ef02a4 100644 --- a/configure.in +++ b/configure.in @@ -3914,6 +3914,7 @@ MOZ_ARG_WITH_BOOL(system-icu, if test -n "$MOZ_NATIVE_ICU"; then PKG_CHECK_MODULES(MOZ_ICU, icu-i18n >= 50.1) MOZ_JS_STATIC_LIBS="$MOZ_JS_STATIC_LIBS $MOZ_ICU_LIBS" + MOZ_SHARED_ICU=1 fi AC_SUBST(MOZ_NATIVE_ICU) diff --git a/js/src/config/Makefile.in b/js/src/config/Makefile.in index 3a0a7a52824..7bed1ff4a1c 100644 --- a/js/src/config/Makefile.in +++ b/js/src/config/Makefile.in @@ -36,7 +36,6 @@ export:: \ $(call mkdir_deps,system_wrappers_js) \ $(NULL) $(PYTHON) -m mozbuild.action.preprocessor $(DEFINES) $(ACDEFINES) \ - -DMOZ_NATIVE_ICU=$(MOZ_NATIVE_ICU) \ $(srcdir)/system-headers | $(PERL) $(srcdir)/make-system-wrappers.pl system_wrappers_js $(INSTALL) system_wrappers_js $(DIST) diff --git a/js/src/configure.in b/js/src/configure.in index 451ed70704c..c586047093e 100644 --- a/js/src/configure.in +++ b/js/src/configure.in @@ -4165,6 +4165,7 @@ MOZ_ARG_WITH_BOOL(system-icu, if test -n "$MOZ_NATIVE_ICU"; then PKG_CHECK_MODULES(MOZ_ICU, icu-i18n >= 50.1) + MOZ_SHARED_ICU=1 fi MOZ_ARG_WITH_STRING(intl-api, From e0ff6eabfe01bc02c8a0a1cac9bb5945e70c8fb3 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 18 Dec 2013 10:43:11 +0900 Subject: [PATCH 02/31] Bug 950559 part.1 nsContentEventHandler should set plugin's rect to query editor rect event if plugin has focus r=smaug --- content/events/src/nsContentEventHandler.cpp | 72 +++++++++++++++----- content/events/src/nsContentEventHandler.h | 7 ++ 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/content/events/src/nsContentEventHandler.cpp b/content/events/src/nsContentEventHandler.cpp index 2e9f959c81e..bea125e50cc 100644 --- a/content/events/src/nsContentEventHandler.cpp +++ b/content/events/src/nsContentEventHandler.cpp @@ -6,6 +6,7 @@ #include "nsContentEventHandler.h" #include "nsCOMPtr.h" +#include "nsFocusManager.h" #include "nsPresContext.h" #include "nsIPresShell.h" #include "nsISelection.h" @@ -31,6 +32,7 @@ using namespace mozilla; using namespace mozilla::dom; +using namespace mozilla::widget; /******************************************************************/ /* nsContentEventHandler */ @@ -127,6 +129,55 @@ nsContentEventHandler::Init(WidgetSelectionEvent* aEvent) return NS_OK; } +nsIContent* +nsContentEventHandler::GetFocusedContent() +{ + nsIDocument* doc = mPresShell->GetDocument(); + if (!doc) { + return nullptr; + } + nsCOMPtr window = do_QueryInterface(doc->GetWindow()); + nsCOMPtr focusedWindow; + return nsFocusManager::GetFocusedDescendant(window, true, + getter_AddRefs(focusedWindow)); +} + +bool +nsContentEventHandler::IsPlugin(nsIContent* aContent) +{ + return aContent && + aContent->GetDesiredIMEState().mEnabled == IMEState::PLUGIN; +} + +nsresult +nsContentEventHandler::QueryContentRect(nsIContent* aContent, + WidgetQueryContentEvent* aEvent) +{ + NS_PRECONDITION(aContent, "aContent must not be null"); + + nsIFrame* frame = aContent->GetPrimaryFrame(); + NS_ENSURE_TRUE(frame, NS_ERROR_FAILURE); + + // get rect for first frame + nsRect resultRect(nsPoint(0, 0), frame->GetRect().Size()); + nsresult rv = ConvertToRootViewRelativeOffset(frame, resultRect); + NS_ENSURE_SUCCESS(rv, rv); + + // account for any additional frames + while ((frame = frame->GetNextContinuation()) != nullptr) { + nsRect frameRect(nsPoint(0, 0), frame->GetRect().Size()); + rv = ConvertToRootViewRelativeOffset(frame, frameRect); + NS_ENSURE_SUCCESS(rv, rv); + resultRect.UnionRect(resultRect, frameRect); + } + + aEvent->mReply.mRect = + resultRect.ToOutsidePixels(mPresContext->AppUnitsPerDevPixel()); + aEvent->mSucceeded = true; + + return NS_OK; +} + // Editor places a bogus BR node under its root content if the editor doesn't // have any text. This happens even for single line editors. // When we get text content and when we change the selection, @@ -676,25 +727,10 @@ nsContentEventHandler::OnQueryEditorRect(WidgetQueryContentEvent* aEvent) if (NS_FAILED(rv)) return rv; - nsIFrame* frame = mRootContent->GetPrimaryFrame(); - NS_ENSURE_TRUE(frame, NS_ERROR_FAILURE); - - // get rect for first frame - nsRect resultRect(nsPoint(0, 0), frame->GetRect().Size()); - rv = ConvertToRootViewRelativeOffset(frame, resultRect); + nsIContent* focusedContent = GetFocusedContent(); + rv = QueryContentRect(IsPlugin(focusedContent) ? + focusedContent : mRootContent.get(), aEvent); NS_ENSURE_SUCCESS(rv, rv); - - // account for any additional frames - while ((frame = frame->GetNextContinuation()) != nullptr) { - nsRect frameRect(nsPoint(0, 0), frame->GetRect().Size()); - rv = ConvertToRootViewRelativeOffset(frame, frameRect); - NS_ENSURE_SUCCESS(rv, rv); - resultRect.UnionRect(resultRect, frameRect); - } - - aEvent->mReply.mRect = - resultRect.ToOutsidePixels(mPresContext->AppUnitsPerDevPixel()); - aEvent->mSucceeded = true; return NS_OK; } diff --git a/content/events/src/nsContentEventHandler.h b/content/events/src/nsContentEventHandler.h index f35b16f33af..09adb8a4480 100644 --- a/content/events/src/nsContentEventHandler.h +++ b/content/events/src/nsContentEventHandler.h @@ -81,6 +81,13 @@ public: static uint32_t GetNativeTextLength(nsIContent* aContent, uint32_t aMaxLength = UINT32_MAX); protected: + // Returns focused content (including its descendant documents). + nsIContent* GetFocusedContent(); + // Returns true if the content is a plugin host. + bool IsPlugin(nsIContent* aContent); + // QueryContentRect() sets the rect of aContent's frame(s) to aEvent. + nsresult QueryContentRect(nsIContent* aContent, + mozilla::WidgetQueryContentEvent* aEvent); // Make the DOM range from the offset of FlatText and the text length. // If aExpandToClusterBoundaries is true, the start offset and the end one are // expanded to nearest cluster boundaries. From b8cb3cd758578cca7d2df15a580a4b72201039a3 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 18 Dec 2013 10:43:15 +0900 Subject: [PATCH 03/31] Bug 950559 part.2 nsIMM32Handler should set IME releated windows to left-bottom of focused plugin r=emk --- widget/windows/WinIMEHandler.cpp | 12 ++++++ widget/windows/nsIMM32Handler.cpp | 72 +++++++++++++++++++++++++++++++ widget/windows/nsIMM32Handler.h | 4 +- 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/widget/windows/WinIMEHandler.cpp b/widget/windows/WinIMEHandler.cpp index c8f24a8e064..581c0cfe5fd 100644 --- a/widget/windows/WinIMEHandler.cpp +++ b/widget/windows/WinIMEHandler.cpp @@ -104,6 +104,18 @@ IMEHandler::ProcessMessage(nsWindow* aWindow, UINT aMessage, { #ifdef NS_ENABLE_TSF if (IsTSFAvailable()) { + if (aMessage == WM_IME_SETCONTEXT) { + // If a windowless plugin had focus and IME was handled on it, composition + // window was set the position. After that, even in TSF mode, WinXP keeps + // to use composition window at the position if the active IME is not + // aware TSF. For avoiding this issue, we need to hide the composition + // window here. + if (aWParam) { + aLParam &= ~ISC_SHOWUICOMPOSITIONWINDOW; + } + return false; + } + if (aMessage == WM_USER_TSF_TEXTCHANGE) { nsTextStore::OnTextChangeMsg(); aResult.mConsumed = true; diff --git a/widget/windows/nsIMM32Handler.cpp b/widget/windows/nsIMM32Handler.cpp index b331b4ef80f..fdc50cad6ab 100644 --- a/widget/windows/nsIMM32Handler.cpp +++ b/widget/windows/nsIMM32Handler.cpp @@ -763,6 +763,8 @@ nsIMM32Handler::OnIMEStartCompositionOnPlugin(nsWindow* aWindow, aWindow->GetWindowHandle(), mIsComposingOnPlugin ? "TRUE" : "FALSE")); mIsComposingOnPlugin = true; mComposingWindow = aWindow; + nsIMEContext IMEContext(aWindow->GetWindowHandle()); + SetIMERelatedWindowsPosOnPlugin(aWindow, IMEContext); aResult.mConsumed = aWindow->DispatchPluginEvent(WM_IME_STARTCOMPOSITION, wParam, lParam, false); @@ -795,6 +797,8 @@ nsIMM32Handler::OnIMECompositionOnPlugin(nsWindow* aWindow, if (IS_COMPOSING_LPARAM(lParam)) { mIsComposingOnPlugin = true; mComposingWindow = aWindow; + nsIMEContext IMEContext(aWindow->GetWindowHandle()); + SetIMERelatedWindowsPosOnPlugin(aWindow, IMEContext); } aResult.mConsumed = aWindow->DispatchPluginEvent(WM_IME_COMPOSITION, wParam, lParam, true); @@ -813,6 +817,12 @@ nsIMM32Handler::OnIMEEndCompositionOnPlugin(nsWindow* aWindow, mIsComposingOnPlugin = false; mComposingWindow = nullptr; + + if (mNativeCaretIsCreated) { + ::DestroyCaret(); + mNativeCaretIsCreated = false; + } + aResult.mConsumed = aWindow->DispatchPluginEvent(WM_IME_ENDCOMPOSITION, wParam, lParam, false); @@ -1930,6 +1940,68 @@ nsIMM32Handler::SetIMERelatedWindowsPos(nsWindow* aWindow, return true; } +void +nsIMM32Handler::SetIMERelatedWindowsPosOnPlugin(nsWindow* aWindow, + const nsIMEContext& aIMEContext) +{ + WidgetQueryContentEvent editorRectEvent(true, NS_QUERY_EDITOR_RECT, aWindow); + aWindow->InitEvent(editorRectEvent); + aWindow->DispatchWindowEvent(&editorRectEvent); + if (!editorRectEvent.mSucceeded) { + PR_LOG(gIMM32Log, PR_LOG_ALWAYS, + ("IMM32: SetIMERelatedWindowsPosOnPlugin, " + "FAILED (NS_QUERY_EDITOR_RECT)")); + return; + } + + // Clip the plugin rect by the client rect of the window because composition + // window needs to be specified the position in the client area. + nsWindow* toplevelWindow = aWindow->GetTopLevelWindow(false); + nsIntRect pluginRectInScreen = + editorRectEvent.mReply.mRect + toplevelWindow->WidgetToScreenOffset(); + nsIntRect winRectInScreen; + aWindow->GetClientBounds(winRectInScreen); + // composition window cannot be positioned on the edge of client area. + winRectInScreen.width--; + winRectInScreen.height--; + nsIntRect clippedPluginRect; + clippedPluginRect.x = + std::min(std::max(pluginRectInScreen.x, winRectInScreen.x), + winRectInScreen.XMost()); + clippedPluginRect.y = + std::min(std::max(pluginRectInScreen.y, winRectInScreen.y), + winRectInScreen.YMost()); + int32_t xMost = std::min(pluginRectInScreen.XMost(), winRectInScreen.XMost()); + int32_t yMost = std::min(pluginRectInScreen.YMost(), winRectInScreen.YMost()); + clippedPluginRect.width = std::max(0, xMost - clippedPluginRect.x); + clippedPluginRect.height = std::max(0, yMost - clippedPluginRect.y); + clippedPluginRect -= aWindow->WidgetToScreenOffset(); + + // Cover the plugin with native caret. This prevents IME's window and plugin + // overlap. + if (mNativeCaretIsCreated) { + ::DestroyCaret(); + } + mNativeCaretIsCreated = + ::CreateCaret(aWindow->GetWindowHandle(), nullptr, + clippedPluginRect.width, clippedPluginRect.height); + ::SetCaretPos(clippedPluginRect.x, clippedPluginRect.y); + + // Set the composition window to bottom-left of the clipped plugin. + // As far as we know, there is no IME for RTL language. Therefore, this code + // must not need to take care of RTL environment. + COMPOSITIONFORM compForm; + compForm.dwStyle = CFS_POINT; + compForm.ptCurrentPos.x = clippedPluginRect.BottomLeft().x; + compForm.ptCurrentPos.y = clippedPluginRect.BottomLeft().y; + if (!::ImmSetCompositionWindow(aIMEContext.get(), &compForm)) { + PR_LOG(gIMM32Log, PR_LOG_ALWAYS, + ("IMM32: SetIMERelatedWindowsPosOnPlugin, " + "FAILED to set composition window")); + return; + } +} + void nsIMM32Handler::ResolveIMECaretPos(nsIWidget* aReferenceWidget, nsIntRect& aCursorRect, diff --git a/widget/windows/nsIMM32Handler.h b/widget/windows/nsIMM32Handler.h index f196d2feff5..48199d3b448 100644 --- a/widget/windows/nsIMM32Handler.h +++ b/widget/windows/nsIMM32Handler.h @@ -261,7 +261,9 @@ protected: nsACString& aANSIStr); bool SetIMERelatedWindowsPos(nsWindow* aWindow, - const nsIMEContext &aIMEContext); + const nsIMEContext& aIMEContext); + void SetIMERelatedWindowsPosOnPlugin(nsWindow* aWindow, + const nsIMEContext& aIMEContext); bool GetCharacterRectOfSelectedTextAt(nsWindow* aWindow, uint32_t aOffset, nsIntRect &aCharRect); From 0fe669918156a9d64f38a839d4e0fca57f8ffc5a Mon Sep 17 00:00:00 2001 From: Matt Woodrow Date: Wed, 18 Dec 2013 14:58:21 +1300 Subject: [PATCH 04/31] Bug 950490 - Don't apply the transform twice when doing blurring in user-space. r=roc --- layout/base/nsCSSRendering.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/layout/base/nsCSSRendering.cpp b/layout/base/nsCSSRendering.cpp index 08037e9d08c..7ece3a98756 100644 --- a/layout/base/nsCSSRendering.cpp +++ b/layout/base/nsCSSRendering.cpp @@ -4862,6 +4862,8 @@ nsContextBoxBlur::BlurRectangle(gfxContext* aDestinationCtx, scaleX = transform.xx; scaleY = transform.yy; aDestinationCtx->IdentityMatrix(); + } else { + transform = gfxMatrix(); } gfxPoint blurStdDev = ComputeBlurStdDev(aBlurRadius, aAppUnitsPerDevPixel, scaleX, scaleY); From ba7c24034b3ce109c572e5d2cd6dc251b1f4725e Mon Sep 17 00:00:00 2001 From: Matt Woodrow Date: Wed, 18 Dec 2013 14:58:57 +1300 Subject: [PATCH 05/31] Bug 946929 - Add some debug logging to IsDOMPaintEventPending. r=mats --- layout/base/nsPresContext.cpp | 36 +++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/layout/base/nsPresContext.cpp b/layout/base/nsPresContext.cpp index d32c3df54ed..e42ff99641f 100644 --- a/layout/base/nsPresContext.cpp +++ b/layout/base/nsPresContext.cpp @@ -123,12 +123,48 @@ nsPresContext::MakeColorPref(const nsString& aColor) : NS_RGB(0, 0, 0); } +static void DumpPresContextState(nsPresContext* aPC) +{ + printf_stderr("PresContext(%p) ", aPC); + nsIURI* uri = aPC->Document()->GetDocumentURI(); + if (uri) { + nsAutoCString uriSpec; + nsresult rv = uri->GetSpec(uriSpec); + if (NS_SUCCEEDED(rv)) { + printf_stderr("%s ", uriSpec.get()); + } + } + nsIPresShell* shell = aPC->GetPresShell(); + if (shell) { + printf_stderr("PresShell(%p) - IsDestroying(%i) IsFrozen(%i) IsActive(%i) IsVisible(%i) IsNeverPainting(%i) GetRootFrame(%p)", + shell->IsDestroying(), + shell->IsFrozen(), + shell->IsActive(), + shell->IsVisible(), + shell->IsNeverPainting(), + shell->GetRootFrame()); + } + printf_stderr("\n"); +} + bool nsPresContext::IsDOMPaintEventPending() { if (mFireAfterPaintEvents) { return true; } + if (!GetDisplayRootPresContext() || + !GetDisplayRootPresContext()->GetRootPresContext()) { + printf_stderr("Failed to find root pres context, dumping pres context and ancestors\n"); + nsPresContext* pc = this; + for (;;) { + DumpPresContextState(pc); + nsPresContext* parent = pc->GetParentPresContext(); + if (!parent) + break; + pc = parent; + } + } if (GetDisplayRootPresContext()->GetRootPresContext()->mRefreshDriver->ViewManagerFlushIsPending()) { // Since we're promising that there will be a MozAfterPaint event // fired, we record an empty invalidation in case display list From bfac9cdb322ddc44f88ba924ff4945e27da21cf3 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 17 Dec 2013 18:01:41 -0800 Subject: [PATCH 06/31] Bug 946815 - Hoist system-principal checks into CommonTestPermission. r=baku --- extensions/cookie/nsPermissionManager.cpp | 30 ++++------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/extensions/cookie/nsPermissionManager.cpp b/extensions/cookie/nsPermissionManager.cpp index 5eb19abf12a..980ffd29251 100644 --- a/extensions/cookie/nsPermissionManager.cpp +++ b/extensions/cookie/nsPermissionManager.cpp @@ -958,13 +958,6 @@ nsPermissionManager::TestExactPermissionFromPrincipal(nsIPrincipal* aPrincipal, const char* aType, uint32_t* aPermission) { - NS_ENSURE_ARG_POINTER(aPrincipal); - - if (nsContentUtils::IsSystemPrincipal(aPrincipal)) { - *aPermission = nsIPermissionManager::ALLOW_ACTION; - return NS_OK; - } - return CommonTestPermission(aPrincipal, aType, aPermission, true, true); } @@ -973,15 +966,6 @@ nsPermissionManager::TestExactPermanentPermission(nsIPrincipal* aPrincipal, const char* aType, uint32_t* aPermission) { - NS_ENSURE_ARG_POINTER(aPrincipal); - - // System principals do not have URI so we can't try to get - // retro-compatibility here. - if (nsContentUtils::IsSystemPrincipal(aPrincipal)) { - *aPermission = nsIPermissionManager::ALLOW_ACTION; - return NS_OK; - } - return CommonTestPermission(aPrincipal, aType, aPermission, true, false); } @@ -1022,15 +1006,6 @@ nsPermissionManager::TestPermissionFromPrincipal(nsIPrincipal* aPrincipal, const char* aType, uint32_t* aPermission) { - NS_ENSURE_ARG_POINTER(aPrincipal); - - // System principals do not have URI so we can't try to get - // retro-compatibility here. - if (nsContentUtils::IsSystemPrincipal(aPrincipal)) { - *aPermission = nsIPermissionManager::ALLOW_ACTION; - return NS_OK; - } - return CommonTestPermission(aPrincipal, aType, aPermission, false, true); } @@ -1101,6 +1076,11 @@ nsPermissionManager::CommonTestPermission(nsIPrincipal* aPrincipal, NS_ENSURE_ARG_POINTER(aPrincipal); NS_ENSURE_ARG_POINTER(aType); + if (nsContentUtils::IsSystemPrincipal(aPrincipal)) { + *aPermission = nsIPermissionManager::ALLOW_ACTION; + return NS_OK; + } + // set the default *aPermission = nsIPermissionManager::UNKNOWN_ACTION; From 156c63105741d6ab594ee3d4f26ef051f9d7fb19 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 17 Dec 2013 18:01:41 -0800 Subject: [PATCH 07/31] Bug 946815 - Handle nsIExpandedPrincipal instances in the permission manager API. r=baku --- extensions/cookie/nsPermissionManager.cpp | 55 +++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/extensions/cookie/nsPermissionManager.cpp b/extensions/cookie/nsPermissionManager.cpp index 980ffd29251..2cc6c94710c 100644 --- a/extensions/cookie/nsPermissionManager.cpp +++ b/extensions/cookie/nsPermissionManager.cpp @@ -208,6 +208,13 @@ public: NS_IMPL_ISUPPORTS1(AppClearDataObserver, nsIObserver) +static bool +IsExpandedPrincipal(nsIPrincipal* aPrincipal) +{ + nsCOMPtr ep = do_QueryInterface(aPrincipal); + return !!ep; +} + } // anonymous namespace //////////////////////////////////////////////////////////////////////////////// @@ -644,6 +651,11 @@ nsPermissionManager::AddFromPrincipal(nsIPrincipal* aPrincipal, return NS_OK; } + // Permissions may not be added to expanded principals. + if (IsExpandedPrincipal(aPrincipal)) { + return NS_ERROR_INVALID_ARG; + } + return AddInternal(aPrincipal, nsDependentCString(aType), aPermission, 0, aExpireType, aExpireTime, eNotify, eWriteToDB); } @@ -874,6 +886,11 @@ nsPermissionManager::RemoveFromPrincipal(nsIPrincipal* aPrincipal, return NS_OK; } + // Permissions may not be added to expanded principals. + if (IsExpandedPrincipal(aPrincipal)) { + return NS_ERROR_INVALID_ARG; + } + // AddInternal() handles removal, just let it do the work return AddInternal(aPrincipal, nsDependentCString(aType), @@ -1024,6 +1041,11 @@ nsPermissionManager::GetPermissionObject(nsIPrincipal* aPrincipal, return NS_OK; } + // Querying the permission object of an nsEP is non-sensical. + if (IsExpandedPrincipal(aPrincipal)) { + return NS_ERROR_INVALID_ARG; + } + nsAutoCString host; nsresult rv = GetHostForPrincipal(aPrincipal, host); NS_ENSURE_SUCCESS(rv, rv); @@ -1081,6 +1103,34 @@ nsPermissionManager::CommonTestPermission(nsIPrincipal* aPrincipal, return NS_OK; } + // For expanded principals, we want to iterate over the whitelist and see + // if the permission is granted for any of them. + nsCOMPtr ep = do_QueryInterface(aPrincipal); + if (ep) { + nsTArray>* whitelist; + nsresult rv = ep->GetWhiteList(&whitelist); + NS_ENSURE_SUCCESS(rv, rv); + + // Start with DENY_ACTION. If we get PROMPT_ACTION, keep going to see if + // we get ALLOW_ACTION from another principal. + *aPermission = nsIPermissionManager::DENY_ACTION; + for (size_t i = 0; i < whitelist->Length(); ++i) { + uint32_t perm; + rv = CommonTestPermission(whitelist->ElementAt(i), aType, &perm, aExactHostMatch, + aIncludingSession); + NS_ENSURE_SUCCESS(rv, rv); + if (perm == nsIPermissionManager::ALLOW_ACTION) { + *aPermission = perm; + return NS_OK; + } else if (perm == nsIPermissionManager::PROMPT_ACTION) { + // Store it, but keep going to see if we can do better. + *aPermission = perm; + } + } + + return NS_OK; + } + // set the default *aPermission = nsIPermissionManager::UNKNOWN_ACTION; @@ -1818,6 +1868,11 @@ nsPermissionManager::UpdateExpireTime(nsIPrincipal* aPrincipal, return NS_OK; } + // Setting the expire time of an nsEP is non-sensical. + if (IsExpandedPrincipal(aPrincipal)) { + return NS_ERROR_INVALID_ARG; + } + nsAutoCString host; nsresult rv = GetHostForPrincipal(aPrincipal, host); NS_ENSURE_SUCCESS(rv, rv); From f31dddba06a07034f50be9785f361c4da16c9a69 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 18 Dec 2013 11:25:10 +0900 Subject: [PATCH 08/31] Bug 951021 Android widget should set modifier state at dispatching events derived from WidgetInputEvent r=nchen --- widget/android/AndroidJavaWrappers.cpp | 52 +++++++++++++++++--------- widget/android/AndroidJavaWrappers.h | 5 +++ widget/android/nsWindow.cpp | 25 +++++++------ 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/widget/android/AndroidJavaWrappers.cpp b/widget/android/AndroidJavaWrappers.cpp index 85c77f9a2e8..263cdb10eb0 100644 --- a/widget/android/AndroidJavaWrappers.cpp +++ b/widget/android/AndroidJavaWrappers.cpp @@ -667,12 +667,8 @@ AndroidGeckoEvent::MakeTouchEvent(nsIWidget* widget) return event; } - event.modifiers = 0; + event.modifiers = DOMModifiers(); event.time = Time(); - event.InitBasicModifiers(IsCtrlPressed(), - IsAltPressed(), - IsShiftPressed(), - IsMetaPressed()); const nsIntPoint& offset = widget->WidgetToScreenOffset(); event.touches.SetCapacity(endIndex - startIndex); @@ -733,18 +729,7 @@ AndroidGeckoEvent::MakeMultiTouchInput(nsIWidget* widget) } MultiTouchInput event(type, Time(), 0); - if (IsCtrlPressed()) { - event.modifiers |= MODIFIER_CONTROL; - } - if (IsAltPressed()) { - event.modifiers |= MODIFIER_ALT; - } - if (IsShiftPressed()) { - event.modifiers |= MODIFIER_SHIFT; - } - if (IsMetaPressed()) { - event.modifiers |= MODIFIER_META; - } + event.modifiers = DOMModifiers(); if (type < 0) { // An event we don't know about @@ -802,7 +787,7 @@ AndroidGeckoEvent::MakeMouseEvent(nsIWidget* widget) if (msg != NS_MOUSE_MOVE) { event.clickCount = 1; } - event.modifiers = 0; + event.modifiers = DOMModifiers(); event.time = Time(); // We are dispatching this event directly into Gecko (as opposed to going @@ -816,6 +801,37 @@ AndroidGeckoEvent::MakeMouseEvent(nsIWidget* widget) return event; } +Modifiers +AndroidGeckoEvent::DOMModifiers() const +{ + Modifiers result = 0; + if (mMetaState & AMETA_ALT_MASK) { + result |= MODIFIER_ALT; + } + if (mMetaState & AMETA_SHIFT_MASK) { + result |= MODIFIER_SHIFT; + } + if (mMetaState & AMETA_CTRL_MASK) { + result |= MODIFIER_CONTROL; + } + if (mMetaState & AMETA_META_MASK) { + result |= MODIFIER_META; + } + if (mMetaState & AMETA_FUNCTION_ON) { + result |= MODIFIER_FN; + } + if (mMetaState & AMETA_CAPS_LOCK_ON) { + result |= MODIFIER_CAPSLOCK; + } + if (mMetaState & AMETA_NUM_LOCK_ON) { + result |= MODIFIER_NUMLOCK; + } + if (mMetaState & AMETA_SCROLL_LOCK_ON) { + result |= MODIFIER_SCROLLLOCK; + } + return result; +} + void AndroidPoint::Init(JNIEnv *jenv, jobject jobj) { diff --git a/widget/android/AndroidJavaWrappers.h b/widget/android/AndroidJavaWrappers.h index 661d6088254..b07b9549151 100644 --- a/widget/android/AndroidJavaWrappers.h +++ b/widget/android/AndroidJavaWrappers.h @@ -342,12 +342,16 @@ enum { AKEYCODE_KANA = 218, AKEYCODE_ASSIST = 219, + AMETA_FUNCTION_ON = 0x00000008, AMETA_CTRL_ON = 0x00001000, AMETA_CTRL_LEFT_ON = 0x00002000, AMETA_CTRL_RIGHT_ON = 0x00004000, AMETA_META_ON = 0x00010000, AMETA_META_LEFT_ON = 0x00020000, AMETA_META_RIGHT_ON = 0x00040000, + AMETA_CAPS_LOCK_ON = 0x00100000, + AMETA_NUM_LOCK_ON = 0x00200000, + AMETA_SCROLL_LOCK_ON = 0x00400000, AMETA_ALT_MASK = AMETA_ALT_LEFT_ON | AMETA_ALT_RIGHT_ON | AMETA_ALT_ON, AMETA_CTRL_MASK = AMETA_CTRL_LEFT_ON | AMETA_CTRL_RIGHT_ON | AMETA_CTRL_ON, @@ -504,6 +508,7 @@ public: int KeyCode() { return mKeyCode; } int MetaState() { return mMetaState; } uint32_t DomKeyLocation() { return mDomKeyLocation; } + Modifiers DOMModifiers() const; bool IsAltPressed() const { return (mMetaState & AMETA_ALT_MASK) != 0; } bool IsShiftPressed() const { return (mMetaState & AMETA_SHIFT_MASK) != 0; } bool IsCtrlPressed() const { return (mMetaState & AMETA_CTRL_MASK) != 0; } diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp index 7f3fa97ce55..c31b4518fe8 100644 --- a/widget/android/nsWindow.cpp +++ b/widget/android/nsWindow.cpp @@ -1263,7 +1263,7 @@ nsWindow::DispatchMotionEvent(WidgetInputEvent &event, AndroidGeckoEvent *ae, { nsIntPoint offset = WidgetToScreenOffset(); - event.modifiers = 0; + event.modifiers = ae->DOMModifiers(); event.time = ae->Time(); // XXX possibly bound the range of event.refPoint here. @@ -1609,16 +1609,19 @@ nsWindow::InitKeyEvent(WidgetKeyboardEvent& event, AndroidGeckoEvent& key, event.pluginEvent = pluginEvent; } - if (event.message != NS_KEY_PRESS || - !key.UnicodeChar() || !key.BaseUnicodeChar() || - key.UnicodeChar() == key.BaseUnicodeChar()) { - // For keypress, if the unicode char already has modifiers applied, we - // don't specify extra modifiers. If UnicodeChar() != BaseUnicodeChar() - // it means UnicodeChar() already has modifiers applied. - event.InitBasicModifiers(gMenu || key.IsCtrlPressed(), - key.IsAltPressed(), - key.IsShiftPressed(), - key.IsMetaPressed()); + event.modifiers = key.DOMModifiers(); + if (gMenu) { + event.modifiers |= MODIFIER_CONTROL; + } + // For keypress, if the unicode char already has modifiers applied, we + // don't specify extra modifiers. If UnicodeChar() != BaseUnicodeChar() + // it means UnicodeChar() already has modifiers applied. + // Note that on Android 4.x, Alt modifier isn't set when the key input + // causes text input even while right Alt key is pressed. However, this + // is necessary for Android 2.3 compatibility. + if (event.message == NS_KEY_PRESS && + key.UnicodeChar() && key.UnicodeChar() != key.BaseUnicodeChar()) { + event.modifiers &= ~(MODIFIER_ALT | MODIFIER_CONTROL | MODIFIER_META); } event.mIsRepeat = From 43049459ca5717e8b8ab6d273f90934e6888723c Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Tue, 17 Dec 2013 22:10:29 -0500 Subject: [PATCH 09/31] Bug 945667 - Disable browser_findbar.js on Linux due to relying on browser_aboutHome.js. --- toolkit/content/tests/browser/browser.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/toolkit/content/tests/browser/browser.ini b/toolkit/content/tests/browser/browser.ini index f2669a4efc0..7996e69d04d 100644 --- a/toolkit/content/tests/browser/browser.ini +++ b/toolkit/content/tests/browser/browser.ini @@ -5,6 +5,7 @@ [browser_bug594509.js] [browser_default_image_filename.js] [browser_findbar.js] +skip-if = os == "linux" # Bug 945667 [browser_input_file_tooltips.js] [browser_keyevents_during_autoscrolling.js] [browser_save_resend_postdata.js] From 197a13c1ee83d39e72d34ae3241d0599654368d6 Mon Sep 17 00:00:00 2001 From: Andrew McCreight Date: Tue, 17 Dec 2013 19:29:56 -0800 Subject: [PATCH 10/31] Bug 937818, part 0 - Root newly created objects in Bind across ADDREF. r=bz --- dom/bindings/Codegen.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 20487ddc01a..280f7f3773c 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -2134,18 +2134,9 @@ class CGConstructorEnabledViaFunc(CGAbstractMethod): return " return %s(cx, obj);" % func[0] def CreateBindingJSObject(descriptor, properties, parent): - # When we have unforgeable properties, we're going to define them - # on our object, so we have to root it when we create it, so it - # won't suddenly die while defining the unforgeables. Similarly, - # if we have members in slots we'll have to call the getters which - # could also GC. - needRoot = (properties.unforgeableAttrs.hasNonChromeOnly() or - properties.unforgeableAttrs.hasChromeOnly() or - descriptor.interface.hasMembersInSlots()) - if needRoot: - objDecl = " JS::Rooted obj(aCx);\n" - else: - objDecl = " JSObject *obj;\n" + # We don't always need to root obj, but there are a variety + # of cases where we do, so for simplicity, just always root it. + objDecl = " JS::Rooted obj(aCx);\n" if descriptor.proxy: create = """ JS::Rooted proxyPrivateVal(aCx, JS::PrivateValue(aObject)); obj = NewProxyObject(aCx, DOMProxyHandler::getInstance(), From 5bbea5ab96c67496d7cc356ca699d47bcfbd46c0 Mon Sep 17 00:00:00 2001 From: Andrew McCreight Date: Tue, 17 Dec 2013 19:29:57 -0800 Subject: [PATCH 11/31] Bug 937818, part 1 - Add objects to the purple buffer on AddRef. r=smaug ICC uses this to track objects that have been AddRef'd during ICC graph building. For those objects, we may not have the proper information for them, so treat them as live. --- js/xpconnect/src/XPCWrappedJS.cpp | 5 +++-- xpcom/base/nsAgg.h | 2 +- xpcom/base/nsCycleCollector.cpp | 25 ++++++++++++++++++++++++- xpcom/glue/nsISupportsImpl.h | 27 ++++++++++++++++++++++----- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/js/xpconnect/src/XPCWrappedJS.cpp b/js/xpconnect/src/XPCWrappedJS.cpp index 3480ef0f099..19e76142dd1 100644 --- a/js/xpconnect/src/XPCWrappedJS.cpp +++ b/js/xpconnect/src/XPCWrappedJS.cpp @@ -225,7 +225,8 @@ nsXPCWrappedJS::AddRef(void) MOZ_CRASH(); MOZ_ASSERT(int32_t(mRefCnt) >= 0, "illegal refcnt"); - nsrefcnt cnt = mRefCnt.incr(); + nsISupports *base = NS_CYCLE_COLLECTION_CLASSNAME(nsXPCWrappedJS)::Upcast(this); + nsrefcnt cnt = mRefCnt.incr(base); NS_LOG_ADDREF(this, cnt, "nsXPCWrappedJS", sizeof(*this)); if (2 == cnt && IsValid()) { @@ -254,7 +255,7 @@ nsXPCWrappedJS::Release(void) mRefCnt.stabilizeForDeletion(); DeleteCycleCollectable(); } else { - mRefCnt.incr(); + mRefCnt.incr(base); Destroy(); mRefCnt.decr(base); } diff --git a/xpcom/base/nsAgg.h b/xpcom/base/nsAgg.h index bd1a36d52cc..487becb52a4 100644 --- a/xpcom/base/nsAgg.h +++ b/xpcom/base/nsAgg.h @@ -144,7 +144,7 @@ _class::Internal::AddRef(void) \ _class* agg = NS_CYCLE_COLLECTION_CLASSNAME(_class)::Downcast(this); \ MOZ_ASSERT(int32_t(agg->mRefCnt) >= 0, "illegal refcnt"); \ NS_ASSERT_OWNINGTHREAD_AGGREGATE(agg, _class); \ - nsrefcnt count = agg->mRefCnt.incr(); \ + nsrefcnt count = agg->mRefCnt.incr(this); \ NS_LOG_ADDREF(this, count, #_class, sizeof(*agg)); \ return count; \ } \ diff --git a/xpcom/base/nsCycleCollector.cpp b/xpcom/base/nsCycleCollector.cpp index 9448093846b..4c5d0ae78ea 100644 --- a/xpcom/base/nsCycleCollector.cpp +++ b/xpcom/base/nsCycleCollector.cpp @@ -71,7 +71,30 @@ // // The second problem is that objects in the graph can be changed, say by // being addrefed or released, or by having a field updated, after the object -// has been added to the graph. This will be addressed in bug 937818. +// has been added to the graph. The problem is that ICC can miss a newly +// created reference to an object, and end up unlinking an object that is +// actually alive. +// +// The basic idea of the solution, from "An on-the-fly Reference Counting +// Garbage Collector for Java" by Levanoni and Petrank, is to notice if an +// object has had an additional reference to it created during the collection, +// and if so, don't collect it during the current collection. This avoids having +// to rerun the scan as in Bacon & Rajan 2001. +// +// For cycle collected C++ objects, we modify AddRef to place the object in +// the purple buffer, in addition to Release. Then, in the CC, we treat any +// objects in the purple buffer as being alive, after graph building has +// completed. Because they are in the purple buffer, they will be suspected +// in the next CC, so there's no danger of leaks. This is imprecise, because +// we will treat as live an object that has been Released but not AddRefed +// during graph building, but that's probably rare enough that the additional +// bookkeeping overhead is not worthwhile. +// +// For JS objects, the cycle collector is only looking at gray objects. If a +// gray object is touched during ICC, it will be made black by UnmarkGray. +// Thus, if a JS object has become black during the ICC, we treat it as live. +// Merged JS zones have to be handled specially: we scan all zone globals. +// If any are black, we treat the zone as being black. // Safety diff --git a/xpcom/glue/nsISupportsImpl.h b/xpcom/glue/nsISupportsImpl.h index 27abc32a1cb..01343b3e60b 100644 --- a/xpcom/glue/nsISupportsImpl.h +++ b/xpcom/glue/nsISupportsImpl.h @@ -92,10 +92,23 @@ public: { } - MOZ_ALWAYS_INLINE uintptr_t incr() + MOZ_ALWAYS_INLINE uintptr_t incr(nsISupports *owner) + { + return incr(owner, nullptr); + } + + MOZ_ALWAYS_INLINE uintptr_t incr(void *owner, nsCycleCollectionParticipant *p) { mRefCntAndFlags += NS_REFCOUNT_CHANGE; mRefCntAndFlags &= ~NS_IS_PURPLE; + // For incremental cycle collection, use the purple buffer to track objects + // that have been AddRef'd. + if (!IsInPurpleBuffer()) { + mRefCntAndFlags |= NS_IN_PURPLE_BUFFER; + // Refcount isn't zero, so Suspect won't delete anything. + MOZ_ASSERT(get() > 0); + NS_CycleCollectorSuspect3(owner, p, this, nullptr); + } return NS_REFCOUNT_VALUE(mRefCntAndFlags); } @@ -265,7 +278,9 @@ public: #define NS_IMPL_CC_NATIVE_ADDREF_BODY(_class) \ MOZ_ASSERT(int32_t(mRefCnt) >= 0, "illegal refcnt"); \ NS_ASSERT_OWNINGTHREAD(_class); \ - nsrefcnt count = mRefCnt.incr(); \ + nsrefcnt count = \ + mRefCnt.incr(static_cast(this), \ + _class::NS_CYCLE_COLLECTION_INNERCLASS::GetParticipant()); \ NS_LOG_ADDREF(this, count, #_class, sizeof(*this)); \ return count; @@ -296,7 +311,8 @@ NS_METHOD_(nsrefcnt) _class::Release(void) &shouldDelete); \ NS_LOG_RELEASE(this, count, #_class); \ if (count == 0) { \ - mRefCnt.incr(); \ + mRefCnt.incr(static_cast(this), \ + _class::NS_CYCLE_COLLECTION_INNERCLASS::GetParticipant()); \ _last; \ mRefCnt.decr(static_cast(this), \ _class::NS_CYCLE_COLLECTION_INNERCLASS::GetParticipant()); \ @@ -502,7 +518,8 @@ NS_IMETHODIMP_(nsrefcnt) _class::AddRef(void) \ { \ MOZ_ASSERT(int32_t(mRefCnt) >= 0, "illegal refcnt"); \ NS_ASSERT_OWNINGTHREAD(_class); \ - nsrefcnt count = mRefCnt.incr(); \ + nsISupports *base = NS_CYCLE_COLLECTION_CLASSNAME(_class)::Upcast(this); \ + nsrefcnt count = mRefCnt.incr(base); \ NS_LOG_ADDREF(this, count, #_class, sizeof(*this)); \ return count; \ } @@ -537,7 +554,7 @@ NS_IMETHODIMP_(nsrefcnt) _class::Release(void) \ nsrefcnt count = mRefCnt.decr(base, &shouldDelete); \ NS_LOG_RELEASE(this, count, #_class); \ if (count == 0) { \ - mRefCnt.incr(); \ + mRefCnt.incr(base); \ _last; \ mRefCnt.decr(base); \ if (shouldDelete) { \ From dd9f3a376e4feeb909e6346ba69ce1c9badeffdb Mon Sep 17 00:00:00 2001 From: Andrew McCreight Date: Tue, 17 Dec 2013 19:29:57 -0800 Subject: [PATCH 12/31] Bug 937818, part 2 - Add js::ZoneGlobalsAreAllGray. r=jonco If all globals in a zone are gray, then all live objects in that zone should also be gray. --- js/src/jsfriendapi.cpp | 11 +++++++++++ js/src/jsfriendapi.h | 3 +++ 2 files changed, 14 insertions(+) diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp index e62b9dec378..ff0a433c803 100644 --- a/js/src/jsfriendapi.cpp +++ b/js/src/jsfriendapi.cpp @@ -647,6 +647,17 @@ js::AreGCGrayBitsValid(JSRuntime *rt) return rt->gcGrayBitsValid; } +JS_FRIEND_API(bool) +js::ZoneGlobalsAreAllGray(JS::Zone *zone) +{ + for (CompartmentsInZoneIter comp(zone); !comp.done(); comp.next()) { + JSObject *obj = comp->maybeGlobal(); + if (!obj || !JS::GCThingIsMarkedGray(obj)) + return false; + } + return true; +} + JS_FRIEND_API(JSGCTraceKind) js::GCThingTraceKind(void *thing) { diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index 6e41232400c..4c5d113d2d3 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -339,6 +339,9 @@ TraceWeakMaps(WeakMapTracer *trc); extern JS_FRIEND_API(bool) AreGCGrayBitsValid(JSRuntime *rt); +extern JS_FRIEND_API(bool) +ZoneGlobalsAreAllGray(JS::Zone *zone); + typedef void (*GCThingCallback)(void *closure, void *gcthing); From d8b513724e51b58967575b6ec0a827cb26a2a640 Mon Sep 17 00:00:00 2001 From: Andrew McCreight Date: Tue, 17 Dec 2013 19:29:57 -0800 Subject: [PATCH 13/31] Bug 937818, part 3 - Add ScanIncrementalRoots(). r=smaug Any object that has been stored away somewhere in the middle of incremental graph building must be treated as live, because we can't trust that the CC graph has accurate information about it. If such an object is truly garbage, we'll unlink it in the next cycle collection instead. --- xpcom/base/nsCycleCollector.cpp | 130 ++++++++++++++++++++++++++++++-- 1 file changed, 125 insertions(+), 5 deletions(-) diff --git a/xpcom/base/nsCycleCollector.cpp b/xpcom/base/nsCycleCollector.cpp index 4c5d0ae78ea..76302358b8d 100644 --- a/xpcom/base/nsCycleCollector.cpp +++ b/xpcom/base/nsCycleCollector.cpp @@ -1178,7 +1178,8 @@ private: void BeginCollection(ccType aCCType, nsICycleCollectorListener *aManualListener); void MarkRoots(SliceBudget &aBudget); - void ScanRoots(); + void ScanRoots(bool aFullySynchGraphBuild); + void ScanIncrementalRoots(); void ScanWeakMaps(); // returns whether anything was collected @@ -2303,6 +2304,16 @@ nsCycleCollector::ForgetSkippable(bool aRemoveChildlessNodes, bool aAsyncSnowWhiteFreeing) { CheckThreadSafety(); + + // If we remove things from the purple buffer during graph building, we may + // lose track of an object that was mutated during graph building. This should + // only happen when somebody calls nsJSContext::CycleCollectNow explicitly + // requesting extra forget skippable calls, during an incremental collection. + // See bug 950949 for fixing this so we actually run the forgetSkippable calls. + if (mIncrementalPhase != IdlePhase) { + return; + } + if (mJSRuntime) { mJSRuntime->PrepareForForgetSkippable(); } @@ -2474,28 +2485,138 @@ nsCycleCollector::ScanWeakMaps() } } +// Flood black from any objects in the purple buffer that are in the CC graph. +class PurpleScanBlackVisitor +{ +public: + PurpleScanBlackVisitor(GCGraph &aGraph, uint32_t &aCount, bool &aFailed) + : mGraph(aGraph), mCount(aCount), mFailed(aFailed) + { + } + + void + Visit(nsPurpleBuffer &aBuffer, nsPurpleBufferEntry *aEntry) + { + MOZ_ASSERT(aEntry->mObject, "Entries with null mObject shouldn't be in the purple buffer."); + MOZ_ASSERT(aEntry->mRefCnt->get() != 0, "Snow-white objects shouldn't be in the purple buffer."); + + void *obj = aEntry->mObject; + if (!aEntry->mParticipant) { + obj = CanonicalizeXPCOMParticipant(static_cast(obj)); + MOZ_ASSERT(obj, "Don't add objects that don't participate in collection!"); + } + + PtrInfo *pi = mGraph.FindNode(obj); + if (!pi) { + return; + } + MOZ_ASSERT(pi->mParticipant, "No dead objects should be in the purple buffer."); + if (pi->mColor == black) { + return; + } + GraphWalker(ScanBlackVisitor(mCount, mFailed)).Walk(pi); + } + +private: + GCGraph &mGraph; + uint32_t &mCount; + bool &mFailed; +}; + +// Objects that have been stored somewhere since the start of incremental graph building must +// be treated as live for this cycle collection, because we may not have accurate information +// about who holds references to them. void -nsCycleCollector::ScanRoots() +nsCycleCollector::ScanIncrementalRoots() { TimeLog timeLog; + + // Reference counted objects: + // We cleared the purple buffer at the start of the current ICC, so if a + // refcounted object is purple, it may have been AddRef'd during the current + // ICC. (It may also have only been released.) If that is the case, we cannot + // be sure that the set of things pointing to the object in the CC graph + // is accurate. Therefore, for safety, we treat any purple objects as being + // live during the current CC. We don't remove anything from the purple + // buffer here, so these objects will be suspected and freed in the next CC + // if they are garbage. + bool failed = false; + PurpleScanBlackVisitor purpleScanBlackVisitor(mGraph, mWhiteNodeCount, failed); + mPurpleBuf.VisitEntries(purpleScanBlackVisitor); + timeLog.Checkpoint("ScanIncrementalRoots::fix purple"); + + // Garbage collected objects: + // If a GCed object was added to the graph with a refcount of zero, and is + // now marked black by the GC, it was probably gray before and was exposed + // to active JS, so it may have been stored somewhere, so it needs to be + // treated as live. + if (mJSRuntime) { + nsCycleCollectionParticipant *jsParticipant = mJSRuntime->GCThingParticipant(); + nsCycleCollectionParticipant *zoneParticipant = mJSRuntime->ZoneParticipant(); + NodePool::Enumerator etor(mGraph.mNodes); + + while (!etor.IsDone()) { + PtrInfo *pi = etor.GetNext(); + + if (pi->mRefCount != 0 || pi->mColor == black) { + continue; + } + + if (pi->mParticipant == jsParticipant) { + if (xpc_GCThingIsGrayCCThing(pi->mPointer)) { + continue; + } + } else if (pi->mParticipant == zoneParticipant) { + JS::Zone *zone = static_cast(pi->mPointer); + if (js::ZoneGlobalsAreAllGray(zone)) { + continue; + } + } else { + MOZ_ASSERT(false, "Non-JS thing with 0 refcount? Treating as live."); + } + + GraphWalker(ScanBlackVisitor(mWhiteNodeCount, failed)).Walk(pi); + } + + timeLog.Checkpoint("ScanIncrementalRoots::fix JS"); + } + + if (failed) { + NS_ASSERTION(false, "Ran out of memory in ScanIncrementalRoots"); + CC_TELEMETRY(_OOM, true); + } +} + +void +nsCycleCollector::ScanRoots(bool aFullySynchGraphBuild) +{ AutoRestore ar(mScanInProgress); MOZ_ASSERT(!mScanInProgress); mScanInProgress = true; mWhiteNodeCount = 0; MOZ_ASSERT(mIncrementalPhase == ScanAndCollectWhitePhase); + if (!aFullySynchGraphBuild) { + ScanIncrementalRoots(); + } + + TimeLog timeLog; + // On the assumption that most nodes will be black, it's // probably faster to use a GraphWalker than a // NodePool::Enumerator. bool failed = false; GraphWalker(scanVisitor(mWhiteNodeCount, failed)).WalkFromRoots(mGraph); + timeLog.Checkpoint("ScanRoots::WalkFromRoots"); if (failed) { NS_ASSERTION(false, "Ran out of memory in ScanRoots"); CC_TELEMETRY(_OOM, true); } + // Scanning weak maps must be done last. ScanWeakMaps(); + timeLog.Checkpoint("ScanRoots::ScanWeakMaps"); if (mListener) { mListener->BeginResults(); @@ -2526,9 +2647,8 @@ nsCycleCollector::ScanRoots() mListener->End(); mListener = nullptr; + timeLog.Checkpoint("ScanRoots::listener"); } - - timeLog.Checkpoint("ScanRoots()"); } @@ -2891,7 +3011,7 @@ nsCycleCollector::Collect(ccType aCCType, // promoted to a strong reference after ScanRoots has finished. // See bug 926533. PrintPhase("ScanRoots"); - ScanRoots(); + ScanRoots(startedIdle); PrintPhase("CollectWhite"); collectedAny = CollectWhite(); break; From a8a41fffec5e23b9d31c86a4b228e4d32c4ad879 Mon Sep 17 00:00:00 2001 From: Andrew McCreight Date: Tue, 17 Dec 2013 19:29:57 -0800 Subject: [PATCH 14/31] Bug 937818, part 4 - Exceeded refcount nodes should already be black. r=smaug Due to graph mutation during an incremental cycle collection, objects in the CC graph may end up with more things pointing to them than they have a ref count. However, these objects should never become garbage. --- xpcom/base/nsCycleCollector.cpp | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/xpcom/base/nsCycleCollector.cpp b/xpcom/base/nsCycleCollector.cpp index 76302358b8d..70deae724fe 100644 --- a/xpcom/base/nsCycleCollector.cpp +++ b/xpcom/base/nsCycleCollector.cpp @@ -2406,8 +2406,9 @@ private: struct scanVisitor { - scanVisitor(uint32_t &aWhiteNodeCount, bool &aFailed) - : mWhiteNodeCount(aWhiteNodeCount), mFailed(aFailed) + scanVisitor(uint32_t &aWhiteNodeCount, bool &aFailed, bool aWasIncremental) + : mWhiteNodeCount(aWhiteNodeCount), mFailed(aFailed), + mWasIncremental(aWasIncremental) { } @@ -2418,8 +2419,16 @@ struct scanVisitor MOZ_NEVER_INLINE void VisitNode(PtrInfo *pi) { - if (pi->mInternalRefs > pi->mRefCount && pi->mRefCount > 0) - Fault("traversed refs exceed refcount", pi); + if (pi->mInternalRefs > pi->mRefCount && pi->mRefCount > 0) { + // If we found more references to an object than its ref count, then + // the object should have already been marked as an incremental + // root. Note that this is imprecise, because pi could have been + // marked black for other reasons. Always fault if we weren't + // incremental, as there were no incremental roots in that case. + if (!mWasIncremental || pi->mColor != black) { + Fault("traversed refs exceed refcount", pi); + } + } if (pi->mInternalRefs == pi->mRefCount || pi->mRefCount == 0) { pi->mColor = white; @@ -2438,6 +2447,7 @@ struct scanVisitor private: uint32_t &mWhiteNodeCount; bool &mFailed; + bool mWasIncremental; }; // Iterate over the WeakMaps. If we mark anything while iterating @@ -2606,7 +2616,8 @@ nsCycleCollector::ScanRoots(bool aFullySynchGraphBuild) // probably faster to use a GraphWalker than a // NodePool::Enumerator. bool failed = false; - GraphWalker(scanVisitor(mWhiteNodeCount, failed)).WalkFromRoots(mGraph); + scanVisitor sv(mWhiteNodeCount, failed, !aFullySynchGraphBuild); + GraphWalker(sv).WalkFromRoots(mGraph); timeLog.Checkpoint("ScanRoots::WalkFromRoots"); if (failed) { From 02a858e731267691943c3982bf96c38eb0702629 Mon Sep 17 00:00:00 2001 From: Chris Pearce Date: Wed, 18 Dec 2013 16:59:11 +1300 Subject: [PATCH 15/31] Bug 938107 - Wait for media state machine thread to shutdown during XPCOM shutdown before returning. r=roc Add a MediaShutdownManager and have that as the only xpcom-shutdown observer. This then shutsdown the MediaDecoders, and blocks waiting for the media state machine's shared thread to complete shutdown before exiting from the xpcom-shutdown observer. This ensures that the MediaDecoder infrastructure does not use XPCOM on any thread after XPCOM has shutdown, which is a logical error. --- content/media/MediaDecoder.cpp | 6 +- content/media/MediaDecoderStateMachine.cpp | 14 +- content/media/MediaShutdownManager.cpp | 238 +++++++++++++++++++++ content/media/MediaShutdownManager.h | 169 +++++++++++++++ content/media/moz.build | 1 + 5 files changed, 418 insertions(+), 10 deletions(-) create mode 100644 content/media/MediaShutdownManager.cpp create mode 100644 content/media/MediaShutdownManager.h diff --git a/content/media/MediaDecoder.cpp b/content/media/MediaDecoder.cpp index 9ca3b7ed2ff..cc6c13b66f6 100644 --- a/content/media/MediaDecoder.cpp +++ b/content/media/MediaDecoder.cpp @@ -13,7 +13,6 @@ #include "VideoUtils.h" #include "MediaDecoderStateMachine.h" #include "mozilla/dom/TimeRanges.h" -#include "nsContentUtils.h" #include "ImageContainer.h" #include "MediaResource.h" #include "nsError.h" @@ -23,6 +22,7 @@ #include "nsComponentManagerUtils.h" #include "nsITimer.h" #include +#include "MediaShutdownManager.h" #ifdef MOZ_WMF #include "WMFDecoder.h" @@ -444,7 +444,7 @@ bool MediaDecoder::Init(MediaDecoderOwner* aOwner) MOZ_ASSERT(NS_IsMainThread()); mOwner = aOwner; mVideoFrameContainer = aOwner->GetVideoFrameContainer(); - nsContentUtils::RegisterShutdownObserver(this); + MediaShutdownManager::Instance().Register(this); return true; } @@ -480,7 +480,7 @@ void MediaDecoder::Shutdown() StopProgress(); mOwner = nullptr; - nsContentUtils::UnregisterShutdownObserver(this); + MediaShutdownManager::Instance().Unregister(this); } MediaDecoder::~MediaDecoder() diff --git a/content/media/MediaDecoderStateMachine.cpp b/content/media/MediaDecoderStateMachine.cpp index 424eec00ad1..4439405c160 100644 --- a/content/media/MediaDecoderStateMachine.cpp +++ b/content/media/MediaDecoderStateMachine.cpp @@ -26,6 +26,8 @@ #include "ImageContainer.h" #include "nsComponentManagerUtils.h" #include "nsITimer.h" +#include "nsContentUtils.h" +#include "MediaShutdownManager.h" #include "prenv.h" #include "mozilla/Preferences.h" @@ -178,7 +180,7 @@ public: { ReentrantMonitorAutoEnter mon(mMonitor); NS_ASSERTION(mStateMachineThread, "Should have non-null state machine thread!"); - return mStateMachineThread; + return mStateMachineThread->GetThread(); } // Requests that a decode thread be created for aStateMachine. The thread @@ -234,7 +236,7 @@ private: // Global state machine thread. Write on the main thread // only, read from the decoder threads. Synchronized via // the mMonitor. - nsIThread* mStateMachineThread; + nsRefPtr mStateMachineThread; // Queue of state machines waiting for decode threads. Entries at the front // get their threads first. @@ -258,7 +260,8 @@ void StateMachineTracker::EnsureGlobalStateMachine() ReentrantMonitorAutoEnter mon(mMonitor); if (mStateMachineCount == 0) { NS_ASSERTION(!mStateMachineThread, "Should have null state machine thread!"); - DebugOnly rv = NS_NewNamedThread("Media State", &mStateMachineThread); + mStateMachineThread = new StateMachineThread(); + DebugOnly rv = mStateMachineThread->Init(); NS_ABORT_IF_FALSE(NS_SUCCEEDED(rv), "Can't create media state machine thread"); } mStateMachineCount++; @@ -291,11 +294,8 @@ void StateMachineTracker::CleanupGlobalStateMachine() NS_ASSERTION(mPending.GetSize() == 0, "Shouldn't all requests be handled by now?"); { ReentrantMonitorAutoEnter mon(mMonitor); - nsCOMPtr event = new ShutdownThreadEvent(mStateMachineThread); - NS_RELEASE(mStateMachineThread); + mStateMachineThread->Shutdown(); mStateMachineThread = nullptr; - NS_DispatchToMainThread(event); - NS_ASSERTION(mDecodeThreadCount == 0, "Decode thread count must be zero."); sInstance = nullptr; } diff --git a/content/media/MediaShutdownManager.cpp b/content/media/MediaShutdownManager.cpp new file mode 100644 index 00000000000..e98ead85e76 --- /dev/null +++ b/content/media/MediaShutdownManager.cpp @@ -0,0 +1,238 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim:set ts=2 sw=2 sts=2 et cindent: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "MediaShutdownManager.h" +#include "nsContentUtils.h" +#include "mozilla/StaticPtr.h" +#include "mozilla/ClearOnShutdown.h" +#include "MediaDecoder.h" + +namespace mozilla { + +StateMachineThread::StateMachineThread() +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_COUNT_CTOR(StateMachineThread); +} + +StateMachineThread::~StateMachineThread() +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_COUNT_DTOR(StateMachineThread); +} + +void +StateMachineThread::Shutdown() +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(mThread); + if (mThread) { + nsCOMPtr event = + NS_NewRunnableMethod(this, &StateMachineThread::ShutdownThread); + NS_DispatchToMainThread(event, NS_DISPATCH_NORMAL); + } +} + +void +StateMachineThread::ShutdownThread() +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(mThread); + mThread->Shutdown(); + mThread = nullptr; + MediaShutdownManager::Instance().Unregister(this); +} + +nsresult +StateMachineThread::Init() +{ + MOZ_ASSERT(NS_IsMainThread()); + nsresult rv = NS_NewNamedThread("Media State", getter_AddRefs(mThread)); + NS_ENSURE_SUCCESS(rv, rv); + MediaShutdownManager::Instance().Register(this); + return NS_OK; +} + +nsIThread* +StateMachineThread::GetThread() +{ + MOZ_ASSERT(mThread); + return mThread; +} + +void +StateMachineThread::SpinUntilShutdownComplete() +{ + MOZ_ASSERT(NS_IsMainThread()); + while (mThread) { + bool processed = false; + nsresult rv = NS_GetCurrentThread()->ProcessNextEvent(true, &processed); + if (NS_FAILED(rv)) { + NS_WARNING("Failed to spin main thread while awaiting media shutdown"); + break; + } + } +} + +NS_IMPL_ISUPPORTS1(MediaShutdownManager, nsIObserver) + +MediaShutdownManager::MediaShutdownManager() + : mIsObservingShutdown(false), + mIsDoingXPCOMShutDown(false) +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_COUNT_CTOR(MediaShutdownManager); +} + +MediaShutdownManager::~MediaShutdownManager() +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_COUNT_DTOR(MediaShutdownManager); +} + +// Note that we don't use ClearOnShutdown() on this StaticRefPtr, as that +// may interfere with our shutdown listener. +StaticRefPtr MediaShutdownManager::sInstance; + +MediaShutdownManager& +MediaShutdownManager::Instance() +{ + MOZ_ASSERT(NS_IsMainThread()); + if (!sInstance) { + sInstance = new MediaShutdownManager(); + } + return *sInstance; +} + +void +MediaShutdownManager::EnsureCorrectShutdownObserverState() +{ + MOZ_ASSERT(!mIsDoingXPCOMShutDown); + bool needShutdownObserver = (mDecoders.Count() > 0) || + (mStateMachineThreads.Count() > 0); + if (needShutdownObserver != mIsObservingShutdown) { + mIsObservingShutdown = needShutdownObserver; + if (mIsObservingShutdown) { + nsContentUtils::RegisterShutdownObserver(this); + } else { + nsContentUtils::UnregisterShutdownObserver(this); + // Clear our singleton reference. This will probably delete + // this instance, so don't deref |this| clearing sInstance. + sInstance = nullptr; + } + } +} + +void +MediaShutdownManager::Register(MediaDecoder* aDecoder) +{ + MOZ_ASSERT(NS_IsMainThread()); + // Don't call Register() after you've Unregistered() all the decoders, + // that's not going to work. + MOZ_ASSERT(!mDecoders.Contains(aDecoder)); + mDecoders.PutEntry(aDecoder); + MOZ_ASSERT(mDecoders.Contains(aDecoder)); + MOZ_ASSERT(mDecoders.Count() > 0); + EnsureCorrectShutdownObserverState(); +} + +void +MediaShutdownManager::Unregister(MediaDecoder* aDecoder) +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(mDecoders.Contains(aDecoder)); + if (!mIsDoingXPCOMShutDown) { + mDecoders.RemoveEntry(aDecoder); + EnsureCorrectShutdownObserverState(); + } +} + +NS_IMETHODIMP +MediaShutdownManager::Observe(nsISupports *aSubjet, + const char *aTopic, + const PRUnichar *someData) +{ + MOZ_ASSERT(NS_IsMainThread()); + if (strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) { + Shutdown(); + } + return NS_OK; +} + +void +MediaShutdownManager::Register(StateMachineThread* aThread) +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(!mStateMachineThreads.Contains(aThread)); + mStateMachineThreads.PutEntry(aThread); + MOZ_ASSERT(mStateMachineThreads.Contains(aThread)); + MOZ_ASSERT(mStateMachineThreads.Count() > 0); + EnsureCorrectShutdownObserverState(); +} + +void +MediaShutdownManager::Unregister(StateMachineThread* aThread) +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(mStateMachineThreads.Contains(aThread)); + if (!mIsDoingXPCOMShutDown) { + mStateMachineThreads.RemoveEntry(aThread); + EnsureCorrectShutdownObserverState(); + } +} + +static PLDHashOperator +ShutdownMediaDecoder(nsRefPtrHashKey* aEntry, void*) +{ + aEntry->GetKey()->Shutdown(); + return PL_DHASH_REMOVE; +} + +static PLDHashOperator +JoinStateMachineThreads(nsRefPtrHashKey* aEntry, void*) +{ + // Hold a strong reference to the StateMachineThread, so that if it + // is Unregistered() and the hashtable's owning reference is cleared, + // it won't be destroyed while we're spinning here. + RefPtr thread = aEntry->GetKey(); + thread->SpinUntilShutdownComplete(); + return PL_DHASH_REMOVE; +} + +void +MediaShutdownManager::Shutdown() +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(sInstance); + + // Mark that we're shutting down, so that Unregister(*) calls don't remove + // hashtable entries. If Unregsiter(*) was to remove from the hash table, + // the iterations over the hashtables below would be disrupted. + mIsDoingXPCOMShutDown = true; + + // Iterate over the decoders and shut them down, and remove them from the + // hashtable. + mDecoders.EnumerateEntries(ShutdownMediaDecoder, nullptr); + + // Iterate over the StateMachineThreads and wait for them to have finished + // shutting down, and remove them from the hashtable. Once all the decoders + // have shutdown the active state machine thread will naturally shutdown + // asynchronously. We may also have another state machine thread active, + // if construction and shutdown of the state machine thread has interleaved. + mStateMachineThreads.EnumerateEntries(JoinStateMachineThreads, nullptr); + + // Remove the MediaShutdownManager instance from the shutdown observer + // list. + nsContentUtils::UnregisterShutdownObserver(this); + + // Clear the singleton instance. The only remaining reference should be the + // reference that the observer service used to call us with. The + // MediaShutdownManager will be deleted once the observer service cleans + // up after it finishes its notifications. + sInstance = nullptr; +} + +} // namespace mozilla diff --git a/content/media/MediaShutdownManager.h b/content/media/MediaShutdownManager.h new file mode 100644 index 00000000000..ceea4d76da8 --- /dev/null +++ b/content/media/MediaShutdownManager.h @@ -0,0 +1,169 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim:set ts=2 sw=2 sts=2 et cindent: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#if !defined(MediaShutdownManager_h_) +#define MediaShutdownManager_h_ + +#include "nsIObserver.h" +#include "mozilla/Monitor.h" +#include "mozilla/RefPtr.h" +#include "mozilla/StaticPtr.h" +#include "nsIThread.h" +#include "nsCOMPtr.h" +#include "nsTHashtable.h" +#include "nsHashKeys.h" + +namespace mozilla { + +class MediaDecoder; +class StateMachineThread; + +// The MediaShutdownManager manages shutting down the MediaDecoder +// infrastructure in response to an xpcom-shutdown notification. This happens +// when Gecko is shutting down in the middle of operation. This is tricky, as +// there are a number of moving parts that must be shutdown in a particular +// order. Additionally the xpcom-shutdown observer *must* block until all +// threads are shutdown, which is tricky since we have a number of threads +// here and their shutdown is asynchronous. We can't have each element of +// our pipeline listening for xpcom-shutdown, as if each observer blocks +// waiting for its threads to shutdown it will block other xpcom-shutdown +// notifications from firing, and shutdown of one part of the media pipeline +// (say the State Machine thread) may depend another part to be shutdown +// first (the MediaDecoder threads). Additionally we need to not interfere +// with shutdown in the the non-xpcom-shutdown case, where we need to be able +// to recreate the State Machine thread after it's been destroyed without +// affecting the shutdown of the old State Machine thread. The +// MediaShutdownManager encapsulates all these dependencies, and provides +// a single xpcom-shutdown listener for the MediaDecoder infrastructure, to +// ensure that no shutdown order dependencies leak out of the MediaDecoder +// stack. The MediaShutdownManager is a singleton. +// +// The MediaShutdownManager ensures that the MediaDecoder stack is shutdown +// before returning from its xpcom-shutdown observer by keeping track of all +// the active MediaDecoders, and upon xpcom-shutdown calling Shutdown() on +// every MediaDecoder and then spinning the main thread event loop until the +// State Machine thread has shutdown. Once the State Machine thread has been +// shutdown, the xpcom-shutdown observer returns. +// +// Note that calling the Unregister() functions may result in the singleton +// being deleted, so don't store references to the singleton, always use the +// singleton by derefing the referenced returned by +// MediaShutdownManager::Instance(), which ensures that the singleton is +// created when needed. +// i.e. like this: +// MediaShutdownManager::Instance()::Unregister(someDecoder); +// MediaShutdownManager::Instance()::Register(someOtherDecoder); +// Not like this: +// MediaShutdownManager& instance = MediaShutdownManager::Instance(); +// instance.Unregister(someDecoder); // Warning! May delete instance! +// instance.Register(someOtherDecoder); // BAD! instance may be dangling! +class MediaShutdownManager : public nsIObserver { +public: + NS_DECL_ISUPPORTS + NS_DECL_NSIOBSERVER + + // The MediaShutdownManager is a singleton, access its instance with + // this accessor. + static MediaShutdownManager& Instance(); + + // Notifies the MediaShutdownManager that it needs to track the shutdown + // of this MediaDecoder. + void Register(MediaDecoder* aDecoder); + + // Notifies the MediaShutdownManager that a MediaDecoder that it was + // tracking has shutdown, and it no longer needs to be shutdown in the + // xpcom-shutdown listener. + void Unregister(MediaDecoder* aDecoder); + + // Notifies the MediaShutdownManager of a state machine thread that + // must be tracked. Note that we track State Machine threads individually + // as their shutdown and the construction of a new state machine thread + // can interleave. This stores a strong ref to the state machine. + void Register(StateMachineThread* aThread); + + // Notifies the MediaShutdownManager that a StateMachineThread that it was + // tracking has shutdown, and it no longer needs to be shutdown in the + // xpcom-shutdown listener. This drops the strong reference to the + // StateMachineThread, which may destroy it. + void Unregister(StateMachineThread* aThread); + +private: + + MediaShutdownManager(); + virtual ~MediaShutdownManager(); + + void Shutdown(); + + // Ensures we have a shutdown listener if we need one, and removes the + // listener and destroys the singleton if we don't. + void EnsureCorrectShutdownObserverState(); + + static StaticRefPtr sInstance; + + // References to the MediaDecoder. The decoders unregister themselves + // in their Shutdown() method, so we'll drop the reference naturally when + // we're shutting down (in the non xpcom-shutdown case). + nsTHashtable> mDecoders; + + // References to the state machine threads that we're tracking shutdown + // of. Note that although there is supposed to be a single state machine, + // the construction and shutdown of these can interleave, so we must track + // individual instances of the state machine threads. + // These are strong references. + nsTHashtable> mStateMachineThreads; + + // True if we have an XPCOM shutdown observer. + bool mIsObservingShutdown; + + bool mIsDoingXPCOMShutDown; +}; + +// A wrapper for the state machine thread. We must wrap this so that the +// state machine threads can shutdown independently from the +// StateMachineTracker, under the control of the MediaShutdownManager. +// The state machine thread is shutdown naturally when all decoders +// complete their shutdown. So if a new decoder is created just as the +// old state machine thread has shutdown, we need to be able to shutdown +// the old state machine thread independently of the StateMachineTracker +// creating a new state machine thread. Also if this happens we need to +// be able to force both state machine threads to shutdown in the +// MediaShutdownManager, which is why we maintain a set of state machine +// threads, even though there's supposed to only be one alive at once. +// This class does not enforce its own thread safety, the StateMachineTracker +// ensures thread safety when it uses the StateMachineThread. +class StateMachineThread { +public: + StateMachineThread(); + ~StateMachineThread(); + + NS_INLINE_DECL_REFCOUNTING(StateMachineThread); + + // Creates the wrapped thread. + nsresult Init(); + + // Returns a reference to the underlying thread. Don't shut this down + // directly, use StateMachineThread::Shutdown() instead. + nsIThread* GetThread(); + + // Dispatches an event to the main thread to shutdown the wrapped thread. + // The owner's (StateMachineTracker's) reference to the StateMachineThread + // can be dropped, the StateMachineThread will shutdown itself + // asynchronously. + void Shutdown(); + + // Processes events on the main thread event loop until this thread + // has been shutdown. Use this to block until the asynchronous shutdown + // has complete. + void SpinUntilShutdownComplete(); + +private: + void ShutdownThread(); + nsCOMPtr mThread; +}; + +} // namespace mozilla + +#endif diff --git a/content/media/moz.build b/content/media/moz.build index b09fbca9bb3..fe861500399 100644 --- a/content/media/moz.build +++ b/content/media/moz.build @@ -123,6 +123,7 @@ UNIFIED_SOURCES += [ 'MediaDecoderStateMachine.cpp', 'MediaRecorder.cpp', 'MediaResource.cpp', + 'MediaShutdownManager.cpp', 'MediaStreamGraph.cpp', 'MediaStreamTrack.cpp', 'MP3FrameParser.cpp', From a969eaf546ff66648ee52bafa8f003d8ac6a20c4 Mon Sep 17 00:00:00 2001 From: Timothy Nikkel Date: Tue, 17 Dec 2013 23:02:56 -0600 Subject: [PATCH 16/31] Bug 951467. Give scroll layer items the bounds of the scroll port, not the bounds of the display port. r=roc The bounds of the scroll port match what will actually be drawn on the screen. The bounds of the contained content (sized to the display port) are still accessible via mList.GetBounds, and similarly the visible rect of the contained content is mList.GetVisibleRect. The external bounds/visible rect are GetBounds and GetVisibleRect on the nsDisplayScrollLayer object itself. We implement nsDisplayScrollInfoLayer::GetBounds solely so that it continues to return an empty rect because we expect active empty layers to have empty visible rects. We no longer have to set our mVisibleRect in nsDisplayScrollLayer::ComputeVisibility because nsDisplayList::ComputeVisibilityForSublist now does it correctly for us (like other items). We also have to teach ContainerState::ProcessDisplayItems to not set the visible region for scroll layers because it has the external visible region, not the larger internal display port sized visible region. We instead let BuildContainerLayer set the visible region of the layer. --- layout/base/FrameLayerBuilder.cpp | 9 +++--- layout/base/nsDisplayList.cpp | 54 ++++++++++++++++++++----------- layout/base/nsDisplayList.h | 6 ++++ 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/layout/base/FrameLayerBuilder.cpp b/layout/base/FrameLayerBuilder.cpp index 9505e2e747a..9658d750aa7 100644 --- a/layout/base/FrameLayerBuilder.cpp +++ b/layout/base/FrameLayerBuilder.cpp @@ -2261,11 +2261,12 @@ ContainerState::ProcessDisplayItems(const nsDisplayList& aList, nsDisplayItem::Type type = item->GetType(); - bool setVisibleRegion = type != nsDisplayItem::TYPE_TRANSFORM; - if (setVisibleRegion) { - mParameters.mAncestorClipRect = nullptr; - } else { + bool setVisibleRegion = (type != nsDisplayItem::TYPE_TRANSFORM) && + (type != nsDisplayItem::TYPE_SCROLL_LAYER); + if (type == nsDisplayItem::TYPE_TRANSFORM) { mParameters.mAncestorClipRect = itemClip.HasClip() ? &clipRect : nullptr; + } else { + mParameters.mAncestorClipRect = nullptr; } // Just use its layer. diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp index 22e5ff2c569..e8cb2669fa7 100644 --- a/layout/base/nsDisplayList.cpp +++ b/layout/base/nsDisplayList.cpp @@ -3484,6 +3484,17 @@ nsDisplayScrollLayer::~nsDisplayScrollLayer() } #endif +nsRect +nsDisplayScrollLayer::GetBounds(nsDisplayListBuilder* aBuilder, bool* aSnap) +{ + nsIScrollableFrame* sf = do_QueryFrame(mScrollFrame); + if (sf) { + *aSnap = false; + return sf->GetScrollPortRect() + aBuilder->ToReferenceFrame(mScrollFrame); + } + return nsDisplayWrapList::GetBounds(aBuilder, aSnap); +} + already_AddRefed nsDisplayScrollLayer::BuildLayer(nsDisplayListBuilder* aBuilder, LayerManager* aManager, @@ -3510,7 +3521,7 @@ nsDisplayScrollLayer::BuildLayer(nsDisplayListBuilder* aBuilder, nsLayoutUtils::GetCriticalDisplayPort(content, &criticalDisplayport); } RecordFrameMetrics(mScrolledFrame, mScrollFrame, ReferenceFrame(), layer, - mVisibleRect, viewport, + mList.GetVisibleRect(), viewport, (usingDisplayport ? &displayport : nullptr), (usingCriticalDisplayport ? &criticalDisplayport : nullptr), scrollId, false, aContainerParameters); @@ -3534,28 +3545,29 @@ nsDisplayScrollLayer::ComputeVisibility(nsDisplayListBuilder* aBuilder, const nsRect& aAllowVisibleRegionExpansion) { nsRect displayport; - if (nsLayoutUtils::GetDisplayPort(mScrolledFrame->GetContent(), &displayport)) { + bool usingDisplayPort = + nsLayoutUtils::GetDisplayPort(mScrolledFrame->GetContent(), &displayport); + nsRegion childVisibleRegion; + if (usingDisplayPort) { // The visible region for the children may be much bigger than the hole we // are viewing the children from, so that the compositor process has enough // content to asynchronously pan while content is being refreshed. - - nsRegion childVisibleRegion = displayport + mScrollFrame->GetOffsetToCrossDoc(ReferenceFrame()); - - nsRect boundedRect = - childVisibleRegion.GetBounds().Intersect(mList.GetBounds(aBuilder)); - nsRect allowExpansion = boundedRect.Intersect(aAllowVisibleRegionExpansion); - bool visible = mList.ComputeVisibilityForSublist( - aBuilder, &childVisibleRegion, boundedRect, allowExpansion); - // We don't allow this computation to influence aVisibleRegion, on the - // assumption that the layer can be asynchronously scrolled so we'll - // definitely need all the content under it. - mVisibleRect = boundedRect; - - return visible; + childVisibleRegion = displayport + mScrollFrame->GetOffsetToCrossDoc(ReferenceFrame()); } else { - return nsDisplayWrapList::ComputeVisibility(aBuilder, aVisibleRegion, - aAllowVisibleRegionExpansion); + bool snap; + childVisibleRegion = GetBounds(aBuilder, &snap); } + + nsRect boundedRect = + childVisibleRegion.GetBounds().Intersect(mList.GetBounds(aBuilder)); + nsRect allowExpansion = boundedRect.Intersect(aAllowVisibleRegionExpansion); + bool visible = mList.ComputeVisibilityForSublist( + aBuilder, &childVisibleRegion, boundedRect, allowExpansion); + // We don't allow this computation to influence aVisibleRegion, on the + // assumption that the layer can be asynchronously scrolled so we'll + // definitely need all the content under it. + + return visible; } LayerState @@ -3668,6 +3680,12 @@ nsDisplayScrollInfoLayer::~nsDisplayScrollInfoLayer() MOZ_COUNT_DTOR(nsDisplayScrollInfoLayer); } +nsRect +nsDisplayScrollInfoLayer::GetBounds(nsDisplayListBuilder* aBuilder, bool* aSnap) +{ + return nsDisplayWrapList::GetBounds(aBuilder, aSnap); +} + LayerState nsDisplayScrollInfoLayer::GetLayerState(nsDisplayListBuilder* aBuilder, LayerManager* aManager, diff --git a/layout/base/nsDisplayList.h b/layout/base/nsDisplayList.h index 77a5fab0311..d7a929ec9e9 100644 --- a/layout/base/nsDisplayList.h +++ b/layout/base/nsDisplayList.h @@ -1512,6 +1512,8 @@ public: bool DidComputeVisibility() const { return mDidComputeVisibility; } #endif + nsRect GetVisibleRect() const { return mVisibleRect; } + private: // This class is only used on stack, so we don't have to worry about leaking // it. Don't let us be heap-allocated! @@ -2709,6 +2711,8 @@ public: virtual ~nsDisplayScrollLayer(); #endif + virtual nsRect GetBounds(nsDisplayListBuilder* aBuilder, bool* aSnap) MOZ_OVERRIDE; + virtual already_AddRefed BuildLayer(nsDisplayListBuilder* aBuilder, LayerManager* aManager, const ContainerLayerParameters& aContainerParameters) MOZ_OVERRIDE; @@ -2760,6 +2764,8 @@ public: virtual ~nsDisplayScrollInfoLayer(); + virtual nsRect GetBounds(nsDisplayListBuilder* aBuilder, bool* aSnap) MOZ_OVERRIDE; + virtual LayerState GetLayerState(nsDisplayListBuilder* aBuilder, LayerManager* aManager, const ContainerLayerParameters& aParameters) MOZ_OVERRIDE; From 9454d623f68b3c0296246d1f50d72b56d815f4ca Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Wed, 18 Dec 2013 18:37:24 +1300 Subject: [PATCH 17/31] Bug 950427. Don't descend into subdocuments at all in elementFromPoint. r=mats --- content/base/src/nsDocument.cpp | 45 ++++++++++++--------------- content/base/src/nsDocument.h | 10 ------ layout/base/nsDisplayList.cpp | 1 + layout/base/nsDisplayList.h | 6 ++++ layout/base/nsLayoutUtils.cpp | 3 ++ layout/base/nsLayoutUtils.h | 6 +++- layout/generic/nsSubDocumentFrame.cpp | 3 +- 7 files changed, 37 insertions(+), 37 deletions(-) diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp index 348203c2cb8..006997c4c87 100644 --- a/content/base/src/nsDocument.cpp +++ b/content/base/src/nsDocument.cpp @@ -3131,6 +3131,20 @@ nsIDocument::ElementFromPoint(float aX, float aY) return ElementFromPointHelper(aX, aY, false, true); } +static nsIContent* +GetNonanonymousContent(nsIFrame* aFrame) +{ + for (nsIFrame* f = aFrame; f; + f = nsLayoutUtils::GetParentOrPlaceholderFor(f)) { + nsIContent* content = f->GetContent(); + if (content && !content->IsInAnonymousSubtree()) { + return content; + } + } + + return nullptr; +} + Element* nsDocument::ElementFromPointHelper(float aX, float aY, bool aIgnoreRootScrollFrame, @@ -3162,13 +3176,14 @@ nsDocument::ElementFromPointHelper(float aX, float aY, } nsIFrame *ptFrame = nsLayoutUtils::GetFrameForPoint(rootFrame, pt, - nsLayoutUtils::IGNORE_PAINT_SUPPRESSION | + nsLayoutUtils::IGNORE_PAINT_SUPPRESSION | nsLayoutUtils::IGNORE_CROSS_DOC | (aIgnoreRootScrollFrame ? nsLayoutUtils::IGNORE_ROOT_SCROLL_FRAME : 0)); if (!ptFrame) { return nullptr; } - nsIContent* elem = GetContentInThisDocument(ptFrame); + nsIContent* elem = GetNonanonymousContent(ptFrame); + NS_ASSERTION(!elem || elem->OwnerDoc() == this, "Wrong document"); if (elem && !elem->IsElement()) { elem = elem->GetParent(); } @@ -3217,14 +3232,15 @@ nsDocument::NodesFromRectHelper(float aX, float aY, nsAutoTArray outFrames; nsLayoutUtils::GetFramesForArea(rootFrame, rect, outFrames, - nsLayoutUtils::IGNORE_PAINT_SUPPRESSION | + nsLayoutUtils::IGNORE_PAINT_SUPPRESSION | nsLayoutUtils::IGNORE_CROSS_DOC | (aIgnoreRootScrollFrame ? nsLayoutUtils::IGNORE_ROOT_SCROLL_FRAME : 0)); // Used to filter out repeated elements in sequence. nsIContent* lastAdded = nullptr; for (uint32_t i = 0; i < outFrames.Length(); i++) { - nsIContent* node = GetContentInThisDocument(outFrames[i]); + nsIContent* node = GetNonanonymousContent(outFrames[i]); + NS_ASSERTION(!node || node->OwnerDoc() == this, "Wrong document"); if (node && !node->IsElement() && !node->IsNodeOfType(nsINode::eTEXT)) { // We have a node that isn't an element or a text node, @@ -8068,27 +8084,6 @@ nsDocument::DoUnblockOnload() } } -nsIContent* -nsDocument::GetContentInThisDocument(nsIFrame* aFrame) const -{ - for (nsIFrame* f = aFrame; f; - f = nsLayoutUtils::GetParentOrPlaceholderForCrossDoc(f)) { - nsIContent* content = f->GetContent(); - if (!content || content->IsInAnonymousSubtree()) - continue; - - if (content->OwnerDoc() == this) { - return content; - } - // We must be in a subdocument so jump directly to the root frame. - // GetParentOrPlaceholderForCrossDoc gets called immediately to jump up to - // the containing document. - f = f->PresContext()->GetPresShell()->GetRootFrame(); - } - - return nullptr; -} - void nsDocument::DispatchPageTransition(EventTarget* aDispatchTarget, const nsAString& aType, diff --git a/content/base/src/nsDocument.h b/content/base/src/nsDocument.h index 8607143debe..fdb5b5883dd 100644 --- a/content/base/src/nsDocument.h +++ b/content/base/src/nsDocument.h @@ -1310,16 +1310,6 @@ private: mCSPWebConsoleErrorQueue.Flush(this); } - /** - * Find the (non-anonymous) content in this document for aFrame. It will - * be aFrame's content node if that content is in this document and not - * anonymous. Otherwise, when aFrame is in a subdocument, we use the frame - * element containing the subdocument containing aFrame, and/or find the - * nearest non-anonymous ancestor in this document. - * Returns null if there is no such element. - */ - nsIContent* GetContentInThisDocument(nsIFrame* aFrame) const; - // Just like EnableStyleSheetsForSet, but doesn't check whether // aSheetSet is null and allows the caller to control whether to set // aSheetSet as the preferred set in the CSSLoader. diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp index e8cb2669fa7..6ef210a2075 100644 --- a/layout/base/nsDisplayList.cpp +++ b/layout/base/nsDisplayList.cpp @@ -486,6 +486,7 @@ nsDisplayListBuilder::nsDisplayListBuilder(nsIFrame* aReferenceFrame, mHadToIgnoreSuppression(false), mIsAtRootOfPseudoStackingContext(false), mIncludeAllOutOfFlows(false), + mDescendIntoSubdocuments(true), mSelectedFramesOnly(false), mAccurateVisibleRegions(false), mAllowMergingAndFlattening(true), diff --git a/layout/base/nsDisplayList.h b/layout/base/nsDisplayList.h index d7a929ec9e9..b02f01ad00a 100644 --- a/layout/base/nsDisplayList.h +++ b/layout/base/nsDisplayList.h @@ -283,6 +283,11 @@ public: */ void SetPaintingToWindow(bool aToWindow) { mIsPaintingToWindow = aToWindow; } bool IsPaintingToWindow() const { return mIsPaintingToWindow; } + /** + * Call this to prevent descending into subdocuments. + */ + void SetDescendIntoSubdocuments(bool aDescend) { mDescendIntoSubdocuments = aDescend; } + bool GetDescendIntoSubdocuments() { return mDescendIntoSubdocuments; } /** * Returns true if merging and flattening of display lists should be @@ -647,6 +652,7 @@ private: bool mHadToIgnoreSuppression; bool mIsAtRootOfPseudoStackingContext; bool mIncludeAllOutOfFlows; + bool mDescendIntoSubdocuments; bool mSelectedFramesOnly; bool mAccurateVisibleRegions; bool mAllowMergingAndFlattening; diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index fd72da235db..7c0539bc70a 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -2090,6 +2090,9 @@ nsLayoutUtils::GetFramesForArea(nsIFrame* aFrame, const nsRect& aRect, builder.SetIgnoreScrollFrame(rootScrollFrame); } } + if (aFlags & IGNORE_CROSS_DOC) { + builder.SetDescendIntoSubdocuments(false); + } builder.EnterPresShell(aFrame, target); aFrame->BuildDisplayListForStackingContext(&builder, target, &list); diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h index 4224503b9c3..5284dd6670c 100644 --- a/layout/base/nsLayoutUtils.h +++ b/layout/base/nsLayoutUtils.h @@ -607,7 +607,11 @@ public: * When set, clipping due to the root scroll frame (and any other viewport- * related clipping) is ignored. */ - IGNORE_ROOT_SCROLL_FRAME = 0x02 + IGNORE_ROOT_SCROLL_FRAME = 0x02, + /** + * When set, return only content in the same document as aFrame. + */ + IGNORE_CROSS_DOC = 0x04 }; /** diff --git a/layout/generic/nsSubDocumentFrame.cpp b/layout/generic/nsSubDocumentFrame.cpp index 3dfa4c59e59..a27aad5d809 100644 --- a/layout/generic/nsSubDocumentFrame.cpp +++ b/layout/generic/nsSubDocumentFrame.cpp @@ -288,8 +288,9 @@ nsSubDocumentFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, DisplayBorderBackgroundOutline(aBuilder, aLists); } - if (!mInnerView) + if (!mInnerView || !aBuilder->GetDescendIntoSubdocuments()) { return; + } nsFrameLoader* frameLoader = FrameLoader(); if (frameLoader) { From 36b99b274c73f8c8c2824cc3a94b421e8f3ab053 Mon Sep 17 00:00:00 2001 From: Eric Faust Date: Tue, 17 Dec 2013 21:50:45 -0800 Subject: [PATCH 18/31] Bug 951517 - Fix broken DOM TI check. (r=bz)Bug 951517 - Only Fix broken DOM TI check. (r=bz)Bug 951517 - Fix broken DOM TI check. (r=bz)Bug 951517 - OnlyFix broken DOM TI check. (r=bz)Bug 951517 - Only Fix broken DOM TI check. (r=bz)Bug 951517 - Fix broken DOM TI check. (r=bz)Bug 951517 - Only Fix broken DOM objectTI check. (r=bz)Bug 951517 - Fix broken DOM TI check. (r=bz)Bug 951517 - Fix broken DOM TI check. (r=bz) --- js/src/jsinfer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/jsinfer.cpp b/js/src/jsinfer.cpp index 5bfec3ee412..ee40baaddcb 100644 --- a/js/src/jsinfer.cpp +++ b/js/src/jsinfer.cpp @@ -1638,7 +1638,7 @@ TemporaryTypeSet::isDOMClass() return false; } - return true; + return count > 0; } bool From e71259f3dd3f57ab97e32e888477449cbd00cbb0 Mon Sep 17 00:00:00 2001 From: Eric Faust Date: Tue, 17 Dec 2013 21:52:35 -0800 Subject: [PATCH 19/31] Backed out changeset 5123ffbafac3 for bogus commit message. --- js/src/jsinfer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/jsinfer.cpp b/js/src/jsinfer.cpp index ee40baaddcb..5bfec3ee412 100644 --- a/js/src/jsinfer.cpp +++ b/js/src/jsinfer.cpp @@ -1638,7 +1638,7 @@ TemporaryTypeSet::isDOMClass() return false; } - return count > 0; + return true; } bool From 7c623f2cf8826c2742d85e42dde97ce34192dfd0 Mon Sep 17 00:00:00 2001 From: Eric Faust Date: Tue, 17 Dec 2013 21:53:21 -0800 Subject: [PATCH 20/31] Bug 951517 - Fix broken DOM TI check. (r=bz) --- js/src/jsinfer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/jsinfer.cpp b/js/src/jsinfer.cpp index 5bfec3ee412..ee40baaddcb 100644 --- a/js/src/jsinfer.cpp +++ b/js/src/jsinfer.cpp @@ -1638,7 +1638,7 @@ TemporaryTypeSet::isDOMClass() return false; } - return true; + return count > 0; } bool From da4f6206c05f873527ead53558cfeff9f8e09f22 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 18 Dec 2013 16:02:46 +0900 Subject: [PATCH 21/31] Bug 947115 All tests shouldn't use nsIDOMWindowUtils.sendNativeKeyEvent() directly. Use synthesizeNativeKey() instead. r=smaug --- browser/metro/base/tests/mochitest/head.js | 17 - docshell/test/test_bug511449.html | 3 +- .../mochitest/tests/SimpleTest/EventUtils.js | 60 +- .../tests/SimpleTest/NativeKeyCodes.js | 4 +- widget/cocoa/TextInputHandler.h | 6 + widget/cocoa/TextInputHandler.mm | 18 + widget/tests/test_bug428405.xul | 8 +- widget/tests/test_key_event_counts.xul | 9 +- widget/tests/test_keycodes.xul | 2753 +++++++++++------ .../tests/test_native_key_bindings_mac.html | 114 +- widget/tests/test_plugin_input_event.html | 12 +- widget/tests/test_secure_input.html | 3 +- 12 files changed, 1914 insertions(+), 1093 deletions(-) diff --git a/browser/metro/base/tests/mochitest/head.js b/browser/metro/base/tests/mochitest/head.js index 3812ee6d8f2..80c01330f05 100644 --- a/browser/metro/base/tests/mochitest/head.js +++ b/browser/metro/base/tests/mochitest/head.js @@ -561,23 +561,6 @@ function waitForObserver(aObsEvent, aTimeoutMs) { * generating os level input events that get processed by widget and * apzc logic. *===========================================================================*/ - -// Keyboard layouts for use with synthesizeNativeKey -const usEnglish = 0x409; -const arSpanish = 0x2C0A; - -// Modifiers for use with synthesizeNativeKey -const leftShift = 0x100; -const rightShift = 0x200; -const leftControl = 0x400; -const rightControl = 0x800; -const leftAlt = 0x1000; -const rightAlt = 0x2000; - -function synthesizeNativeKey(aKbLayout, aVKey, aModifiers) { - Browser.windowUtils.sendNativeKeyEvent(aKbLayout, aVKey, aModifiers, '', ''); -} - function synthesizeNativeMouse(aElement, aOffsetX, aOffsetY, aMsg) { let x = aOffsetX; let y = aOffsetY; diff --git a/docshell/test/test_bug511449.html b/docshell/test/test_bug511449.html index e4b18a54f3a..d408d7512ec 100644 --- a/docshell/test/test_bug511449.html +++ b/docshell/test/test_bug511449.html @@ -38,8 +38,7 @@ function runNextTest() { win.onunload = function() { didClose = true; } - var utils = SpecialPowers.getDOMWindowUtils(win); - utils.sendNativeKeyEvent(0, MAC_VK_ANSI_W, 0x4000 /* cmd */, "w", "w"); + synthesizeNativeKey(KEYBOARD_LAYOUT_EN_US, MAC_VK_ANSI_W, {metaKey:1}, "w", "w"); setTimeout(function () { ok(didClose, "Cmd+W should have closed the tab"); diff --git a/testing/mochitest/tests/SimpleTest/EventUtils.js b/testing/mochitest/tests/SimpleTest/EventUtils.js index 87ece73c112..bc1564a04f1 100644 --- a/testing/mochitest/tests/SimpleTest/EventUtils.js +++ b/testing/mochitest/tests/SimpleTest/EventUtils.js @@ -620,10 +620,47 @@ function _parseNativeModifiers(aModifiers) modifiers |= (navigator.platform.indexOf("Mac") == 0) ? 0x00008000 : 0x00000800; } + if (aModifiers.altGrKey) { + modifiers |= + (navigator.platform.indexOf("Win") == 0) ? 0x00002800 : 0x00001000; + } return modifiers; } -const KEYBOARD_LAYOUT_EN_US = 0; +// Mac: Any unused number is okay for adding new keyboard layout. +// When you add new keyboard layout here, you need to modify +// TISInputSourceWrapper::InitByLayoutID(). +// Win: These constants can be found by inspecting registry keys under +// HKEY_LOCAL_MACHINE\System\CurrentControlSet\Control\Keyboard Layouts + +const KEYBOARD_LAYOUT_ARABIC = + { name: "Arabic", Mac: 6, Win: 0x00000401 }; +const KEYBOARD_LAYOUT_BRAZILIAN_ABNT = + { name: "Brazilian ABNT", Mac: null, Win: 0x00000416 }; +const KEYBOARD_LAYOUT_DVORAK_QWERTY = + { name: "Dvorak-QWERTY", Mac: 4, Win: null }; +const KEYBOARD_LAYOUT_EN_US = + { name: "US", Mac: 0, Win: 0x00000409 }; +const KEYBOARD_LAYOUT_FRENCH = + { name: "French", Mac: 7, Win: 0x0000040C }; +const KEYBOARD_LAYOUT_GREEK = + { name: "Greek", Mac: 1, Win: 0x00000408 }; +const KEYBOARD_LAYOUT_GERMAN = + { name: "German", Mac: 2, Win: 0x00000407 }; +const KEYBOARD_LAYOUT_HEBREW = + { name: "Hebrew", Mac: 8, Win: 0x0000040D }; +const KEYBOARD_LAYOUT_JAPANESE = + { name: "Japanese", Mac: null, Win: 0x00000411 }; +const KEYBOARD_LAYOUT_LITHUANIAN = + { name: "Lithuanian", Mac: 9, Win: 0x00010427 }; +const KEYBOARD_LAYOUT_NORWEGIAN = + { name: "Norwegian", Mac: 10, Win: 0x00000414 }; +const KEYBOARD_LAYOUT_SPANISH = + { name: "Spanish", Mac: 11, Win: 0x0000040A }; +const KEYBOARD_LAYOUT_SWEDISH = + { name: "Swedish", Mac: 3, Win: 0x0000041D }; +const KEYBOARD_LAYOUT_THAI = + { name: "Thai", Mac: 5, Win: 0x0002041E }; /** * synthesizeNativeKey() dispatches native key event on active window. @@ -651,24 +688,13 @@ function synthesizeNativeKey(aKeyboardLayout, aNativeKeyCode, aModifiers, if (!utils) { return false; } - var nativeKeyboardLayout; + var nativeKeyboardLayout = null; if (navigator.platform.indexOf("Mac") == 0) { - switch (aKeyboardLayout) { - case KEYBOARD_LAYOUT_EN_US: - nativeKeyboardLayout = 0; - break; - default: - return false; - } + nativeKeyboardLayout = aKeyboardLayout.Mac; } else if (navigator.platform.indexOf("Win") == 0) { - switch (aKeyboardLayout) { - case KEYBOARD_LAYOUT_EN_US: - nativeKeyboardLayout = 0x409; - break; - default: - return false; - } - } else { + nativeKeyboardLayout = aKeyboardLayout.Win; + } + if (nativeKeyboardLayout === null) { return false; } utils.sendNativeKeyEvent(nativeKeyboardLayout, aNativeKeyCode, diff --git a/testing/mochitest/tests/SimpleTest/NativeKeyCodes.js b/testing/mochitest/tests/SimpleTest/NativeKeyCodes.js index a3442b3e39a..4a5045e417e 100644 --- a/testing/mochitest/tests/SimpleTest/NativeKeyCodes.js +++ b/testing/mochitest/tests/SimpleTest/NativeKeyCodes.js @@ -1,6 +1,6 @@ /** - * This file defines all virtual keycodes for - * nsIDOMWindowUtils.sendNativeKeyEvent() + * This file defines all virtual keycodes for synthesizeNativeKey() of + * EventUtils.js and nsIDOMWindowUtils.sendNativeKeyEvent(). * These values are defined in each platform's SDK or documents. */ diff --git a/widget/cocoa/TextInputHandler.h b/widget/cocoa/TextInputHandler.h index ee559985045..cf2a3de05de 100644 --- a/widget/cocoa/TextInputHandler.h +++ b/widget/cocoa/TextInputHandler.h @@ -94,6 +94,12 @@ public: * 3: Swedish-Pro * 4: Dvorak-Qwerty Cmd * 5: Thai + * 6: Arabic + * 7: French + * 8: Hebrew + * 9: Lithuanian + * 10: Norwegian + * 11: Spanish * @param aOverrideKeyboard When testing set to TRUE, otherwise, set to * FALSE. When TRUE, we use an ANSI keyboard * instead of the actual keyboard. diff --git a/widget/cocoa/TextInputHandler.mm b/widget/cocoa/TextInputHandler.mm index 0e29414b99f..dec0383f1d5 100644 --- a/widget/cocoa/TextInputHandler.mm +++ b/widget/cocoa/TextInputHandler.mm @@ -527,6 +527,24 @@ TISInputSourceWrapper::InitByLayoutID(SInt32 aLayoutID, case 5: InitByInputSourceID("com.apple.keylayout.Thai"); break; + case 6: + InitByInputSourceID("com.apple.keylayout.Arabic"); + break; + case 7: + InitByInputSourceID("com.apple.keylayout.French"); + break; + case 8: + InitByInputSourceID("com.apple.keylayout.Hebrew"); + break; + case 9: + InitByInputSourceID("com.apple.keylayout.Lithuanian"); + break; + case 10: + InitByInputSourceID("com.apple.keylayout.Norwegian"); + break; + case 11: + InitByInputSourceID("com.apple.keylayout.Spanish"); + break; default: Clear(); break; diff --git a/widget/tests/test_bug428405.xul b/widget/tests/test_bug428405.xul index 711f98c4683..3e616d77d51 100644 --- a/widget/tests/test_bug428405.xul +++ b/widget/tests/test_bug428405.xul @@ -26,8 +26,6 @@ SimpleTest.waitForExplicitFinish(); - var gUtils; - var gCmdOptYReceived = false; // Look for a cmd-opt-y event. @@ -41,8 +39,6 @@ } function setGlobals() { - gUtils = window.QueryInterface(Components.interfaces.nsIInterfaceRequestor). - getInterface(Components.interfaces.nsIDOMWindowUtils); var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"]. getService(Components.interfaces.nsIWindowMediator); gChromeWindow = wm.getMostRecentWindow("navigator:browser"); @@ -147,11 +143,11 @@ // DebugEventsPlugin v1.01 from bmo bug 441880. function synthesizeNativeReturnKey() { - gUtils.sendNativeKeyEvent(0, MAC_VK_Return, 0, "\u000a", "\u000a"); + synthesizeNativeKey(KEYBOARD_LAYOUT_EN_US, MAC_VK_Return, {}, "\u000a", "\u000a"); } function synthesizeNativeCmdOptY() { - gUtils.sendNativeKeyEvent(0, MAC_VK_ANSI_Y, 0x5000, "y", "y"); + synthesizeNativeKey(KEYBOARD_LAYOUT_EN_US, MAC_VK_ANSI_Y, {metaKey:1, altKey:1}, "y", "y"); } ]]> diff --git a/widget/tests/test_key_event_counts.xul b/widget/tests/test_key_event_counts.xul index 22af60a7f01..c2e00fac168 100644 --- a/widget/tests/test_key_event_counts.xul +++ b/widget/tests/test_key_event_counts.xul @@ -31,24 +31,21 @@ function runTest() { - var domWindowUtils = window.QueryInterface(Components.interfaces.nsIInterfaceRequestor). - getInterface(Components.interfaces.nsIDOMWindowUtils); - window.addEventListener("keypress", onKeyPress, false); // Test ctrl-tab gKeyPressEventCount = 0; - domWindowUtils.sendNativeKeyEvent(0, MAC_VK_Tab, 0x0400, "\t", "\t"); + synthesizeNativeKey(KEYBOARD_LAYOUT_EN_US, MAC_VK_Tab, {ctrlKey:1}, "\t", "\t"); is(gKeyPressEventCount, 1); // Test cmd+shift+a gKeyPressEventCount = 0; - domWindowUtils.sendNativeKeyEvent(0, MAC_VK_ANSI_A, 0x4000 | 0x0100, "a", "A"); + synthesizeNativeKey(KEYBOARD_LAYOUT_EN_US, MAC_VK_ANSI_A, {metaKey:1, shiftKey:1}, "a", "A"); is(gKeyPressEventCount, 1); // Test cmd-; gKeyPressEventCount = 0; - domWindowUtils.sendNativeKeyEvent(0, MAC_VK_ANSI_Semicolon, 0x4000, ";", ";"); + synthesizeNativeKey(KEYBOARD_LAYOUT_EN_US, MAC_VK_ANSI_Semicolon, {metaKey:1}, ";", ";"); is(gKeyPressEventCount, 1); window.removeEventListener("keypress", onKeyPress, false); diff --git a/widget/tests/test_keycodes.xul b/widget/tests/test_keycodes.xul index 695dbbb7312..ad22855bcdf 100644 --- a/widget/tests/test_keycodes.xul +++ b/widget/tests/test_keycodes.xul @@ -12,6 +12,8 @@ src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />