From 3492fb331ebfc18c1725ff326febd13d4460fb34 Mon Sep 17 00:00:00 2001 From: Zhang Junzhi Date: Fri, 23 Feb 2018 02:40:44 +0800 Subject: [PATCH 01/48] Bug 1438794 - Makes single-line text controls in vertical-writing mode vertically scrollable if they overflow vertically; and makes them horizontally unscrollable no matter whether they overflow horizontally. r=kats As for now, the scrollable direction with a mouse wheel for a single-line text control is hard-coded; that is, only horizontal wheel scrolls are able to take effect while vertical ones aren't. However, this isn't the desired case for vertical writing mode, where the opposite case definitely suits better. This commit refines the hard-coded scrollable direction for a single-line text control to be writing-mode-adaptive. MozReview-Commit-ID: 4Zkoe2ExPCZ --HG-- extra : rebase_source : 113b2ea80b6bbbcd2d8379b438de97eedd616551 --- dom/events/EventStateManager.cpp | 42 +++++++++---------- dom/events/EventStateManager.h | 4 -- dom/events/WheelHandlingHelper.cpp | 30 ++++++++++++- dom/events/WheelHandlingHelper.h | 10 ++++- gfx/layers/FrameMetrics.h | 39 ++++++++++------- gfx/layers/apz/src/AsyncPanZoomController.cpp | 31 ++++++++++---- gfx/layers/apz/src/Axis.h | 6 +-- gfx/layers/apz/src/GenericScrollAnimation.cpp | 8 ++-- gfx/layers/apz/src/GenericScrollAnimation.h | 7 +++- gfx/layers/apz/src/WheelScrollAnimation.cpp | 3 +- .../apz/test/gtest/APZCTreeManagerTester.h | 1 - gfx/layers/ipc/LayersMessageUtils.h | 4 +- layout/base/nsLayoutUtils.cpp | 10 ++--- 13 files changed, 124 insertions(+), 71 deletions(-) diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp index 80b1d1174dfe..d21255d4dd5d 100644 --- a/dom/events/EventStateManager.cpp +++ b/dom/events/EventStateManager.cpp @@ -44,7 +44,6 @@ #include "nsNameSpaceManager.h" #include "nsIBaseWindow.h" #include "nsISelection.h" -#include "nsITextControlElement.h" #include "nsFrameSelection.h" #include "nsPIDOMWindow.h" #include "nsPIWindowRoot.h" @@ -2294,21 +2293,6 @@ GetParentFrameToScroll(nsIFrame* aFrame) return aFrame->GetParent(); } -/*static*/ bool -EventStateManager::CanVerticallyScrollFrameWithWheel(nsIFrame* aFrame) -{ - nsIContent* c = aFrame->GetContent(); - if (!c) { - return true; - } - nsCOMPtr ctrl = - do_QueryInterface(c->IsInAnonymousSubtree() ? c->GetBindingParent() : c); - if (ctrl && ctrl->IsSingleLineTextControl()) { - return false; - } - return true; -} - void EventStateManager::DispatchLegacyMouseScrollEvents(nsIFrame* aTargetFrame, WidgetWheelEvent* aEvent, @@ -2608,17 +2592,29 @@ EventStateManager::ComputeScrollTarget(nsIFrame* aTargetFrame, nsIFrame* frameToScroll = do_QueryFrame(scrollableFrame); MOZ_ASSERT(frameToScroll); - // Don't scroll vertically by mouse-wheel on a single-line text control. - if (checkIfScrollableY) { - if (!CanVerticallyScrollFrameWithWheel(scrollFrame)) { - continue; - } - } - if (!checkIfScrollableX && !checkIfScrollableY) { return frameToScroll; } + // If the frame disregards the direction the user is trying to scroll, then + // it should just bubbles the scroll event up to its parental scroll frame + Maybe disregardedDirection = + WheelHandlingUtils::GetDisregardedWheelScrollDirection(scrollFrame); + if (disregardedDirection) { + switch (disregardedDirection.ref()) { + case layers::ScrollDirection::eHorizontal: + if (checkIfScrollableX) { + continue; + } + break; + case layers::ScrollDirection::eVertical: + if (checkIfScrollableY) { + continue; + } + break; + } + } + ScrollbarStyles ss = scrollableFrame->GetScrollbarStyles(); bool hiddenForV = (NS_STYLE_OVERFLOW_HIDDEN == ss.mVertical); bool hiddenForH = (NS_STYLE_OVERFLOW_HIDDEN == ss.mHorizontal); diff --git a/dom/events/EventStateManager.h b/dom/events/EventStateManager.h index 171f88496f3a..c3492c150976 100644 --- a/dom/events/EventStateManager.h +++ b/dom/events/EventStateManager.h @@ -311,10 +311,6 @@ public: double* aOutMultiplierX, double* aOutMultiplierY); - // Returns whether or not a frame can be vertically scrolled with a mouse - // wheel (as opposed to, say, a selection or touch scroll). - static bool CanVerticallyScrollFrameWithWheel(nsIFrame* aFrame); - // Holds the point in screen coords that a mouse event was dispatched to, // before we went into pointer lock mode. This is constantly updated while // the pointer is not locked, but we don't update it while the pointer is diff --git a/dom/events/WheelHandlingHelper.cpp b/dom/events/WheelHandlingHelper.cpp index b6357bfa4057..7176a829a650 100644 --- a/dom/events/WheelHandlingHelper.cpp +++ b/dom/events/WheelHandlingHelper.cpp @@ -16,6 +16,7 @@ #include "nsIDocument.h" #include "nsIPresShell.h" #include "nsIScrollableFrame.h" +#include "nsITextControlElement.h" #include "nsITimer.h" #include "nsPluginFrame.h" #include "nsPresContext.h" @@ -79,6 +80,31 @@ WheelHandlingUtils::CanScrollOn(nsIScrollableFrame* aScrollFrame, scrollRange.YMost(), aDirectionY)); } +/*static*/ Maybe +WheelHandlingUtils::GetDisregardedWheelScrollDirection(const nsIFrame* aFrame) +{ + nsIContent* content = aFrame->GetContent(); + if (!content) { + return Nothing(); + } + nsCOMPtr ctrl = + do_QueryInterface(content->IsInAnonymousSubtree() + ? content->GetBindingParent() : content); + if (!ctrl || !ctrl->IsSingleLineTextControl()) { + return Nothing(); + } + // Disregard scroll in the block-flow direction by mouse wheel on a + // single-line text control. For instance, in tranditional Chinese writing + // system, a single-line text control cannot be scrolled horizontally with + // mouse wheel even if they overflow at the right and left edges; Whereas in + // latin-based writing system, a single-line text control cannot be scrolled + // vertically with mouse wheel even if they overflow at the top and bottom + // edges + return Some(aFrame->GetWritingMode().IsVertical() + ? layers::ScrollDirection::eHorizontal + : layers::ScrollDirection::eVertical); +} + /******************************************************************/ /* mozilla::WheelTransaction */ /******************************************************************/ @@ -105,7 +131,7 @@ WheelTransaction::OwnScrollbars(bool aOwn) /* static */ void WheelTransaction::BeginTransaction(nsIFrame* aTargetFrame, - WidgetWheelEvent* aEvent) + const WidgetWheelEvent* aEvent) { NS_ASSERTION(!sTargetFrame, "previous transaction is not finished!"); MOZ_ASSERT(aEvent->mMessage == eWheel, @@ -120,7 +146,7 @@ WheelTransaction::BeginTransaction(nsIFrame* aTargetFrame, } /* static */ bool -WheelTransaction::UpdateTransaction(WidgetWheelEvent* aEvent) +WheelTransaction::UpdateTransaction(const WidgetWheelEvent* aEvent) { nsIFrame* scrollToFrame = GetTargetFrame(); nsIScrollableFrame* scrollableFrame = scrollToFrame->GetScrollTargetFrame(); diff --git a/dom/events/WheelHandlingHelper.h b/dom/events/WheelHandlingHelper.h index 28f1cf356497..25685561013f 100644 --- a/dom/events/WheelHandlingHelper.h +++ b/dom/events/WheelHandlingHelper.h @@ -68,6 +68,12 @@ public: static bool CanScrollOn(nsIScrollableFrame* aScrollFrame, double aDirectionX, double aDirectionY); + // For more details about the concept of a disregarded direction, refer to the + // code in struct mozilla::layers::ScrollMetadata which defines + // mDisregardedDirection. + static Maybe + GetDisregardedWheelScrollDirection(const nsIFrame* aFrame); + private: static bool CanScrollInRange(nscoord aMin, nscoord aValue, nscoord aMax, double aDirection); @@ -159,10 +165,10 @@ public: protected: static void BeginTransaction(nsIFrame* aTargetFrame, - WidgetWheelEvent* aEvent); + const WidgetWheelEvent* aEvent); // Be careful, UpdateTransaction may fire a DOM event, therefore, the target // frame might be destroyed in the event handler. - static bool UpdateTransaction(WidgetWheelEvent* aEvent); + static bool UpdateTransaction(const WidgetWheelEvent* aEvent); static void MayEndTransaction(); static LayoutDeviceIntPoint GetScreenPoint(WidgetGUIEvent* aEvent); diff --git a/gfx/layers/FrameMetrics.h b/gfx/layers/FrameMetrics.h index 5d85f66731e4..7d5b2bccd815 100644 --- a/gfx/layers/FrameMetrics.h +++ b/gfx/layers/FrameMetrics.h @@ -7,7 +7,7 @@ #ifndef GFX_FRAMEMETRICS_H #define GFX_FRAMEMETRICS_H -#include // for uint32_t, uint64_t +#include // for uint8_t, uint32_t, uint64_t #include "Units.h" // for CSSRect, CSSPixel, etc #include "mozilla/DefineEnum.h" // for MOZ_DEFINE_ENUM #include "mozilla/HashFunctions.h" // for HashGeneric @@ -16,6 +16,7 @@ #include "mozilla/gfx/Rect.h" // for RoundedIn #include "mozilla/gfx/ScaleFactor.h" // for ScaleFactor #include "mozilla/gfx/Logging.h" // for Log +#include "mozilla/layers/LayersTypes.h" // for ScrollDirection #include "mozilla/StaticPtr.h" // for StaticAutoPtr #include "mozilla/TimeStamp.h" // for TimeStamp #include "nsString.h" @@ -819,7 +820,6 @@ public: , mPageScrollAmount(0, 0) , mScrollClip() , mHasScrollgrab(false) - , mAllowVerticalScrollWithWheel(false) , mIsLayersIdRoot(false) , mUsesContainerScrolling(false) , mForceDisableApz(false) @@ -837,10 +837,10 @@ public: mPageScrollAmount == aOther.mPageScrollAmount && mScrollClip == aOther.mScrollClip && mHasScrollgrab == aOther.mHasScrollgrab && - mAllowVerticalScrollWithWheel == aOther.mAllowVerticalScrollWithWheel && mIsLayersIdRoot == aOther.mIsLayersIdRoot && mUsesContainerScrolling == aOther.mUsesContainerScrolling && mForceDisableApz == aOther.mForceDisableApz && + mDisregardedDirection == aOther.mDisregardedDirection && mOverscrollBehavior == aOther.mOverscrollBehavior; } @@ -926,12 +926,6 @@ public: bool GetHasScrollgrab() const { return mHasScrollgrab; } - bool AllowVerticalScrollWithWheel() const { - return mAllowVerticalScrollWithWheel; - } - void SetAllowVerticalScrollWithWheel(bool aValue) { - mAllowVerticalScrollWithWheel = aValue; - } void SetIsLayersIdRoot(bool aValue) { mIsLayersIdRoot = aValue; } @@ -951,6 +945,16 @@ public: return mForceDisableApz; } + // For more details about the concept of a disregarded direction, refer to the + // code which defines mDisregardedDirection. + Maybe GetDisregardedDirection() const { + return mDisregardedDirection; + } + void + SetDisregardedDirection(const Maybe& aValue) { + mDisregardedDirection = aValue; + } + void SetOverscrollBehavior(const OverscrollBehaviorInfo& aOverscrollBehavior) { mOverscrollBehavior = aOverscrollBehavior; } @@ -992,9 +996,6 @@ private: // Whether or not this frame is for an element marked 'scrollgrab'. bool mHasScrollgrab:1; - // Whether or not the frame can be vertically scrolled with a mouse wheel. - bool mAllowVerticalScrollWithWheel:1; - // Whether these framemetrics are for the root scroll frame (root element if // we don't have a root scroll frame) for its layers id. bool mIsLayersIdRoot:1; @@ -1007,6 +1008,13 @@ private: // scrollframe. bool mForceDisableApz:1; + // The disregarded direction means the direction which is disregarded anyway, + // even if the scroll frame overflows in that direction and the direction is + // specified as scrollable. This could happen in some scenarios, for instance, + // a single-line text control frame should disregard wheel scroll in + // its block-flow direction even if it overflows in that direction. + Maybe mDisregardedDirection; + // The overscroll behavior for this scroll frame. OverscrollBehaviorInfo mOverscrollBehavior; @@ -1014,9 +1022,10 @@ private: // // When adding new fields to ScrollMetadata, the following places should be // updated to include them (as needed): - // ScrollMetadata::operator == - // AsyncPanZoomController::NotifyLayersUpdated - // The ParamTraits specialization in GfxMessageUtils.h + // 1. ScrollMetadata::operator == + // 2. AsyncPanZoomController::NotifyLayersUpdated + // 3. The ParamTraits specialization in GfxMessageUtils.h and/or + // LayersMessageUtils.h // // Please add new fields above this comment. }; diff --git a/gfx/layers/apz/src/AsyncPanZoomController.cpp b/gfx/layers/apz/src/AsyncPanZoomController.cpp index 5f2c02d65bf1..5a196cb5810b 100644 --- a/gfx/layers/apz/src/AsyncPanZoomController.cpp +++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp @@ -1965,10 +1965,17 @@ bool AsyncPanZoomController::CanScrollWithWheel(const ParentLayerPoint& aDelta) const { RecursiveMutexAutoLock lock(mRecursiveMutex); - if (mX.CanScroll(aDelta.x)) { + + // For more details about the concept of a disregarded direction, refer to the + // code in struct ScrollMetadata which defines mDisregardedDirection. + Maybe disregardedDirection = + mScrollMetadata.GetDisregardedDirection(); + if (mX.CanScroll(aDelta.x) && + disregardedDirection != Some(ScrollDirection::eHorizontal)) { return true; } - if (mY.CanScroll(aDelta.y) && mScrollMetadata.AllowVerticalScrollWithWheel()) { + if (mY.CanScroll(aDelta.y) && + disregardedDirection != Some(ScrollDirection::eVertical)) { return true; } return false; @@ -2713,15 +2720,20 @@ bool AsyncPanZoomController::AttemptScroll(ParentLayerPoint& aStartPoint, if (scrollThisApzc) { RecursiveMutexAutoLock lock(mRecursiveMutex); + bool forcesVerticalOverscroll = + ScrollSource::Wheel == aOverscrollHandoffState.mScrollSource && + mScrollMetadata.GetDisregardedDirection() == Some(ScrollDirection::eVertical); + bool forcesHorizontalOverscroll = + ScrollSource::Wheel == aOverscrollHandoffState.mScrollSource && + mScrollMetadata.GetDisregardedDirection() == Some(ScrollDirection::eHorizontal); ParentLayerPoint adjustedDisplacement; - bool forceVerticalOverscroll = - (aOverscrollHandoffState.mScrollSource == ScrollSource::Wheel && - !mScrollMetadata.AllowVerticalScrollWithWheel()); - bool yChanged = mY.AdjustDisplacement(displacement.y, adjustedDisplacement.y, overscroll.y, - forceVerticalOverscroll); - bool xChanged = mX.AdjustDisplacement(displacement.x, adjustedDisplacement.x, overscroll.x); - + bool yChanged = mY.AdjustDisplacement(displacement.y, adjustedDisplacement.y, + overscroll.y, + forcesVerticalOverscroll); + bool xChanged = mX.AdjustDisplacement(displacement.x, adjustedDisplacement.x, + overscroll.x, + forcesHorizontalOverscroll); if (xChanged || yChanged) { ScheduleComposite(); } @@ -3945,6 +3957,7 @@ void AsyncPanZoomController::NotifyLayersUpdated(const ScrollMetadata& aScrollMe mScrollMetadata.SetUsesContainerScrolling(aScrollMetadata.UsesContainerScrolling()); mFrameMetrics.SetIsScrollInfoLayer(aLayerMetrics.IsScrollInfoLayer()); mScrollMetadata.SetForceDisableApz(aScrollMetadata.IsApzForceDisabled()); + mScrollMetadata.SetDisregardedDirection(aScrollMetadata.GetDisregardedDirection()); mScrollMetadata.SetOverscrollBehavior(aScrollMetadata.GetOverscrollBehavior()); if (scrollOffsetUpdated) { diff --git a/gfx/layers/apz/src/Axis.h b/gfx/layers/apz/src/Axis.h index 302f094e757c..fea65a80d388 100644 --- a/gfx/layers/apz/src/Axis.h +++ b/gfx/layers/apz/src/Axis.h @@ -83,9 +83,9 @@ public: * to prevent the viewport from overscrolling the page rect), and axis locking * (which might prevent any displacement from happening). If overscroll * ocurred, its amount is written to |aOverscrollAmountOut|. - * The |aDisplacementOut| parameter is set to the adjusted - * displacement, and the function returns true iff internal overscroll amounts - * were changed. + * The |aDisplacementOut| parameter is set to the adjusted displacement, and + * the function returns true if and only if internal overscroll amounts were + * changed. */ bool AdjustDisplacement(ParentLayerCoord aDisplacement, /* ParentLayerCoord */ float& aDisplacementOut, diff --git a/gfx/layers/apz/src/GenericScrollAnimation.cpp b/gfx/layers/apz/src/GenericScrollAnimation.cpp index b45190cdbb2b..d42bb7e60d8a 100644 --- a/gfx/layers/apz/src/GenericScrollAnimation.cpp +++ b/gfx/layers/apz/src/GenericScrollAnimation.cpp @@ -21,7 +21,6 @@ GenericScrollAnimation::GenericScrollAnimation(AsyncPanZoomController& aApzc, const ScrollAnimationBezierPhysicsSettings& aSettings) : mApzc(aApzc) , mFinalDestination(aInitialPosition) - , mForceVerticalOverscroll(false) { if (gfxPrefs::SmoothScrollMSDPhysicsEnabled()) { mAnimationPhysics = MakeUnique(aInitialPosition); @@ -86,9 +85,12 @@ GenericScrollAnimation::DoSample(FrameMetrics& aFrameMetrics, const TimeDuration // Note: we ignore overscroll for generic animations. ParentLayerPoint adjustedOffset, overscroll; - mApzc.mX.AdjustDisplacement(displacement.x, adjustedOffset.x, overscroll.x); + mApzc.mX.AdjustDisplacement(displacement.x, adjustedOffset.x, overscroll.x, + mDirectionForcedToOverscroll + == Some(ScrollDirection::eHorizontal)); mApzc.mY.AdjustDisplacement(displacement.y, adjustedOffset.y, overscroll.y, - mForceVerticalOverscroll); + mDirectionForcedToOverscroll + == Some(ScrollDirection::eVertical)); // If we expected to scroll, but there's no more scroll range on either axis, // then end the animation early. Note that the initial displacement could be 0 diff --git a/gfx/layers/apz/src/GenericScrollAnimation.h b/gfx/layers/apz/src/GenericScrollAnimation.h index d4764d5b166f..66df51617d07 100644 --- a/gfx/layers/apz/src/GenericScrollAnimation.h +++ b/gfx/layers/apz/src/GenericScrollAnimation.h @@ -42,7 +42,12 @@ protected: AsyncPanZoomController& mApzc; UniquePtr mAnimationPhysics; nsPoint mFinalDestination; - bool mForceVerticalOverscroll; + // If a direction is forced to overscroll, it means it's axis in that + // direction is locked, and scroll in that direction is treated as overscroll + // of an equal amount, which, for example, may then bubble up a scroll action + // to its parent, or may behave as whatever an overscroll occurence requires + // to behave + Maybe mDirectionForcedToOverscroll; }; } // namespace layers diff --git a/gfx/layers/apz/src/WheelScrollAnimation.cpp b/gfx/layers/apz/src/WheelScrollAnimation.cpp index 7f440c8ce915..d3d180afd1ee 100644 --- a/gfx/layers/apz/src/WheelScrollAnimation.cpp +++ b/gfx/layers/apz/src/WheelScrollAnimation.cpp @@ -46,7 +46,8 @@ WheelScrollAnimation::WheelScrollAnimation(AsyncPanZoomController& aApzc, ScrollWheelInput::ScrollDeltaType aDeltaType) : GenericScrollAnimation(aApzc, aInitialPosition, SettingsForDeltaType(aDeltaType)) { - mForceVerticalOverscroll = !mApzc.mScrollMetadata.AllowVerticalScrollWithWheel(); + mDirectionForcedToOverscroll = + mApzc.mScrollMetadata.GetDisregardedDirection(); } } // namespace layers diff --git a/gfx/layers/apz/test/gtest/APZCTreeManagerTester.h b/gfx/layers/apz/test/gtest/APZCTreeManagerTester.h index 3e39debc6eec..1312e256e0f3 100644 --- a/gfx/layers/apz/test/gtest/APZCTreeManagerTester.h +++ b/gfx/layers/apz/test/gtest/APZCTreeManagerTester.h @@ -72,7 +72,6 @@ protected: metrics.SetScrollOffset(CSSPoint(0, 0)); metadata.SetPageScrollAmount(LayoutDeviceIntSize(50, 100)); metadata.SetLineScrollAmount(LayoutDeviceIntSize(5, 10)); - metadata.SetAllowVerticalScrollWithWheel(true); return metadata; } diff --git a/gfx/layers/ipc/LayersMessageUtils.h b/gfx/layers/ipc/LayersMessageUtils.h index 92dcd726c613..d5f3d12eef81 100644 --- a/gfx/layers/ipc/LayersMessageUtils.h +++ b/gfx/layers/ipc/LayersMessageUtils.h @@ -289,10 +289,10 @@ struct ParamTraits WriteParam(aMsg, aParam.mPageScrollAmount); WriteParam(aMsg, aParam.mScrollClip); WriteParam(aMsg, aParam.mHasScrollgrab); - WriteParam(aMsg, aParam.mAllowVerticalScrollWithWheel); WriteParam(aMsg, aParam.mIsLayersIdRoot); WriteParam(aMsg, aParam.mUsesContainerScrolling); WriteParam(aMsg, aParam.mForceDisableApz); + WriteParam(aMsg, aParam.mDisregardedDirection); WriteParam(aMsg, aParam.mOverscrollBehavior); } @@ -317,10 +317,10 @@ struct ParamTraits ReadParam(aMsg, aIter, &aResult->mPageScrollAmount) && ReadParam(aMsg, aIter, &aResult->mScrollClip) && ReadBoolForBitfield(aMsg, aIter, aResult, ¶mType::SetHasScrollgrab) && - ReadBoolForBitfield(aMsg, aIter, aResult, ¶mType::SetAllowVerticalScrollWithWheel) && ReadBoolForBitfield(aMsg, aIter, aResult, ¶mType::SetIsLayersIdRoot) && ReadBoolForBitfield(aMsg, aIter, aResult, ¶mType::SetUsesContainerScrolling) && ReadBoolForBitfield(aMsg, aIter, aResult, ¶mType::SetForceDisableApz) && + ReadParam(aMsg, aIter, &aResult->mDisregardedDirection) && ReadParam(aMsg, aIter, &aResult->mOverscrollBehavior)); } }; diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index d5a440158650..48bcd2745a4a 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -115,11 +115,11 @@ #include "mozilla/layers/CompositorBridgeChild.h" #include "mozilla/Telemetry.h" #include "mozilla/EventDispatcher.h" -#include "mozilla/EventStateManager.h" #include "mozilla/RuleNodeCacheConditions.h" #include "mozilla/StyleAnimationValue.h" #include "mozilla/StyleSetHandle.h" #include "mozilla/StyleSetHandleInlines.h" +#include "mozilla/WheelHandlingHelper.h" // for WheelHandlingUtils #include "RegionBuilder.h" #include "SVGViewportElement.h" #include "DisplayItemClip.h" @@ -9403,10 +9403,10 @@ nsLayoutUtils::ComputeScrollMetadata(nsIFrame* aForFrame, LayoutDeviceIntSize::FromAppUnitsRounded(pageScrollAmount, presContext->AppUnitsPerDevPixel()); metadata.SetPageScrollAmount(pageScrollAmountInDevPixels); - if (!aScrollFrame->GetParent() || - EventStateManager::CanVerticallyScrollFrameWithWheel(aScrollFrame->GetParent())) - { - metadata.SetAllowVerticalScrollWithWheel(true); + if (aScrollFrame->GetParent()) { + metadata.SetDisregardedDirection( + WheelHandlingUtils::GetDisregardedWheelScrollDirection( + aScrollFrame->GetParent())); } metadata.SetUsesContainerScrolling(scrollableFrame->UsesContainerScrolling()); From 46356ed826e34619c5486b4325e24c8be3237830 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Tue, 20 Feb 2018 18:57:35 +0900 Subject: [PATCH 02/48] Bug 1439500 - Make devtools/client/shared/widgets handle non-printable keys and key combinations with keydown event r=jryans We'll stop dispatching keypress event for non-printable keys and key combinations at least in web content. This means that widget of devtools/client cannot be tested with loading them into tabs with mochitest browser chrome. Fortunately, they are cleanly independent from other module's keypress event listeners. So, we can make they use keydown event to handle non-printable keys and key combinations. MozReview-Commit-ID: 6fNSsGi9VbQ --HG-- extra : rebase_source : d8c201d2233dd1c08a87ad5123aaa2942af643d7 --- ...browser_treeWidget_keyboard_interaction.js | 12 ++--- .../shared/widgets/AbstractTreeItem.jsm | 8 +-- .../shared/widgets/BreadcrumbsWidget.jsm | 2 +- .../client/shared/widgets/FastListWidget.js | 2 +- .../client/shared/widgets/FilterWidget.js | 2 +- devtools/client/shared/widgets/FlameGraph.js | 10 ---- .../client/shared/widgets/SideMenuWidget.jsm | 2 +- devtools/client/shared/widgets/TableWidget.js | 2 +- devtools/client/shared/widgets/TreeWidget.js | 6 +-- .../client/shared/widgets/VariablesView.jsm | 51 ++++++++----------- .../tooltip/SwatchBasedEditorTooltip.js | 6 +-- .../client/shared/widgets/tooltip/Tooltip.js | 12 ++--- .../client/shared/widgets/view-helpers.js | 10 ++-- 13 files changed, 51 insertions(+), 74 deletions(-) diff --git a/devtools/client/shared/test/browser_treeWidget_keyboard_interaction.js b/devtools/client/shared/test/browser_treeWidget_keyboard_interaction.js index e461816dab9c..307c787b7d42 100644 --- a/devtools/client/shared/test/browser_treeWidget_keyboard_interaction.js +++ b/devtools/client/shared/test/browser_treeWidget_keyboard_interaction.js @@ -11,9 +11,6 @@ const TEST_URI = "data:text/html;charset=utf-8," + "ets.css'>
"; const {TreeWidget} = require("devtools/client/shared/widgets/TreeWidget"); -const kStrictKeyPressEvents = SpecialPowers.getBoolPref( - "dom.keyboardevent.keypress.dispatch_non_printable_keys_only_system_group_in_content"); - add_task(async function () { await addTab("about:blank"); let [host, win, doc] = await createHost("bottom", TEST_URI); @@ -146,8 +143,7 @@ async function testKeyboardInteraction(tree, win) { // pressing left to check expand collapse feature. // This does not emit any event, so listening for keypress - const eventToListen = kStrictKeyPressEvents ? "keydown" : "keypress"; - tree.root.children.addEventListener(eventToListen, () => { + tree.root.children.addEventListener("keydown", () => { // executeSoon so that other listeners on the same method are executed first executeSoon(() => event.resolve(null)); }, {once: true}); @@ -189,7 +185,7 @@ async function testKeyboardInteraction(tree, win) { // collapsing the item to check expand feature. - tree.root.children.addEventListener(eventToListen, () => { + tree.root.children.addEventListener("keydown", () => { executeSoon(() => event.resolve(null)); }, {once: true}); info("Pressing left key to collapse the item"); @@ -202,7 +198,7 @@ async function testKeyboardInteraction(tree, win) { // pressing right should expand this now. - tree.root.children.addEventListener(eventToListen, () => { + tree.root.children.addEventListener("keydown", () => { executeSoon(() => event.resolve(null)); }, {once: true}); info("Pressing right key to expend the collapsed item"); @@ -219,7 +215,7 @@ async function testKeyboardInteraction(tree, win) { node = tree._selectedLabel; // pressing down again should not change selection event = defer(); - tree.root.children.addEventListener(eventToListen, () => { + tree.root.children.addEventListener("keydown", () => { executeSoon(() => event.resolve(null)); }, {once: true}); info("Pressing down key on last item of the tree"); diff --git a/devtools/client/shared/widgets/AbstractTreeItem.jsm b/devtools/client/shared/widgets/AbstractTreeItem.jsm index c28658b4044d..a7dc1381a3a6 100644 --- a/devtools/client/shared/widgets/AbstractTreeItem.jsm +++ b/devtools/client/shared/widgets/AbstractTreeItem.jsm @@ -442,7 +442,7 @@ AbstractTreeItem.prototype = { this._onArrowClick = this._onArrowClick.bind(this); this._onClick = this._onClick.bind(this); this._onDoubleClick = this._onDoubleClick.bind(this); - this._onKeyPress = this._onKeyPress.bind(this); + this._onKeyDown = this._onKeyDown.bind(this); this._onFocus = this._onFocus.bind(this); this._onBlur = this._onBlur.bind(this); @@ -457,7 +457,7 @@ AbstractTreeItem.prototype = { targetNode.addEventListener("mousedown", this._onClick); targetNode.addEventListener("dblclick", this._onDoubleClick); - targetNode.addEventListener("keypress", this._onKeyPress); + targetNode.addEventListener("keydown", this._onKeyDown); targetNode.addEventListener("focus", this._onFocus); targetNode.addEventListener("blur", this._onBlur); @@ -579,9 +579,9 @@ AbstractTreeItem.prototype = { }, /** - * Handler for the "keypress" event on the element displaying this tree item. + * Handler for the "keydown" event on the element displaying this tree item. */ - _onKeyPress: function (e) { + _onKeyDown: function (e) { // Prevent scrolling when pressing navigation keys. ViewHelpers.preventScrolling(e); diff --git a/devtools/client/shared/widgets/BreadcrumbsWidget.jsm b/devtools/client/shared/widgets/BreadcrumbsWidget.jsm index 49cf23bb2c6f..97a209b26c7d 100644 --- a/devtools/client/shared/widgets/BreadcrumbsWidget.jsm +++ b/devtools/client/shared/widgets/BreadcrumbsWidget.jsm @@ -36,7 +36,7 @@ this.BreadcrumbsWidget = function BreadcrumbsWidget(aNode, aOptions = {}) { this._list.setAttribute("orient", "horizontal"); this._list.setAttribute("clicktoscroll", "true"); this._list.setAttribute("smoothscroll", !!aOptions.smoothScroll); - this._list.addEventListener("keypress", e => this.emit("keyPress", e)); + this._list.addEventListener("keydown", e => this.emit("keyDown", e)); this._list.addEventListener("mousedown", e => this.emit("mousePress", e)); this._parent.appendChild(this._list); diff --git a/devtools/client/shared/widgets/FastListWidget.js b/devtools/client/shared/widgets/FastListWidget.js index 3a32f29b0e5b..a4be8362d113 100644 --- a/devtools/client/shared/widgets/FastListWidget.js +++ b/devtools/client/shared/widgets/FastListWidget.js @@ -32,7 +32,7 @@ const FastListWidget = module.exports = function FastListWidget(node) { this._list.setAttribute("flex", "1"); this._list.setAttribute("orient", "vertical"); this._list.setAttribute("tabindex", "0"); - this._list.addEventListener("keypress", e => this.emit("keyPress", e)); + this._list.addEventListener("keydown", e => this.emit("keyDown", e)); this._list.addEventListener("mousedown", e => this.emit("mousePress", e)); this._parent.appendChild(this._list); diff --git a/devtools/client/shared/widgets/FilterWidget.js b/devtools/client/shared/widgets/FilterWidget.js index 2a9e8ce69025..3eb36423ebd9 100644 --- a/devtools/client/shared/widgets/FilterWidget.js +++ b/devtools/client/shared/widgets/FilterWidget.js @@ -341,7 +341,7 @@ CSSFilterEditorWidget.prototype = { // Filters that have units are number-type filters. For them, // the value can be incremented/decremented simply. // For other types of filters (e.g. drop-shadow) we need to check - // if the keypress happened close to a number first. + // if the keydown happened close to a number first. if (filter.unit) { let startValue = parseFloat(e.target.value); let value = startValue + direction * multiplier; diff --git a/devtools/client/shared/widgets/FlameGraph.js b/devtools/client/shared/widgets/FlameGraph.js index 56ac531438a1..1adc250d3f95 100644 --- a/devtools/client/shared/widgets/FlameGraph.js +++ b/devtools/client/shared/widgets/FlameGraph.js @@ -185,7 +185,6 @@ function FlameGraph(parent, sharpness) { this._onAnimationFrame = this._onAnimationFrame.bind(this); this._onKeyDown = this._onKeyDown.bind(this); this._onKeyUp = this._onKeyUp.bind(this); - this._onKeyPress = this._onKeyPress.bind(this); this._onMouseMove = this._onMouseMove.bind(this); this._onMouseDown = this._onMouseDown.bind(this); this._onMouseUp = this._onMouseUp.bind(this); @@ -195,7 +194,6 @@ function FlameGraph(parent, sharpness) { this._window.addEventListener("keydown", this._onKeyDown); this._window.addEventListener("keyup", this._onKeyUp); - this._window.addEventListener("keypress", this._onKeyPress); this._window.addEventListener("mousemove", this._onMouseMove); this._window.addEventListener("mousedown", this._onMouseDown); this._window.addEventListener("mouseup", this._onMouseUp); @@ -239,7 +237,6 @@ FlameGraph.prototype = { this._window.removeEventListener("keydown", this._onKeyDown); this._window.removeEventListener("keyup", this._onKeyUp); - this._window.removeEventListener("keypress", this._onKeyPress); this._window.removeEventListener("mousemove", this._onMouseMove); this._window.removeEventListener("mousedown", this._onMouseDown); this._window.removeEventListener("mouseup", this._onMouseUp); @@ -938,13 +935,6 @@ FlameGraph.prototype = { } }, - /** - * Listener for the "keypress" event on the graph's container. - */ - _onKeyPress: function (e) { - ViewHelpers.preventScrolling(e); - }, - /** * Listener for the "mousemove" event on the graph's container. */ diff --git a/devtools/client/shared/widgets/SideMenuWidget.jsm b/devtools/client/shared/widgets/SideMenuWidget.jsm index 8cb637278e60..96959dfdb0bc 100644 --- a/devtools/client/shared/widgets/SideMenuWidget.jsm +++ b/devtools/client/shared/widgets/SideMenuWidget.jsm @@ -54,7 +54,7 @@ this.SideMenuWidget = function SideMenuWidget(aNode, aOptions = {}) { this._list.setAttribute("with-group-checkboxes", this._showGroupCheckboxes); this._list.setAttribute("tabindex", "0"); this._list.addEventListener("contextmenu", e => this._showContextMenu(e)); - this._list.addEventListener("keypress", e => this.emit("keyPress", e)); + this._list.addEventListener("keydown", e => this.emit("keyDown", e)); this._list.addEventListener("mousedown", e => this.emit("mousePress", e)); this._parent.appendChild(this._list); diff --git a/devtools/client/shared/widgets/TableWidget.js b/devtools/client/shared/widgets/TableWidget.js index 3ebc14b47656..13f7f4b1624b 100644 --- a/devtools/client/shared/widgets/TableWidget.js +++ b/devtools/client/shared/widgets/TableWidget.js @@ -1760,7 +1760,7 @@ EditableFieldsEngine.prototype = { }, /** - * Handle keypresses when in edit mode: + * Handle keydowns when in edit mode: * - revert the value and close the textbox. * - apply the value and close the textbox. * - Handled by the consumer's `onTab` callback. diff --git a/devtools/client/shared/widgets/TreeWidget.js b/devtools/client/shared/widgets/TreeWidget.js index 806d9ec086c5..7cd33413db32 100644 --- a/devtools/client/shared/widgets/TreeWidget.js +++ b/devtools/client/shared/widgets/TreeWidget.js @@ -135,7 +135,7 @@ TreeWidget.prototype = { this._parent.appendChild(this.root.children); this.root.children.addEventListener("mousedown", e => this.onClick(e)); - this.root.children.addEventListener("keypress", e => this.onKeypress(e)); + this.root.children.addEventListener("keydown", e => this.onKeydown(e)); }, /** @@ -341,10 +341,10 @@ TreeWidget.prototype = { }, /** - * Keypress handler for this tree. Used to select next and previous visible + * Keydown handler for this tree. Used to select next and previous visible * items, as well as collapsing and expanding any item. */ - onKeypress: function (event) { + onKeydown: function (event) { switch (event.keyCode) { case KeyCodes.DOM_VK_UP: this.selectPreviousItem(); diff --git a/devtools/client/shared/widgets/VariablesView.jsm b/devtools/client/shared/widgets/VariablesView.jsm index d6fcf6dc0c4b..f04248bfeb99 100644 --- a/devtools/client/shared/widgets/VariablesView.jsm +++ b/devtools/client/shared/widgets/VariablesView.jsm @@ -81,14 +81,12 @@ this.VariablesView = function VariablesView(aParentNode, aFlags = {}) { this._appendEmptyNotice(); this._onSearchboxInput = this._onSearchboxInput.bind(this); - this._onSearchboxKeyPress = this._onSearchboxKeyPress.bind(this); - this._onViewKeyPress = this._onViewKeyPress.bind(this); + this._onSearchboxKeyDown = this._onSearchboxKeyDown.bind(this); this._onViewKeyDown = this._onViewKeyDown.bind(this); // Create an internal scrollbox container. this._list = this.document.createElement("scrollbox"); this._list.setAttribute("orient", "vertical"); - this._list.addEventListener("keypress", this._onViewKeyPress); this._list.addEventListener("keydown", this._onViewKeyDown); this._parent.appendChild(this._list); @@ -192,9 +190,7 @@ VariablesView.prototype = { let currList = this._list = this.document.createElement("scrollbox"); this.window.setTimeout(() => { - prevList.removeEventListener("keypress", this._onViewKeyPress); prevList.removeEventListener("keydown", this._onViewKeyDown); - currList.addEventListener("keypress", this._onViewKeyPress); currList.addEventListener("keydown", this._onViewKeyDown); currList.setAttribute("orient", "vertical"); @@ -457,7 +453,7 @@ VariablesView.prototype = { searchbox.setAttribute("type", "search"); searchbox.setAttribute("flex", "1"); searchbox.addEventListener("command", this._onSearchboxInput); - searchbox.addEventListener("keypress", this._onSearchboxKeyPress); + searchbox.addEventListener("keydown", this._onSearchboxKeyDown); container.appendChild(searchbox); ownerNode.insertBefore(container, this._parent); @@ -474,7 +470,7 @@ VariablesView.prototype = { } this._searchboxContainer.remove(); this._searchboxNode.removeEventListener("command", this._onSearchboxInput); - this._searchboxNode.removeEventListener("keypress", this._onSearchboxKeyPress); + this._searchboxNode.removeEventListener("keydown", this._onSearchboxKeyDown); this._searchboxContainer = null; this._searchboxNode = null; @@ -503,9 +499,9 @@ VariablesView.prototype = { }, /** - * Listener handling the searchbox key press event. + * Listener handling the searchbox keydown event. */ - _onSearchboxKeyPress: function (e) { + _onSearchboxKeyDown: function (e) { switch (e.keyCode) { case KeyCodes.DOM_VK_RETURN: this._onSearchboxInput(); @@ -811,15 +807,25 @@ VariablesView.prototype = { }, /** - * Listener handling a key press event on the view. + * Listener handling a key down event on the view. */ - _onViewKeyPress: function (e) { + _onViewKeyDown: function (e) { let item = this.getFocusedItem(); // Prevent scrolling when pressing navigation keys. ViewHelpers.preventScrolling(e); switch (e.keyCode) { + case KeyCodes.DOM_VK_C: + // Copy current selection to clipboard. + if (e.ctrlKey || e.metaKey) { + let item = this.getFocusedItem(); + clipboardHelper.copyString( + item._nameString + item.separatorStr + item._valueString + ); + } + return; + case KeyCodes.DOM_VK_UP: // Always rewind focus. this.focusPrevItem(true); @@ -899,21 +905,6 @@ VariablesView.prototype = { } }, - /** - * Listener handling a key down event on the view. - */ - _onViewKeyDown: function (e) { - if (e.keyCode == KeyCodes.DOM_VK_C) { - // Copy current selection to clipboard. - if (e.ctrlKey || e.metaKey) { - let item = this.getFocusedItem(); - clipboardHelper.copyString( - item._nameString + item.separatorStr + item._valueString - ); - } - } - }, - /** * Sets the text displayed in this container when there are no available items. * @param string aValue @@ -4016,9 +4007,9 @@ Editable.prototype = { input.selectionStart++; } - this._onKeypress = this._onKeypress.bind(this); + this._onKeydown = this._onKeydown.bind(this); this._onBlur = this._onBlur.bind(this); - input.addEventListener("keypress", this._onKeypress); + input.addEventListener("keydown", this._onKeydown); input.addEventListener("blur", this._onBlur); this._prevExpandable = this._variable.twisty; @@ -4034,7 +4025,7 @@ Editable.prototype = { * state. */ deactivate: function () { - this._input.removeEventListener("keypress", this._onKeypress); + this._input.removeEventListener("keydown", this._onKeydown); this._input.removeEventListener("blur", this.deactivate); this._input.parentNode.replaceChild(this.label, this._input); this._input = null; @@ -4087,7 +4078,7 @@ Editable.prototype = { /** * Event handler for when the input receives a key press. */ - _onKeypress: function (e) { + _onKeydown: function (e) { e.stopPropagation(); switch (e.keyCode) { diff --git a/devtools/client/shared/widgets/tooltip/SwatchBasedEditorTooltip.js b/devtools/client/shared/widgets/tooltip/SwatchBasedEditorTooltip.js index 2108006aba90..d294ef0158fc 100644 --- a/devtools/client/shared/widgets/tooltip/SwatchBasedEditorTooltip.js +++ b/devtools/client/shared/widgets/tooltip/SwatchBasedEditorTooltip.js @@ -213,7 +213,7 @@ class SwatchBasedEditorTooltip { } /** - * This parent class only calls this on keypress + * This parent class only calls this on keydown */ revert() { if (this.activeSwatch) { @@ -226,7 +226,7 @@ class SwatchBasedEditorTooltip { } /** - * This parent class only calls this on keypress + * This parent class only calls this on keydown */ commit() { if (this.activeSwatch) { @@ -238,7 +238,7 @@ class SwatchBasedEditorTooltip { destroy() { this.swatches.clear(); this.activeSwatch = null; - this.tooltip.off("keypress", this._onTooltipKeypress); + this.tooltip.off("keydown", this._onTooltipKeydown); this.tooltip.destroy(); this.shortcuts.destroy(); } diff --git a/devtools/client/shared/widgets/tooltip/Tooltip.js b/devtools/client/shared/widgets/tooltip/Tooltip.js index 2c5a69387ac1..f0ebb45a3ef7 100644 --- a/devtools/client/shared/widgets/tooltip/Tooltip.js +++ b/devtools/client/shared/widgets/tooltip/Tooltip.js @@ -85,7 +85,7 @@ const POPUP_EVENTS = ["shown", "hidden", "showing", "hiding"]; * - shown : when the tooltip is shown * - hiding : just before the tooltip closes * - hidden : when the tooltip gets hidden - * - keypress : when any key gets pressed, with keyCode + * - keydown : when any key gets pressed, with keyCode */ class Tooltip { @@ -131,21 +131,21 @@ class Tooltip { this["_onPopup" + eventName]); } - // Listen to keypress events to close the tooltip if configured to do so + // Listen to keydown events to close the tooltip if configured to do so let win = this.doc.querySelector("window"); - this._onKeyPress = event => { + this._onKeyDown = event => { if (this.panel.hidden) { return; } - this.emit("keypress", event.keyCode); + this.emit("keydown", event.keyCode); if (this.closeOnKeys.includes(event.keyCode) && this.isShown()) { event.stopPropagation(); this.hide(); } }; - win.addEventListener("keypress", this._onKeyPress); + win.addEventListener("keydown", this._onKeyDown); // Listen to custom emitters' events to close the tooltip this.hide = this.hide.bind(this); @@ -234,7 +234,7 @@ class Tooltip { } let win = this.doc.querySelector("window"); - win.removeEventListener("keypress", this._onKeyPress); + win.removeEventListener("keydown", this._onKeyDown); for (let {emitter, event, useCapture} of this.closeOnEvents) { for (let remove of ["removeEventListener", "off"]) { diff --git a/devtools/client/shared/widgets/view-helpers.js b/devtools/client/shared/widgets/view-helpers.js index 24f9da38d59a..67babb501552 100644 --- a/devtools/client/shared/widgets/view-helpers.js +++ b/devtools/client/shared/widgets/view-helpers.js @@ -197,7 +197,7 @@ const ViewHelpers = exports.ViewHelpers = { * Check if the enter key or space was pressed * * @param event event - * The event triggered by a keypress on an element + * The event triggered by a keydown or keypress on an element */ isSpaceOrReturn: function (event) { return event.keyCode === KeyCodes.DOM_VK_SPACE || @@ -507,7 +507,7 @@ Item.prototype = { * * For automagical keyboard and mouse accessibility, the widget should be an * event emitter with the following events: - * - "keyPress" -> (aName:string, aEvent:KeyboardEvent) + * - "keyDown" -> (aName:string, aEvent:KeyboardEvent) * - "mousePress" -> (aName:string, aEvent:MouseEvent) */ const WidgetMethods = exports.WidgetMethods = { @@ -526,7 +526,7 @@ const WidgetMethods = exports.WidgetMethods = { // Handle internal events emitted by the widget if necessary. if (ViewHelpers.isEventEmitter(widget)) { - widget.on("keyPress", this._onWidgetKeyPress.bind(this)); + widget.on("keyDown", this._onWidgetKeyDown.bind(this)); widget.on("mousePress", this._onWidgetMousePress.bind(this)); } }, @@ -1489,11 +1489,11 @@ const WidgetMethods = exports.WidgetMethods = { }, /** - * The keyPress event listener for this container. + * The keyDown event listener for this container. * @param string name * @param KeyboardEvent event */ - _onWidgetKeyPress: function (name, event) { + _onWidgetKeyDown: function (name, event) { // Prevent scrolling when pressing navigation keys. ViewHelpers.preventScrolling(event); From 173aea933e9662be83971b0962bd37aa14a449de Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Wed, 14 Feb 2018 16:38:18 +0000 Subject: [PATCH 03/48] Bug 1437850 - Enable browser_console_nsiconsolemessage.js in new frontend r=nchevobbe MozReview-Commit-ID: 7U1P72flVrO --HG-- extra : rebase_source : 128292a3c1ffad9e385554a450baa65a4c4162e3 --- .../test/mochitest/browser.ini | 1 - .../browser_console_nsiconsolemessage.js | 128 +++++++++--------- 2 files changed, 66 insertions(+), 63 deletions(-) diff --git a/devtools/client/webconsole/new-console-output/test/mochitest/browser.ini b/devtools/client/webconsole/new-console-output/test/mochitest/browser.ini index 3ff80693cad4..5dc1cebc2f7d 100644 --- a/devtools/client/webconsole/new-console-output/test/mochitest/browser.ini +++ b/devtools/client/webconsole/new-console-output/test/mochitest/browser.ini @@ -180,7 +180,6 @@ skip-if = true # Bug 1437847 [browser_console_filters.js] skip-if = true # Bug 1437848 [browser_console_nsiconsolemessage.js] -skip-if = true # Bug 1437850 [browser_console_open_or_focus.js] [browser_console_restore.js] [browser_console_webconsole_ctrlw_close_tab.js] diff --git a/devtools/client/webconsole/new-console-output/test/mochitest/browser_console_nsiconsolemessage.js b/devtools/client/webconsole/new-console-output/test/mochitest/browser_console_nsiconsolemessage.js index fedd0c71c3d7..47fb085efdbb 100644 --- a/devtools/client/webconsole/new-console-output/test/mochitest/browser_console_nsiconsolemessage.js +++ b/devtools/client/webconsole/new-console-output/test/mochitest/browser_console_nsiconsolemessage.js @@ -3,83 +3,87 @@ /* Any copyright is dedicated to the Public Domain. * http://creativecommons.org/publicdomain/zero/1.0/ */ +/* import-globals-from head.js */ + // Check that nsIConsoleMessages are displayed in the Browser Console. -// See bug 859756. "use strict"; -const TEST_URI = "data:text/html;charset=utf8,bug859756\n" + - "

hello world\n

nsIConsoleMessages ftw!"; +const TEST_URI = +`data:text/html;charset=utf8, +browser_console_nsiconsolemessage.js +

hello world

+nsIConsoleMessages ftw!`; -function test() { - const FILTER_PREF = "devtools.browserconsole.filter.jslog"; - Services.prefs.setBoolPref(FILTER_PREF, true); +add_task(async function () { + // We don't use `openNewTabAndConsole()` here because we need to log a message + // before opening the web console. + await addTab(TEST_URI); - registerCleanupFunction(() => { - Services.prefs.clearUserPref(FILTER_PREF); + // Test for cached nsIConsoleMessages. + Services.console.logStringMessage("cachedBrowserConsoleMessage"); + + info("open web console"); + let hud = await openConsole(); + + ok(hud, "web console opened"); + + // This "liveBrowserConsoleMessage" message should not be displayed. + Services.console.logStringMessage("liveBrowserConsoleMessage"); + + // Log a "foobarz" message so that we can be certain the previous message is + // not displayed. + let text = "foobarz"; + let onFooBarzMessage = waitForMessage(hud, text); + ContentTask.spawn(gBrowser.selectedBrowser, text, function (msg) { + content.console.log(msg); }); + await onFooBarzMessage; + ok(true, `"${text}" log is displayed in the Web Console as expected`); - Task.spawn(function* () { - const {tab} = yield loadTab(TEST_URI); + // Ensure the "liveBrowserConsoleMessage" and "cachedBrowserConsoleMessage" + // messages are not displayed. + text = hud.outputNode.textContent; + ok(!text.includes("cachedBrowserConsoleMessage"), + "cached nsIConsoleMessages are not displayed"); + ok(!text.includes("liveBrowserConsoleMessage"), + "nsIConsoleMessages are not displayed"); - // Test for cached nsIConsoleMessages. - Services.console.logStringMessage("test1 for bug859756"); + await closeConsole(); - info("open web console"); - let hud = yield openConsole(tab); + info("web console closed"); + hud = await HUDService.toggleBrowserConsole(); + ok(hud, "browser console opened"); - ok(hud, "web console opened"); - Services.console.logStringMessage("do-not-show-me"); + await waitFor(() => findMessage(hud, "cachedBrowserConsoleMessage")); + Services.console.logStringMessage("liveBrowserConsoleMessage2"); + await waitFor(() => findMessage(hud, "liveBrowserConsoleMessage2")); - ContentTask.spawn(gBrowser.selectedBrowser, null, function* () { - content.console.log("foobarz"); - }); + let msg = await waitFor(() => findMessage(hud, "liveBrowserConsoleMessage")); + ok(msg, "message element for liveBrowserConsoleMessage (nsIConsoleMessage)"); - yield waitForMessages({ - webconsole: hud, - messages: [{ - text: "foobarz", - category: CATEGORY_WEBDEV, - severity: SEVERITY_LOG, - }], - }); + const outputNode = hud.ui.outputNode; + const toolbar = outputNode.querySelector(".webconsole-filterbar-primary"); - let text = hud.outputNode.textContent; - is(text.indexOf("do-not-show-me"), -1, - "nsIConsoleMessages are not displayed"); - is(text.indexOf("test1 for bug859756"), -1, - "nsIConsoleMessages are not displayed (confirmed)"); + // Test that filtering is working by hiding log messages... + // show the filter bar. + toolbar.querySelector(".devtools-filter-icon").click(); + const filterBar = await waitFor(() => { + return outputNode.querySelector(".webconsole-filterbar-secondary"); + }); + ok(filterBar, "Filter bar is shown when filter icon is clicked."); - yield closeConsole(tab); + // Check that messages are not shown when their filter is turned off. + filterBar.querySelector(".log").click(); - info("web console closed"); - hud = yield HUDService.toggleBrowserConsole(); - ok(hud, "browser console opened"); + // And then checking that log messages are hidden. + await waitFor(() => findMessages(hud, "cachedBrowserConsoleMessage").length === 0); + await waitFor(() => findMessages(hud, "liveBrowserConsoleMessage").length === 0); + await waitFor(() => findMessages(hud, "liveBrowserConsoleMessage2").length === 0); - Services.console.logStringMessage("test2 for bug859756"); + // Turn the log filter off. + filterBar.querySelector(".log").click(); - let results = yield waitForMessages({ - webconsole: hud, - messages: [{ - text: "test1 for bug859756", - category: CATEGORY_JS, - }, { - text: "test2 for bug859756", - category: CATEGORY_JS, - }, { - text: "do-not-show-me", - category: CATEGORY_JS, - }], - }); - - let msg = [...results[2].matched][0]; - ok(msg, "message element for do-not-show-me (nsIConsoleMessage)"); - isnot(msg.textContent.indexOf("do-not-show"), -1, - "element content is correct"); - ok(!msg.classList.contains("filtered-by-type"), "element is not filtered"); - - hud.setFilterState("jslog", false); - - ok(msg.classList.contains("filtered-by-type"), "element is filtered"); - }).then(finishTest); -} + // Hide the filter bar. + toolbar.querySelector(".devtools-filter-icon").click(); +}); From 1e8599f22de0e724d9ad55693903833390bd2bba Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Wed, 14 Feb 2018 15:45:28 +0000 Subject: [PATCH 04/48] Bug 1437854 - Enable browser_console_webconsole_ctrlw_close_tab.js in new frontend r=Honza MozReview-Commit-ID: Ez3Qbs9FSwD --HG-- extra : rebase_source : 2e60a146da441f8eec1f38f89bb1939ba1070182 --- .../test/mochitest/browser.ini | 1 - ...wser_console_webconsole_ctrlw_close_tab.js | 20 ++++++++----------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/devtools/client/webconsole/new-console-output/test/mochitest/browser.ini b/devtools/client/webconsole/new-console-output/test/mochitest/browser.ini index 5dc1cebc2f7d..4c7f39417576 100644 --- a/devtools/client/webconsole/new-console-output/test/mochitest/browser.ini +++ b/devtools/client/webconsole/new-console-output/test/mochitest/browser.ini @@ -183,7 +183,6 @@ skip-if = true # Bug 1437848 [browser_console_open_or_focus.js] [browser_console_restore.js] [browser_console_webconsole_ctrlw_close_tab.js] -skip-if = true # Bug 1437854 [browser_console_webconsole_iframe_messages.js] [browser_console_webconsole_private_browsing.js] skip-if = true # Bug 1403188 diff --git a/devtools/client/webconsole/new-console-output/test/mochitest/browser_console_webconsole_ctrlw_close_tab.js b/devtools/client/webconsole/new-console-output/test/mochitest/browser_console_webconsole_ctrlw_close_tab.js index 65cfb3be69dc..0aef69de6503 100644 --- a/devtools/client/webconsole/new-console-output/test/mochitest/browser_console_webconsole_ctrlw_close_tab.js +++ b/devtools/client/webconsole/new-console-output/test/mochitest/browser_console_webconsole_ctrlw_close_tab.js @@ -3,25 +3,21 @@ /* Any copyright is dedicated to the Public Domain. * http://creativecommons.org/publicdomain/zero/1.0/ */ +/* import-globals-from head.js */ + // Check that Ctrl-W closes the Browser Console and that Ctrl-W closes the // current tab when using the Web Console - bug 871156. "use strict"; -add_task(function* () { +add_task(async function () { const TEST_URI = "data:text/html;charset=utf8,bug871156\n" + "

hello world"; let firstTab = gBrowser.selectedTab; - Services.prefs.setBoolPref("toolkit.cosmeticAnimations.enabled", false); - registerCleanupFunction(() => { - Services.prefs.clearUserPref("toolkit.cosmeticAnimations.enabled"); - }); + await pushPref("toolkit.cosmeticAnimations.enabled", false); - yield loadTab(TEST_URI); - - let hud = yield openConsole(); - ok(hud, "Web Console opened"); + let hud = await openNewTabAndConsole(TEST_URI); let tabClosed = defer(); let toolboxDestroyed = defer(); @@ -52,11 +48,11 @@ add_task(function* () { EventUtils.synthesizeKey("w", { accelKey: true }); }); - yield promise.all([tabClosed.promise, toolboxDestroyed.promise, + await promise.all([tabClosed.promise, toolboxDestroyed.promise, tabSelected.promise]); info("promise.all resolved. start testing the Browser Console"); - hud = yield HUDService.toggleBrowserConsole(); + hud = await HUDService.toggleBrowserConsole(); ok(hud, "Browser Console opened"); let deferred = defer(); @@ -72,5 +68,5 @@ add_task(function* () { EventUtils.synthesizeKey("w", { accelKey: true }, hud.iframeWindow); }, hud.iframeWindow); - yield deferred.promise; + await deferred.promise; }); From 992d7d94eb06a2fc0dfe063f9a18dbab15366d89 Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Fri, 23 Feb 2018 14:06:51 +0100 Subject: [PATCH 05/48] Bug 1439046 - Guard against division by zero in DynamicsCompressorKernel.cpp. r=pehrsons MozReview-Commit-ID: 2DlebpogUHm --HG-- extra : rebase_source : fb28ec70ed9bff57fec0475f08563d080c00477f --- dom/media/webaudio/blink/DynamicsCompressorKernel.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dom/media/webaudio/blink/DynamicsCompressorKernel.cpp b/dom/media/webaudio/blink/DynamicsCompressorKernel.cpp index e5b4aba2f2a7..c09b775450f2 100644 --- a/dom/media/webaudio/blink/DynamicsCompressorKernel.cpp +++ b/dom/media/webaudio/blink/DynamicsCompressorKernel.cpp @@ -39,6 +39,7 @@ using namespace std; using namespace mozilla::dom; // for WebAudioUtils using mozilla::IsInfinite; +using mozilla::PositiveInfinity; using mozilla::IsNaN; using mozilla::MakeUnique; @@ -321,7 +322,13 @@ void DynamicsCompressorKernel::process(float* sourceChannels[], bool isReleasing = scaledDesiredGain > m_compressorGain; // compressionDiffDb is the difference between current compression level and the desired level. - float compressionDiffDb = WebAudioUtils::ConvertLinearToDecibels(m_compressorGain / scaledDesiredGain, -1000.0f); + float compressionDiffDb; + if (scaledDesiredGain == 0.0) { + compressionDiffDb = PositiveInfinity(); + } else { + compressionDiffDb = WebAudioUtils::ConvertLinearToDecibels(m_compressorGain / scaledDesiredGain, -1000.0f); + } + if (isReleasing) { // Release mode - compressionDiffDb should be negative dB From e818c86453a9cf4e8c451f8a1728168ce1860c4a Mon Sep 17 00:00:00 2001 From: "Francesco Lodolo (:flod)" Date: Sun, 25 Feb 2018 19:21:26 +0100 Subject: [PATCH 06/48] Bug 1441021 - Add Occitan (oc) to shipped-locales for Firefox 60 r=Pike MozReview-Commit-ID: 5zeZqpWJJ1T --HG-- extra : rebase_source : dc2193b045930515603d5712f5dddff8752b6c14 --- browser/locales/shipped-locales | 1 + 1 file changed, 1 insertion(+) diff --git a/browser/locales/shipped-locales b/browser/locales/shipped-locales index 887895a28715..deee722d2669 100644 --- a/browser/locales/shipped-locales +++ b/browser/locales/shipped-locales @@ -70,6 +70,7 @@ nb-NO ne-NP nl nn-NO +oc or pa-IN pl From 214857579079055bfc4a5eea0f227ef2da1ed209 Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Sat, 24 Feb 2018 16:10:47 +0100 Subject: [PATCH 07/48] Bug 1432426 - Remove synchronous Bookmarks::moveItem. r=standard8 MozReview-Commit-ID: Bg5Py6aiEDw --HG-- extra : rebase_source : 1b2bbd46ee3e7c374b172553ebb9ed9da59def70 --- .../sync/tests/unit/test_bookmark_tracker.js | 92 +++----- .../places/nsINavBookmarksService.idl | 21 -- toolkit/components/places/nsNavBookmarks.cpp | 204 ------------------ toolkit/components/places/nsNavBookmarks.h | 6 - .../tests/bookmarks/test_sync_fields.js | 28 +-- .../places/tests/legacy/test_bookmarks.js | 23 -- .../places/tests/legacy/test_protectRoots.js | 5 - .../places/tests/unit/test_sync_utils.js | 39 ++-- 8 files changed, 55 insertions(+), 363 deletions(-) diff --git a/services/sync/tests/unit/test_bookmark_tracker.js b/services/sync/tests/unit/test_bookmark_tracker.js index 2a1bb7a5bc5e..378131211e4b 100644 --- a/services/sync/tests/unit/test_bookmark_tracker.js +++ b/services/sync/tests/unit/test_bookmark_tracker.js @@ -1067,48 +1067,6 @@ add_task(async function test_onLivemarkDeleted() { } }); -add_task(async function test_onItemMoved() { - _("Items moved via the synchronous API should be tracked"); - - try { - let fx_id = PlacesUtils.bookmarks.insertBookmark( - PlacesUtils.bookmarks.bookmarksMenuFolder, - CommonUtils.makeURI("http://getfirefox.com"), - PlacesUtils.bookmarks.DEFAULT_INDEX, - "Get Firefox!"); - let fx_guid = await PlacesUtils.promiseItemGuid(fx_id); - _("Firefox GUID: " + fx_guid); - let tb_id = PlacesUtils.bookmarks.insertBookmark( - PlacesUtils.bookmarks.bookmarksMenuFolder, - CommonUtils.makeURI("http://getthunderbird.com"), - PlacesUtils.bookmarks.DEFAULT_INDEX, - "Get Thunderbird!"); - let tb_guid = await PlacesUtils.promiseItemGuid(tb_id); - _("Thunderbird GUID: " + tb_guid); - - await startTracking(); - - // Moving within the folder will just track the folder. - PlacesUtils.bookmarks.moveItem( - tb_id, PlacesUtils.bookmarks.bookmarksMenuFolder, 0); - await verifyTrackedItems(["menu"]); - Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE); - await resetTracker(); - await PlacesTestUtils.markBookmarksAsSynced(); - - // Moving a bookmark to a different folder will track the old - // folder, the new folder and the bookmark. - PlacesUtils.bookmarks.moveItem(fx_id, PlacesUtils.bookmarks.toolbarFolder, - PlacesUtils.bookmarks.DEFAULT_INDEX); - await verifyTrackedItems(["menu", "toolbar", fx_guid]); - Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE); - - } finally { - _("Clean up."); - await cleanup(); - } -}); - add_task(async function test_async_onItemMoved_update() { _("Items moved via the asynchronous API should be tracked"); @@ -1263,38 +1221,42 @@ add_task(async function test_treeMoved() { try { // Create a couple of parent folders. - let folder1_id = PlacesUtils.bookmarks.createFolder( - PlacesUtils.bookmarks.bookmarksMenuFolder, - "First test folder", - PlacesUtils.bookmarks.DEFAULT_INDEX); - let folder1_guid = await PlacesUtils.promiseItemGuid(folder1_id); + let folder1 = await PlacesUtils.bookmarks.insert({ + parentGuid: PlacesUtils.bookmarks.menuGuid, + test: "First test folder", + type: PlacesUtils.bookmarks.TYPE_FOLDER + }); // A second folder in the first. - let folder2_id = PlacesUtils.bookmarks.createFolder( - folder1_id, - "Second test folder", - PlacesUtils.bookmarks.DEFAULT_INDEX); - let folder2_guid = await PlacesUtils.promiseItemGuid(folder2_id); + let folder2 = await PlacesUtils.bookmarks.insert({ + parentGuid: folder1.guid, + title: "Second test folder", + type: PlacesUtils.bookmarks.TYPE_FOLDER + }); // Create a couple of bookmarks in the second folder. - PlacesUtils.bookmarks.insertBookmark( - folder2_id, - CommonUtils.makeURI("http://getfirefox.com"), - PlacesUtils.bookmarks.DEFAULT_INDEX, - "Get Firefox!"); - PlacesUtils.bookmarks.insertBookmark( - folder2_id, - CommonUtils.makeURI("http://getthunderbird.com"), - PlacesUtils.bookmarks.DEFAULT_INDEX, - "Get Thunderbird!"); + await PlacesUtils.bookmarks.insert({ + parentGuid: folder2.guid, + url: "http://getfirefox.com", + title: "Get Firefox!" + }); + await PlacesUtils.bookmarks.insert({ + parentGuid: folder2.guid, + url: "http://getthunderbird.com", + title: "Get Thunderbird!" + }); await startTracking(); // Move folder 2 to be a sibling of folder1. - PlacesUtils.bookmarks.moveItem( - folder2_id, PlacesUtils.bookmarks.bookmarksMenuFolder, 0); + await PlacesUtils.bookmarks.update({ + guid: folder2.guid, + parentGuid: PlacesUtils.bookmarks.menuGuid, + index: 0 + }); + // the menu and both folders should be tracked, the children should not be. - await verifyTrackedItems(["menu", folder1_guid, folder2_guid]); + await verifyTrackedItems(["menu", folder1.guid, folder2.guid]); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE); } finally { _("Clean up."); diff --git a/toolkit/components/places/nsINavBookmarksService.idl b/toolkit/components/places/nsINavBookmarksService.idl index 758149716ba2..86a334ac53d7 100644 --- a/toolkit/components/places/nsINavBookmarksService.idl +++ b/toolkit/components/places/nsINavBookmarksService.idl @@ -413,27 +413,6 @@ interface nsINavBookmarksService : nsISupports void removeFolderChildren(in long long aItemId, [optional] in unsigned short aSource); - /** - * Moves an item to a different container, preserving its contents. - * @param aItemId - * The id of the item to move - * @param aNewParentId - * The id of the new parent - * @param aIndex - * The index under aNewParent, or DEFAULT_INDEX to append - * @param [optional] aSource - * The change source, forwarded to all bookmark observers. Defaults - * to SOURCE_DEFAULT. - * - * NOTE: When moving down in the same container we take into account the - * removal of the original item. If you want to move from index X to - * index Y > X you must use moveItem(id, folder, Y + 1) - */ - void moveItem(in long long aItemId, - in long long aNewParentId, - in long aIndex, - [optional] in unsigned short aSource); - /** * Set the title for an item. * @param aItemId diff --git a/toolkit/components/places/nsNavBookmarks.cpp b/toolkit/components/places/nsNavBookmarks.cpp index 4ff14fd1b5e2..ca632e31a263 100644 --- a/toolkit/components/places/nsNavBookmarks.cpp +++ b/toolkit/components/places/nsNavBookmarks.cpp @@ -1145,185 +1145,6 @@ nsNavBookmarks::RemoveFolderChildren(int64_t aFolderId, uint16_t aSource) } -NS_IMETHODIMP -nsNavBookmarks::MoveItem(int64_t aItemId, - int64_t aNewParent, - int32_t aIndex, - uint16_t aSource) -{ - NS_ENSURE_ARG(!IsRoot(aItemId)); - NS_ENSURE_ARG_MIN(aItemId, 1); - NS_ENSURE_ARG_MIN(aNewParent, 1); - // -1 is append, but no other negative number is allowed. - NS_ENSURE_ARG_MIN(aIndex, -1); - // Disallow making an item its own parent. - NS_ENSURE_ARG(aItemId != aNewParent); - - mozStorageTransaction transaction(mDB->MainConn(), false); - - BookmarkData bookmark; - nsresult rv = FetchItemInfo(aItemId, bookmark); - NS_ENSURE_SUCCESS(rv, rv); - - // if parent and index are the same, nothing to do - if (bookmark.parentId == aNewParent && bookmark.position == aIndex) - return NS_OK; - - // Make sure aNewParent is not aFolder or a subfolder of aFolder. - // TODO: make this performant, maybe with a nested tree (bug 408991). - if (bookmark.type == TYPE_FOLDER) { - int64_t ancestorId = aNewParent; - - while (ancestorId) { - if (ancestorId == bookmark.id) { - return NS_ERROR_INVALID_ARG; - } - rv = GetFolderIdForItem(ancestorId, &ancestorId); - if (NS_FAILED(rv)) { - break; - } - } - } - - // calculate new index - int32_t newIndex, folderCount; - int64_t grandParentId; - nsAutoCString newParentGuid; - rv = FetchFolderInfo(aNewParent, &folderCount, newParentGuid, &grandParentId); - NS_ENSURE_SUCCESS(rv, rv); - if (aIndex == nsINavBookmarksService::DEFAULT_INDEX || - aIndex >= folderCount) { - newIndex = folderCount; - // If the parent remains the same, then the folder is really being moved - // to count - 1 (since it's being removed from the old position) - if (bookmark.parentId == aNewParent) { - --newIndex; - } - } else { - newIndex = aIndex; - - if (bookmark.parentId == aNewParent && newIndex > bookmark.position) { - // when an item is being moved lower in the same folder, the new index - // refers to the index before it was removed. Removal causes everything - // to shift up. - --newIndex; - } - } - - // this is like the previous check, except this covers if - // the specified index was -1 (append), and the calculated - // new index is the same as the existing index - if (aNewParent == bookmark.parentId && newIndex == bookmark.position) { - // Nothing to do! - return NS_OK; - } - - // adjust indices to account for the move - // do this before we update the parent/index fields - // or we'll re-adjust the index for the item we are moving - bool sameParent = bookmark.parentId == aNewParent; - if (sameParent) { - // We can optimize the updates if moving within the same container. - // We only shift the items between the old and new positions, since the - // insertion will offset the deletion. - if (bookmark.position > newIndex) { - rv = AdjustIndices(bookmark.parentId, newIndex, bookmark.position - 1, 1); - } - else { - rv = AdjustIndices(bookmark.parentId, bookmark.position + 1, newIndex, -1); - } - NS_ENSURE_SUCCESS(rv, rv); - } - else { - // We're moving between containers, so this happens in two steps. - // First, fill the hole from the removal from the old parent. - rv = AdjustIndices(bookmark.parentId, bookmark.position + 1, INT32_MAX, -1); - NS_ENSURE_SUCCESS(rv, rv); - // Now, make room in the new parent for the insertion. - rv = AdjustIndices(aNewParent, newIndex, INT32_MAX, 1); - NS_ENSURE_SUCCESS(rv, rv); - } - - int64_t syncChangeDelta = DetermineSyncChangeDelta(aSource); - - { - // Update parent and position. - nsCOMPtr stmt = mDB->GetStatement( - "UPDATE moz_bookmarks SET parent = :parent, position = :item_index " - "WHERE id = :item_id " - ); - NS_ENSURE_STATE(stmt); - mozStorageStatementScoper scoper(stmt); - - rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("parent"), aNewParent); - NS_ENSURE_SUCCESS(rv, rv); - rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("item_index"), newIndex); - NS_ENSURE_SUCCESS(rv, rv); - rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), bookmark.id); - NS_ENSURE_SUCCESS(rv, rv); - rv = stmt->Execute(); - NS_ENSURE_SUCCESS(rv, rv); - } - - PRTime now = RoundedPRNow(); - rv = SetItemDateInternal(LAST_MODIFIED, syncChangeDelta, - bookmark.parentId, now); - NS_ENSURE_SUCCESS(rv, rv); - if (sameParent) { - // If we're moving within the same container, only the parent needs a sync - // change. Update the item's last modified date without bumping its counter. - rv = SetItemDateInternal(LAST_MODIFIED, 0, bookmark.id, now); - NS_ENSURE_SUCCESS(rv, rv); - - // Mark all affected separators as changed - int32_t startIndex = std::min(bookmark.position, newIndex); - rv = AdjustSeparatorsSyncCounter(bookmark.parentId, startIndex, syncChangeDelta); - NS_ENSURE_SUCCESS(rv, rv); - } else { - // Otherwise, if we're moving between containers, both parents and the child - // need sync changes. - rv = SetItemDateInternal(LAST_MODIFIED, syncChangeDelta, aNewParent, now); - NS_ENSURE_SUCCESS(rv, rv); - rv = SetItemDateInternal(LAST_MODIFIED, syncChangeDelta, bookmark.id, now); - NS_ENSURE_SUCCESS(rv, rv); - - // Mark all affected separators as changed - rv = AdjustSeparatorsSyncCounter(bookmark.parentId, bookmark.position, syncChangeDelta); - NS_ENSURE_SUCCESS(rv, rv); - rv = AdjustSeparatorsSyncCounter(aNewParent, newIndex, syncChangeDelta); - NS_ENSURE_SUCCESS(rv, rv); - } - - int64_t tagsRootId = TagsRootId(); - bool isChangingTagFolder = bookmark.parentId == tagsRootId; - if (isChangingTagFolder) { - // Moving a tag folder out of the tags root untags all its bookmarks. This - // is an odd case, but the tagging service adds an observer to handle it, - // so we bump the change counter for each untagged item for consistency. - rv = AddSyncChangesForBookmarksInFolder(bookmark.id, syncChangeDelta); - NS_ENSURE_SUCCESS(rv, rv); - } - - rv = PreventSyncReparenting(bookmark); - NS_ENSURE_SUCCESS(rv, rv); - - rv = transaction.Commit(); - NS_ENSURE_SUCCESS(rv, rv); - - NOTIFY_OBSERVERS(mCanNotify, mObservers, nsINavBookmarkObserver, - OnItemMoved(bookmark.id, - bookmark.parentId, - bookmark.position, - aNewParent, - newIndex, - bookmark.type, - bookmark.guid, - bookmark.parentGuid, - newParentGuid, - aSource)); - return NS_OK; -} - nsresult nsNavBookmarks::FetchItemInfo(int64_t aItemId, BookmarkData& _bookmark) @@ -1655,31 +1476,6 @@ nsNavBookmarks::RemoveTombstone(const nsACString& aGUID) } -nsresult -nsNavBookmarks::PreventSyncReparenting(const BookmarkData& aBookmark) -{ - nsCOMPtr stmt = mDB->GetStatement( - "DELETE FROM moz_items_annos WHERE " - "item_id = :item_id AND " - "anno_attribute_id = (SELECT id FROM moz_anno_attributes " - "WHERE name = :orphan_anno)" - ); - NS_ENSURE_STATE(stmt); - mozStorageStatementScoper scoper(stmt); - - nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("item_id"), aBookmark.id); - NS_ENSURE_SUCCESS(rv, rv); - rv = stmt->BindUTF8StringByName(NS_LITERAL_CSTRING("orphan_anno"), - NS_LITERAL_CSTRING(SYNC_PARENT_ANNO)); - NS_ENSURE_SUCCESS(rv, rv); - - rv = stmt->Execute(); - NS_ENSURE_SUCCESS(rv, rv); - - return NS_OK; -} - - NS_IMETHODIMP nsNavBookmarks::SetItemTitle(int64_t aItemId, const nsACString& aTitle, uint16_t aSource) diff --git a/toolkit/components/places/nsNavBookmarks.h b/toolkit/components/places/nsNavBookmarks.h index 2d86fd04abc6..8c5ffc0ca6a2 100644 --- a/toolkit/components/places/nsNavBookmarks.h +++ b/toolkit/components/places/nsNavBookmarks.h @@ -285,12 +285,6 @@ private: // Removes a stale synced bookmark tombstone. nsresult RemoveTombstone(const nsACString& aGUID); - // Removes the Sync orphan annotation from a synced item, so that Sync doesn't - // try to reparent the item once it sees the original parent. Only synced - // bookmarks should have this anno, but we do this for all bookmarks because - // the anno may be backed up and restored. - nsresult PreventSyncReparenting(const BookmarkData& aBookmark); - nsresult SetItemTitleInternal(BookmarkData& aBookmark, const nsACString& aTitle, int64_t aSyncChangeDelta); diff --git a/toolkit/components/places/tests/bookmarks/test_sync_fields.js b/toolkit/components/places/tests/bookmarks/test_sync_fields.js index c6e626277e51..fa3b56acbab1 100644 --- a/toolkit/components/places/tests/bookmarks/test_sync_fields.js +++ b/toolkit/components/places/tests/bookmarks/test_sync_fields.js @@ -67,12 +67,14 @@ class TestCases { await PlacesTestUtils.markBookmarksAsSynced(); } - info("Test 2: reparenting"); - try { - await this.testReparenting(); - } finally { - info("Reset sync fields after test 2"); - await PlacesTestUtils.markBookmarksAsSynced(); + if (("moveItem" in this) && ("reorder" in this)) { + info("Test 2: reparenting"); + try { + await this.testReparenting(); + } finally { + info("Reset sync fields after test 2"); + await PlacesTestUtils.markBookmarksAsSynced(); + } } if ("insertSeparator" in this) { @@ -267,12 +269,6 @@ class SyncTestCases extends TestCases { return PlacesUtils.promiseItemGuid(id); } - async moveItem(guid, newParentGuid, index) { - let id = await PlacesUtils.promiseItemId(guid); - let newParentId = await PlacesUtils.promiseItemId(newParentGuid); - PlacesUtils.bookmarks.moveItem(id, newParentId, index); - } - async removeItem(guid) { let id = await PlacesUtils.promiseItemId(guid); PlacesUtils.bookmarks.removeItem(id); @@ -286,14 +282,6 @@ class SyncTestCases extends TestCases { async tagURI(uri, tags) { PlacesUtils.tagging.tagURI(uri, tags); } - - async reorder(parentGuid, childGuids) { - let parentId = await PlacesUtils.promiseItemId(parentGuid); - for (let index = 0; index < childGuids.length; ++index) { - let id = await PlacesUtils.promiseItemId(childGuids[index]); - PlacesUtils.bookmarks.moveItem(id, parentId, index); - } - } } async function findTagFolder(tag) { diff --git a/toolkit/components/places/tests/legacy/test_bookmarks.js b/toolkit/components/places/tests/legacy/test_bookmarks.js index 71849dc311c8..e6449001b72c 100644 --- a/toolkit/components/places/tests/legacy/test_bookmarks.js +++ b/toolkit/components/places/tests/legacy/test_bookmarks.js @@ -246,29 +246,6 @@ add_task(async function test_bookmarks() { bs.setItemTitle(newId6, "Google Sites"); Assert.equal(bookmarksObserver._itemChangedProperty, "title"); - // move folder, appending, to different folder - let oldParentCC = getChildCount(testRoot); - bs.moveItem(workFolder, homeFolder, bs.DEFAULT_INDEX); - Assert.equal(bookmarksObserver._itemMovedId, workFolder); - Assert.equal(bookmarksObserver._itemMovedOldParent, testRoot); - Assert.equal(bookmarksObserver._itemMovedOldIndex, 0); - Assert.equal(bookmarksObserver._itemMovedNewParent, homeFolder); - Assert.equal(bookmarksObserver._itemMovedNewIndex, 1); - - // test that the new index is properly stored - Assert.equal(bs.getFolderIdForItem(workFolder), homeFolder); - - // XXX expose FolderCount, and check that the old parent has one less child? - Assert.equal(getChildCount(testRoot), oldParentCC - 1); - - // move item, appending, to different folder - bs.moveItem(newId5, testRoot, bs.DEFAULT_INDEX); - Assert.equal(bookmarksObserver._itemMovedId, newId5); - Assert.equal(bookmarksObserver._itemMovedOldParent, homeFolder); - Assert.equal(bookmarksObserver._itemMovedOldIndex, 0); - Assert.equal(bookmarksObserver._itemMovedNewParent, testRoot); - Assert.equal(bookmarksObserver._itemMovedNewIndex, 3); - // test get folder's index let tmpFolder = bs.createFolder(testRoot, "tmp", 2); diff --git a/toolkit/components/places/tests/legacy/test_protectRoots.js b/toolkit/components/places/tests/legacy/test_protectRoots.js index 30531288ade7..596f9a0367b1 100644 --- a/toolkit/components/places/tests/legacy/test_protectRoots.js +++ b/toolkit/components/places/tests/legacy/test_protectRoots.js @@ -19,11 +19,6 @@ function run_test() { do_throw("Trying to remove a root should throw"); } catch (ex) {} - try { - PlacesUtils.bookmarks.moveItem(root, PlacesUtils.placesRootId, 0); - do_throw("Trying to move a root should throw"); - } catch (ex) {} - try { PlacesUtils.bookmarks.removeFolderChildren(root); if (root == PlacesUtils.placesRootId) diff --git a/toolkit/components/places/tests/unit/test_sync_utils.js b/toolkit/components/places/tests/unit/test_sync_utils.js index 758df2ab5918..be18ef529b86 100644 --- a/toolkit/components/places/tests/unit/test_sync_utils.js +++ b/toolkit/components/places/tests/unit/test_sync_utils.js @@ -1696,17 +1696,6 @@ add_task(async function test_move_orphans() { "Should remove orphan annos from updated bookmark"); } - info("Move synced orphan using sync API"); - { - let tbId = await recordIdToId(tbBmk.recordId); - PlacesUtils.bookmarks.moveItem(tbId, PlacesUtils.toolbarFolderId, - PlacesUtils.bookmarks.DEFAULT_INDEX); - let orphanGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno( - SYNC_PARENT_ANNO, nonexistentRecordId); - deepEqual(orphanGuids, [], - "Should remove orphan annos from moved bookmark"); - } - await PlacesUtils.bookmarks.eraseEverything(); await PlacesSyncUtils.bookmarks.reset(); }); @@ -1808,9 +1797,12 @@ add_task(async function test_unsynced_orphans() { info("Move unsynced orphan"); { - let unknownId = await recordIdToId(unknownBmk.recordId); - PlacesUtils.bookmarks.moveItem(unknownId, PlacesUtils.toolbarFolderId, - PlacesUtils.bookmarks.DEFAULT_INDEX); + let unknownGuid = await PlacesSyncUtils.bookmarks.recordIdToGuid(unknownBmk.recordId); + await PlacesUtils.bookmarks.update({ + guid: unknownGuid, + parentGuid: PlacesUtils.bookmarks.toolbarGuid, + index: PlacesUtils.bookmarks.DEFAULT_INDEX + }); let orphanGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno( SYNC_PARENT_ANNO, nonexistentRecordId); deepEqual(orphanGuids.sort(), [newBmk.recordId].sort(), @@ -2590,13 +2582,17 @@ add_task(async function test_separator() { recordId: makeGuid(), url: "https://bar.foo", }); - let child2Id = await recordIdToId(childBmk.recordId); - let parentId = await recordIdToId("menu"); - let separatorId = await recordIdToId(separator.recordId); + + let child2Guid = await PlacesSyncUtils.bookmarks.recordIdToGuid(childBmk.recordId); + let parentGuid = await await PlacesSyncUtils.bookmarks.recordIdToGuid("menu"); let separatorGuid = PlacesSyncUtils.bookmarks.recordIdToGuid(separatorRecordId); info("Move a bookmark around the separator"); - PlacesUtils.bookmarks.moveItem(child2Id, parentId, separator + 1); + await PlacesUtils.bookmarks.update({ + guid: child2Guid, + parentGuid, + index: 2 + }); let changes = await PlacesSyncUtils.bookmarks.pullChanges(); deepEqual(Object.keys(changes).sort(), [separator.recordId, "menu"].sort()); @@ -2604,7 +2600,12 @@ add_task(async function test_separator() { await setChangesSynced(changes); info("Move a separator around directly"); - PlacesUtils.bookmarks.moveItem(separatorId, parentId, 0); + await PlacesUtils.bookmarks.update({ + guid: separatorGuid, + parentGuid, + index: 0 + }); + changes = await PlacesSyncUtils.bookmarks.pullChanges(); deepEqual(Object.keys(changes).sort(), [separator.recordId, "menu"].sort()); From b69949883cbd4874d459e3a5d5e9e84e59afd2d4 Mon Sep 17 00:00:00 2001 From: Jan Horak Date: Mon, 26 Feb 2018 11:44:25 +0100 Subject: [PATCH 08/48] Bug 1441108 - Don't print warning for less than 1 scaling factor; r=stransky A lot of "WARNING: Invalid monitor scale: -1" appering for the puppet widget since fix for bug 1439881 landed. We don't need to print the warning, fallback to 1 is sufficient enough. MozReview-Commit-ID: 73BGc8neUmu --HG-- extra : rebase_source : 74b2a818a29514095b5c56b05966cff5e4e9ba60 --- widget/gtk/nsNativeThemeGTK.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/widget/gtk/nsNativeThemeGTK.cpp b/widget/gtk/nsNativeThemeGTK.cpp index 006df05897e5..fafe45a102b9 100644 --- a/widget/gtk/nsNativeThemeGTK.cpp +++ b/widget/gtk/nsNativeThemeGTK.cpp @@ -81,8 +81,9 @@ GetMonitorScaleFactor(nsIFrame* aFrame) // integer to avoid rounding errors which would lead to returning 0. int monitorScale = int(round(rootWidget->GetDefaultScale().scale / gfxPlatformGtk::GetFontScaleFactor())); + // Monitor scale can be negative if it has not been initialized in the + // puppet widget yet. We also make sure that we return positive value. if (monitorScale < 1) { - NS_WARNING(nsPrintfCString("Invalid monitor scale: %d", monitorScale).get()); return 1; } return monitorScale; From adbe589f59a4fd26880abf8e2768f42d9c4532b4 Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Tue, 13 Feb 2018 15:16:37 -0500 Subject: [PATCH 09/48] Bug 1353461 - [manifestparser] Implement a chunk_by_manifest algorithm, r=jmaher This implements a chunk_by_manifest algorithm. It is similar to chunk_by_slice in that it tries to make an even number of tests run in each chunk. However, unlike chunk_by_slice it will guarantee that tests in the same manifest will all run in the same chunk. This makes it suitable to use with run-by-manifest. This means the chunks won't be perfect (as manifests are differnet sizes). It is also prone to more randomization, similar to chunk-by-runtime. In fact, this algorithm is nearly identical to the chunk-by-runtime one, so it was refactored out to a base class. MozReview-Commit-ID: HI2ByxW0i8V --HG-- extra : rebase_source : e066c034b85222d26bafe6873a80366d5bd9df9e --- layout/tools/reftest/runreftest.py | 2 +- .../manifestparser/manifestparser/filters.py | 88 ++++++++++++------- 2 files changed, 58 insertions(+), 32 deletions(-) diff --git a/layout/tools/reftest/runreftest.py b/layout/tools/reftest/runreftest.py index 6de674103442..abe114027818 100644 --- a/layout/tools/reftest/runreftest.py +++ b/layout/tools/reftest/runreftest.py @@ -828,7 +828,7 @@ class RefTest(object): filters = [] if options.totalChunks: - filters.append(mpf.chunk_by_slice(options.thisChunk, options.totalChunks)) + filters.append(mpf.chunk_by_manifest(options.thisChunk, options.totalChunks)) tests = mp.active_tests(exists=False, filters=filters) return tests diff --git a/testing/mozbase/manifestparser/manifestparser/filters.py b/testing/mozbase/manifestparser/manifestparser/filters.py index 8a92429fabbc..70b7a7fb4e95 100644 --- a/testing/mozbase/manifestparser/manifestparser/filters.py +++ b/testing/mozbase/manifestparser/manifestparser/filters.py @@ -10,9 +10,10 @@ possible to define custom filters if the built-in ones are not enough. from __future__ import absolute_import -from collections import defaultdict, MutableSequence import itertools import os +from abc import ABCMeta, abstractmethod +from collections import defaultdict, MutableSequence from .expression import ( parse, @@ -257,7 +258,56 @@ class chunk_by_dir(InstanceFilter): yield disabled_test -class chunk_by_runtime(InstanceFilter): +class ManifestChunk(InstanceFilter): + """ + Base class for chunking tests by manifest using a numerical key. + """ + __metaclass__ = ABCMeta + + def __init__(self, this_chunk, total_chunks, *args, **kwargs): + InstanceFilter.__init__(self, this_chunk, total_chunks, *args, **kwargs) + self.this_chunk = this_chunk + self.total_chunks = total_chunks + + @abstractmethod + def key(self, tests): + pass + + def __call__(self, tests, values): + tests = list(tests) + manifests = set(t['manifest'] for t in tests) + + tests_by_manifest = [] + for manifest in manifests: + mtests = [t for t in tests if t['manifest'] == manifest] + tests_by_manifest.append((self.key(mtests), mtests)) + tests_by_manifest.sort(reverse=True) + + tests_by_chunk = [[0, []] for i in range(self.total_chunks)] + for key, batch in tests_by_manifest: + # sort first by key, then by number of tests in case of a tie. + # This guarantees the chunk with the lowest key will always + # get the next batch of tests. + tests_by_chunk.sort(key=lambda x: (x[0], len(x[1]))) + tests_by_chunk[0][0] += key + tests_by_chunk[0][1].extend(batch) + + return (t for t in tests_by_chunk[self.this_chunk - 1][1]) + + +class chunk_by_manifest(ManifestChunk): + """ + Chunking algorithm that tries to evenly distribute tests while ensuring + tests in the same manifest stay together. + + :param this_chunk: the current chunk, 1 <= this_chunk <= total_chunks + :param total_chunks: the total number of chunks + """ + def key(self, tests): + return len(tests) + + +class chunk_by_runtime(ManifestChunk): """ Chunking algorithm that attempts to group tests into chunks based on their average runtimes. It keeps manifests of tests together and pairs slow @@ -272,41 +322,17 @@ class chunk_by_runtime(InstanceFilter): """ def __init__(self, this_chunk, total_chunks, runtimes, default_runtime=0): - InstanceFilter.__init__(self, this_chunk, total_chunks, runtimes, - default_runtime=default_runtime) - self.this_chunk = this_chunk - self.total_chunks = total_chunks - + ManifestChunk.__init__(self, this_chunk, total_chunks, runtimes, + default_runtime=default_runtime) # defaultdict(lambda:) assigns all non-existent keys the value of # . This means all tests we encounter that don't exist in the # runtimes file will be assigned `default_runtime`. self.runtimes = defaultdict(lambda: default_runtime) self.runtimes.update(runtimes) - def __call__(self, tests, values): - tests = list(tests) - manifests = set(t['manifest'] for t in tests) - - def total_runtime(tests): - return sum(self.runtimes[t['relpath']] for t in tests - if 'disabled' not in t) - - tests_by_manifest = [] - for manifest in manifests: - mtests = [t for t in tests if t['manifest'] == manifest] - tests_by_manifest.append((total_runtime(mtests), mtests)) - tests_by_manifest.sort(reverse=True) - - tests_by_chunk = [[0, []] for i in range(self.total_chunks)] - for runtime, batch in tests_by_manifest: - # sort first by runtime, then by number of tests in case of a tie. - # This guarantees the chunk with the fastest runtime will always - # get the next batch of tests. - tests_by_chunk.sort(key=lambda x: (x[0], len(x[1]))) - tests_by_chunk[0][0] += runtime - tests_by_chunk[0][1].extend(batch) - - return (t for t in tests_by_chunk[self.this_chunk - 1][1]) + def key(self, tests): + return sum(self.runtimes[t['relpath']] for t in tests + if 'disabled' not in t) class tags(InstanceFilter): From 463bd10a6298843a21fe2b265206db0646122d1f Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Mon, 5 Feb 2018 14:24:49 -0500 Subject: [PATCH 10/48] Bug 1353461 - [reftest] remove the start-after-crash feature, r=jmaher The point of start-after-crash was to resume running tests after a crash so we don't miss out on test coverage when a crash happens. Now that we're close to landing run-by-manifest, this feature is as necessary since only tests that appear later in the same manifest as the crashing test will be skipped. Another reason to remove it, is that it's current implementation is buggy. It assumes tests have a unique ID (they don't), which means we could accidentally get into an infinite loop if the wrong test happens to crash at the wrong time. A third reason to remove it is that it adds a lot of complexity to the harness. Especially now with run-by-manifest, supporting both would require handling a lot of different edge cases and make things much more complicated than the already are. All that being said, it would still provide value. Not only would it allow us to resume tests in the same manifest, but run-by-manifest isn't yet enabled on Android, jsreftest or crashtest (though we hope to get it enabled in those places in the future). So it may be worth re-implementing start-after-crash again based on the new manifest parsing mechanism that run-by-manifest uses. It should be possible to implement it a lot more cleanly now. This work will have to be left to a follow-up. MozReview-Commit-ID: P2hh5VecKW --HG-- extra : rebase_source : 809d5103b8d513c709aa950b102d3efee28fb205 --- layout/tools/reftest/reftest.jsm | 26 +------- layout/tools/reftest/remotereftest.py | 29 ++++---- layout/tools/reftest/runreftest.py | 95 ++++++++------------------- 3 files changed, 40 insertions(+), 110 deletions(-) diff --git a/layout/tools/reftest/reftest.jsm b/layout/tools/reftest/reftest.jsm index 8ab306a3e930..de6bafa0bb03 100644 --- a/layout/tools/reftest/reftest.jsm +++ b/layout/tools/reftest/reftest.jsm @@ -267,12 +267,6 @@ function InitAndStartRefTests() g.focusFilterMode = prefs.getCharPref("reftest.focusFilterMode"); } catch(e) {} - try { - g.startAfter = prefs.getCharPref("reftest.startAfter"); - } catch(e) { - g.startAfter = undefined; - } - try { g.compareRetainedDisplayLists = prefs.getBoolPref("reftest.compareRetainedDisplayLists"); } catch (e) {} @@ -509,7 +503,7 @@ function StartTests() g.urls = g.urls.slice(start, end); } - if (g.manageSuite && g.startAfter === undefined && !g.suiteStarted) { + if (g.manageSuite && !g.suiteStarted) { var ids = g.urls.map(function(obj) { return obj.identifier; }); @@ -519,28 +513,10 @@ function StartTests() } if (g.shuffle) { - if (g.startAfter !== undefined) { - logger.error("Can't resume from a crashed test when " + - "--shuffle is enabled, continue by shuffling " + - "all the tests"); - DoneTests(); - return; - } Shuffle(g.urls); - } else if (g.startAfter !== undefined) { - // Skip through previously crashed test - // We have to do this after chunking so we don't break the numbers - var crash_idx = g.urls.map(function(url) { - return url['url1']['spec']; - }).indexOf(g.startAfter); - if (crash_idx == -1) { - throw "Can't find the previously crashed test"; - } - g.urls = g.urls.slice(crash_idx + 1); } g.totalTests = g.urls.length; - if (!g.totalTests && !g.verify) throw "No tests to run"; diff --git a/layout/tools/reftest/remotereftest.py b/layout/tools/reftest/remotereftest.py index f7d3864a5f2b..f94ad818f70f 100644 --- a/layout/tools/reftest/remotereftest.py +++ b/layout/tools/reftest/remotereftest.py @@ -259,17 +259,13 @@ class RemoteReftest(RefTest): # may not be able to access process info for all processes continue - def createReftestProfile(self, options, startAfter=None, **kwargs): + def createReftestProfile(self, options, **kwargs): profile = RefTest.createReftestProfile(self, options, server=options.remoteWebServer, port=options.httpPort, **kwargs) - if startAfter is not None: - print ("WARNING: Continuing after a crash is not supported for remote " - "reftest yet.") profileDir = profile.profile - prefs = {} prefs["app.update.url.android"] = "" prefs["browser.firstrun.show.localepicker"] = False @@ -353,21 +349,18 @@ class RemoteReftest(RefTest): env = self.buildBrowserEnv(options, profile.profile) self.log.info("Running with e10s: {}".format(options.e10s)) - status, lastTestSeen = self.automation.runApp(None, env, - binary, - profile.profile, - cmdargs, - utilityPath=options.utilityPath, - xrePath=options.xrePath, - debuggerInfo=debuggerInfo, - symbolsPath=symbolsPath, - timeout=timeout) - if status == 1: - # when max run time exceeded, avoid restart - lastTestSeen = RefTest.TEST_SEEN_FINAL + status, self.lastTestSeen = self.automation.runApp(None, env, + binary, + profile.profile, + cmdargs, + utilityPath=options.utilityPath, + xrePath=options.xrePath, + debuggerInfo=debuggerInfo, + symbolsPath=symbolsPath, + timeout=timeout) self.cleanup(profile.profile) - return status, lastTestSeen, self.outputHandler.results + return status, self.outputHandler.results def cleanup(self, profileDir): # Pull results back from device diff --git a/layout/tools/reftest/runreftest.py b/layout/tools/reftest/runreftest.py index abe114027818..e6f0f8c10d76 100644 --- a/layout/tools/reftest/runreftest.py +++ b/layout/tools/reftest/runreftest.py @@ -8,7 +8,6 @@ Runs the reftest test harness. import collections import copy -import itertools import json import multiprocessing import os @@ -227,8 +226,6 @@ class ReftestResolver(object): class RefTest(object): - TEST_SEEN_INITIAL = 'reftest' - TEST_SEEN_FINAL = 'Main app process exited normally' oldcwd = os.getcwd() parse_manifest = True resolver_cls = ReftestResolver @@ -236,7 +233,7 @@ class RefTest(object): def __init__(self): update_mozinfo() - self.lastTestSeen = self.TEST_SEEN_INITIAL + self.lastTestSeen = None self.haveDumpedScreen = False self.resolver = self.resolver_cls() self.log = None @@ -265,8 +262,7 @@ class RefTest(object): return os.path.normpath(os.path.join(self.oldcwd, os.path.expanduser(path))) def createReftestProfile(self, options, tests=None, manifests=None, - server='localhost', port=0, profile_to_clone=None, - startAfter=None, prefs=None): + server='localhost', port=0, profile_to_clone=None, prefs=None): """Sets up a profile for reftest. :param options: Object containing command line options @@ -276,7 +272,6 @@ class RefTest(object): :param server: Server name to use for http tests :param profile_to_clone: Path to a profile to use as the basis for the test profile - :param startAfter: Start running tests after the specified test id :param prefs: Extra preferences to set in the profile """ locations = mozprofile.permissions.ServerLocations() @@ -306,10 +301,6 @@ class RefTest(object): prefs['reftest.logLevel'] = options.log_tbpl_level or 'info' prefs['reftest.suite'] = options.suite - if startAfter not in (None, self.TEST_SEEN_INITIAL, self.TEST_SEEN_FINAL): - self.log.info("Setting reftest.startAfter to %s" % startAfter) - prefs['reftest.startAfter'] = startAfter - # Unconditionally update the e10s pref. if options.e10s: prefs['browser.tabs.remote.autostart'] = True @@ -782,11 +773,9 @@ class RefTest(object): if status: msg = "TEST-UNEXPECTED-FAIL | %s | application terminated with exit code %s" % \ - (self.lastTestSeen, status) + (self.lastTestSeen, status) # use process_output so message is logged verbatim self.log.process_output(None, msg) - else: - self.lastTestSeen = self.TEST_SEEN_FINAL crashed = mozcrash.log_crashes(self.log, os.path.join(profile.profile, 'minidumps'), symbolsPath, test=self.lastTestSeen) @@ -801,7 +790,7 @@ class RefTest(object): raise exc, value, tb self.log.info("Process mode: {}".format('e10s' if options.e10s else 'non-e10s')) - return status, self.lastTestSeen, outputHandler.results + return status, outputHandler.results def getActiveTests(self, manifests, options, testDumpFile=None): # These prefs will cause reftest.jsm to parse the manifests, @@ -811,7 +800,7 @@ class RefTest(object): 'reftest.manifests.dumpTests': testDumpFile or self.testDumpFile, } cmdargs = [] # ['-headless'] - status, _, _ = self.runApp(options, cmdargs=cmdargs, prefs=prefs) + status, _ = self.runApp(options, cmdargs=cmdargs, prefs=prefs) with open(self.testDumpFile, 'r') as fh: tests = json.load(fh) @@ -846,57 +835,29 @@ class RefTest(object): ids = [t['identifier'] for t in tests] self.log.suite_start(ids, name=options.suite) - startAfter = None # When the previous run crashed, we skip the tests we ran before - prevStartAfter = None - for i in itertools.count(): - status, startAfter, results = self.runApp( - options, - tests=tests, - manifests=manifests, - cmdargs=cmdargs, - # We generally want the JS harness or marionette - # to handle timeouts if they can. - # The default JS harness timeout is currently - # 300 seconds (default options.timeout). - # The default Marionette socket timeout is - # currently 360 seconds. - # Give the JS harness extra time to deal with - # its own timeouts and try to usually exceed - # the 360 second marionette socket timeout. - # See bug 479518 and bug 1414063. - timeout=options.timeout + 70.0, - symbolsPath=options.symbolsPath, - debuggerInfo=debuggerInfo - ) - mozleak.process_leak_log(self.leakLogFile, - leak_thresholds=options.leakThresholds, - stack_fixer=get_stack_fixer_function(options.utilityPath, - options.symbolsPath)) - - if status == 0: - break - - if startAfter == self.TEST_SEEN_FINAL: - self.log.info("Finished running all tests, skipping resume " - "despite non-zero status code: %s" % status) - break - - if startAfter is not None and options.shuffle: - self.log.error("Can not resume from a crash with --shuffle " - "enabled. Please consider disabling --shuffle") - break - if startAfter is not None and options.maxRetries <= i: - self.log.error("Hit maximum number of allowed retries ({}) " - "in the test run".format(options.maxRetries)) - break - if startAfter == prevStartAfter: - # If the test stuck on the same test, or there the crashed - # test appeared more then once, stop - self.log.error("Force stop because we keep running into " - "test \"{}\"".format(startAfter)) - break - prevStartAfter = startAfter - # TODO: we need to emit an SUITE-END log if it crashed + status, results = self.runApp( + options, + tests=tests, + manifests=manifests, + cmdargs=cmdargs, + # We generally want the JS harness or marionette + # to handle timeouts if they can. + # The default JS harness timeout is currently + # 300 seconds (default options.timeout). + # The default Marionette socket timeout is + # currently 360 seconds. + # Give the JS harness extra time to deal with + # its own timeouts and try to usually exceed + # the 360 second marionette socket timeout. + # See bug 479518 and bug 1414063. + timeout=options.timeout + 70.0, + symbolsPath=options.symbolsPath, + debuggerInfo=debuggerInfo + ) + mozleak.process_leak_log(self.leakLogFile, + leak_thresholds=options.leakThresholds, + stack_fixer=get_stack_fixer_function(options.utilityPath, + options.symbolsPath)) if self.parse_manifest: self.log.suite_end(extra={'results': results}) From 608b69a9a4015edc4a071e67e03af7cfd8c671ea Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Thu, 8 Feb 2018 16:16:34 -0500 Subject: [PATCH 11/48] Bug 1353461 - [reftest] Implement run-by-manifest for reftest, r=jmaher Run-by-manifest is a mode where we restart Firefox between every manifest of tests. It sacrifices a little bit of runtime for better test isolation and improved stability. This turns run-by-manifest on for all platforms except Android. It also skips jsreftests and crashtests for now (mostly to limit the scope of what was landing all at once). Follow-ups will be filed to get it turned on in those places. MozReview-Commit-ID: DmvozAIPE5Q --HG-- extra : rebase_source : 67470894a7aa0b3189380a4874495395401909bb --- layout/tools/reftest/remotereftest.py | 11 +-- layout/tools/reftest/runreftest.py | 113 +++++++++++++++----------- 2 files changed, 70 insertions(+), 54 deletions(-) diff --git a/layout/tools/reftest/remotereftest.py b/layout/tools/reftest/remotereftest.py index f94ad818f70f..62db4ea15353 100644 --- a/layout/tools/reftest/remotereftest.py +++ b/layout/tools/reftest/remotereftest.py @@ -143,12 +143,12 @@ class ReftestServer: class RemoteReftest(RefTest): use_marionette = False - parse_manifest = False remoteApp = '' resolver_cls = RemoteReftestResolver def __init__(self, automation, devicemanager, options, scriptDir): - RefTest.__init__(self) + RefTest.__init__(self, options.suite) + self.run_by_manifest = False self.automation = automation self._devicemanager = devicemanager self.scriptDir = scriptDir @@ -279,11 +279,6 @@ class RemoteReftest(RefTest): # reftest pages at 1.0 zoom, rather than zooming to fit the CSS viewport. prefs["apz.allow_zooming"] = False - if options.totalChunks: - prefs['reftest.totalChunks'] = options.totalChunks - if options.thisChunk: - prefs['reftest.thisChunk'] = options.thisChunk - # Set the extra prefs. profile.set_preferences(prefs) @@ -360,7 +355,7 @@ class RemoteReftest(RefTest): timeout=timeout) self.cleanup(profile.profile) - return status, self.outputHandler.results + return status def cleanup(self, profileDir): # Pull results back from device diff --git a/layout/tools/reftest/runreftest.py b/layout/tools/reftest/runreftest.py index e6f0f8c10d76..c8b7258cc845 100644 --- a/layout/tools/reftest/runreftest.py +++ b/layout/tools/reftest/runreftest.py @@ -6,7 +6,6 @@ Runs the reftest test harness. """ -import collections import copy import json import multiprocessing @@ -19,6 +18,7 @@ import subprocess import sys import tempfile import threading +from collections import defaultdict from datetime import datetime, timedelta SCRIPT_DIRECTORY = os.path.abspath( @@ -227,18 +227,22 @@ class ReftestResolver(object): class RefTest(object): oldcwd = os.getcwd() - parse_manifest = True resolver_cls = ReftestResolver use_marionette = True - def __init__(self): + def __init__(self, suite): update_mozinfo() self.lastTestSeen = None self.haveDumpedScreen = False self.resolver = self.resolver_cls() self.log = None + self.outputHandler = None self.testDumpFile = os.path.join(tempfile.gettempdir(), 'reftests.json') + self.run_by_manifest = True + if suite in ('crashtest', 'jstestbrowser'): + self.run_by_manifest = False + def _populate_logger(self, options): if self.log: return @@ -307,6 +311,12 @@ class RefTest(object): else: prefs['browser.tabs.remote.autostart'] = False + if not self.run_by_manifest: + if options.totalChunks: + prefs['reftest.totalChunks'] = options.totalChunks + if options.thisChunk: + prefs['reftest.thisChunk'] = options.thisChunk + # Bug 1262954: For winXP + e10s disable acceleration if platform.system() in ("Windows", "Microsoft") and \ '5.1' in platform.version() and options.e10s: @@ -523,6 +533,7 @@ class RefTest(object): def runTests(self, tests, options, cmdargs=None): cmdargs = cmdargs or [] self._populate_logger(options) + self.outputHandler = OutputHandler(self.log, options.utilityPath, options.symbolsPath) if options.cleanupCrashes: mozcrash.cleanup_pending_crash_reports() @@ -594,7 +605,7 @@ class RefTest(object): focusThread.join() # Output the summaries that the ReftestThread filters suppressed. - summaryObjects = [collections.defaultdict(int) for s in summaryLines] + summaryObjects = [defaultdict(int) for s in summaryLines] for t in threads: for (summaryObj, (text, categories)) in zip(summaryObjects, summaryLines): threadMatches = t.summaryMatches[text] @@ -668,6 +679,7 @@ class RefTest(object): if cmdargs is None: cmdargs = [] + cmdargs = cmdargs[:] if self.use_marionette: cmdargs.append('-marionette') @@ -700,19 +712,17 @@ class RefTest(object): self.log.add_handler(record_last_test) - outputHandler = OutputHandler(self.log, options.utilityPath, symbolsPath=symbolsPath) - kp_kwargs = { 'kill_on_timeout': False, 'cwd': SCRIPT_DIRECTORY, 'onTimeout': [timeoutHandler], - 'processOutputLine': [outputHandler], + 'processOutputLine': [self.outputHandler], } if mozinfo.isWin: # Prevents log interleaving on Windows at the expense of losing # true log order. See bug 798300 and bug 1324961 for more details. - kp_kwargs['processStderrLine'] = [outputHandler] + kp_kwargs['processStderrLine'] = [self.outputHandler] if interactive: # If an interactive debugger is attached, @@ -732,7 +742,7 @@ class RefTest(object): interactive=interactive, outputTimeout=timeout) proc = runner.process_handler - outputHandler.proc_name = 'GECKO({})'.format(proc.pid) + self.outputHandler.proc_name = 'GECKO({})'.format(proc.pid) # Used to defer a possible IOError exception from Marionette marionette_exception = None @@ -769,7 +779,7 @@ class RefTest(object): status = runner.wait() runner.process_handler = None - outputHandler.proc_name = None + self.outputHandler.proc_name = None if status: msg = "TEST-UNEXPECTED-FAIL | %s | application terminated with exit code %s" % \ @@ -778,7 +788,7 @@ class RefTest(object): self.log.process_output(None, msg) crashed = mozcrash.log_crashes(self.log, os.path.join(profile.profile, 'minidumps'), - symbolsPath, test=self.lastTestSeen) + options.symbolsPath, test=self.lastTestSeen) if not status and crashed: status = 1 @@ -790,7 +800,7 @@ class RefTest(object): raise exc, value, tb self.log.info("Process mode: {}".format('e10s' if options.e10s else 'non-e10s')) - return status, outputHandler.results + return status def getActiveTests(self, manifests, options, testDumpFile=None): # These prefs will cause reftest.jsm to parse the manifests, @@ -799,8 +809,8 @@ class RefTest(object): 'reftest.manifests': json.dumps(manifests), 'reftest.manifests.dumpTests': testDumpFile or self.testDumpFile, } - cmdargs = [] # ['-headless'] - status, _ = self.runApp(options, cmdargs=cmdargs, prefs=prefs) + cmdargs = [] + self.runApp(options, cmdargs=cmdargs, prefs=prefs) with open(self.testDumpFile, 'r') as fh: tests = json.load(fh) @@ -828,40 +838,51 @@ class RefTest(object): debuggerInfo = mozdebug.get_debugger_info(options.debugger, options.debuggerArgs, options.debuggerInteractive) - tests = None - if self.parse_manifest: - tests = self.getActiveTests(manifests, options) + def run(**kwargs): + status = self.runApp( + options, + manifests=manifests, + cmdargs=cmdargs, + # We generally want the JS harness or marionette + # to handle timeouts if they can. + # The default JS harness timeout is currently + # 300 seconds (default options.timeout). + # The default Marionette socket timeout is + # currently 360 seconds. + # Give the JS harness extra time to deal with + # its own timeouts and try to usually exceed + # the 360 second marionette socket timeout. + # See bug 479518 and bug 1414063. + timeout=options.timeout + 70.0, + debuggerInfo=debuggerInfo, + **kwargs) - ids = [t['identifier'] for t in tests] - self.log.suite_start(ids, name=options.suite) + mozleak.process_leak_log(self.leakLogFile, + leak_thresholds=options.leakThresholds, + stack_fixer=get_stack_fixer_function(options.utilityPath, + options.symbolsPath)) + return status - status, results = self.runApp( - options, - tests=tests, - manifests=manifests, - cmdargs=cmdargs, - # We generally want the JS harness or marionette - # to handle timeouts if they can. - # The default JS harness timeout is currently - # 300 seconds (default options.timeout). - # The default Marionette socket timeout is - # currently 360 seconds. - # Give the JS harness extra time to deal with - # its own timeouts and try to usually exceed - # the 360 second marionette socket timeout. - # See bug 479518 and bug 1414063. - timeout=options.timeout + 70.0, - symbolsPath=options.symbolsPath, - debuggerInfo=debuggerInfo - ) - mozleak.process_leak_log(self.leakLogFile, - leak_thresholds=options.leakThresholds, - stack_fixer=get_stack_fixer_function(options.utilityPath, - options.symbolsPath)) + if not self.run_by_manifest: + return run() - if self.parse_manifest: - self.log.suite_end(extra={'results': results}) - return status + tests = self.getActiveTests(manifests, options) + tests_by_manifest = defaultdict(list) + ids_by_manifest = defaultdict(list) + for t in tests: + tests_by_manifest[t['manifest']].append(t) + ids_by_manifest[t['manifest']].append(t['identifier']) + + self.log.suite_start(ids_by_manifest, name=options.suite) + + overall = 0 + for manifest, tests in tests_by_manifest.items(): + self.log.info("Running tests in {}".format(manifest)) + status = run(tests=tests) + overall = overall or status + + self.log.suite_end(extra={'results': self.outputHandler.results}) + return overall def copyExtraFilesToProfile(self, options, profile): "Copy extra files or dirs specified on the command line to the testing profile." @@ -889,7 +910,7 @@ class RefTest(object): def run_test_harness(parser, options): - reftest = RefTest() + reftest = RefTest(options.suite) parser.validate(options, reftest) # We have to validate options.app here for the case when the mach From cd242ed7e970d92d47fea2a30c0d86896cac5d32 Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Fri, 23 Feb 2018 15:31:04 -0500 Subject: [PATCH 12/48] Bug 1353461 - Chunk OSX debug reftests to 3, r=jmaher With run-by-manifest, the debug reftests timeout on OSX. Increasing the chunks by 1 seems to improve them. MozReview-Commit-ID: 14xidhwXCue --HG-- extra : rebase_source : b3042c3a65c67fc54af5d64624427593e48c8364 --- taskcluster/ci/test/reftest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taskcluster/ci/test/reftest.yml b/taskcluster/ci/test/reftest.yml index 43fa14ef95ad..8dad9f585d82 100644 --- a/taskcluster/ci/test/reftest.yml +++ b/taskcluster/ci/test/reftest.yml @@ -100,7 +100,7 @@ reftest: android-4.3-arm7-api-16/debug: 56 android.*: 28 macosx64.*/opt: 1 - macosx64.*/debug: 2 + macosx64.*/debug: 3 windows10-64.*/opt: 1 windows10-64.*/debug: 2 default: 8 From ec94dfd4df09fdb3f5e83e39ca21aadc21636b94 Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Fri, 23 Feb 2018 15:50:47 -0500 Subject: [PATCH 13/48] Bug 1353461 - Increase fuzzy-if for layout/reftests/svg/foreignObject-form-theme.svg with webrender, r=jmaher This test already has a fuzzy-if annotation with webrender, but when run-by-manifest is enabled, the fuzz increases. The fix will be tracked in bug 1439980. MozReview-Commit-ID: 14xidhwXCue --HG-- extra : rebase_source : cc16f16421478189dc367a739c84e5894c58366c --- layout/reftests/svg/reftest.list | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/layout/reftests/svg/reftest.list b/layout/reftests/svg/reftest.list index 4c47f79be907..cca38b5f53fc 100644 --- a/layout/reftests/svg/reftest.list +++ b/layout/reftests/svg/reftest.list @@ -216,7 +216,7 @@ fuzzy-if(/^Windows\x20NT\x2010\.0/.test(http.oscpu)||skiaContent,1,800000) == fi == foreignObject-ancestor-style-change-01.svg foreignObject-ancestor-style-change-01-ref.svg == foreignObject-change-transform-01.svg pass.svg == foreignObject-display-01.svg pass.svg -fuzzy-if(webrender,1,35) == foreignObject-form-theme.svg foreignObject-form-theme-ref.html +fuzzy-if(webrender,190,229) == foreignObject-form-theme.svg foreignObject-form-theme-ref.html # Bug 1439980 == foreignObject-img.html foreignObject-img-ref.html == foreignObject-img-form-theme.html foreignObject-img-form-theme-ref.html == foreignObject-move-repaint-01.svg pass.svg From 0ecd6cd8a461f8d1fc1b7cc3eb698103c1c95510 Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Fri, 23 Feb 2018 15:52:38 -0500 Subject: [PATCH 14/48] Bug 1353461 - Mark toolkit/content/tests/reftests/bug-442419-progressmeter-max.xul fails on Windows, r=jmaher This test is already marked fails-if on Windows 8 and all OSX. With run-by-manifest enabled it also starts failing on Windows 7 and 10. The fix will be tracked in bug 1439988. MozReview-Commit-ID: 14xidhwXCue --HG-- extra : rebase_source : 391fbc0d0b3ee349cde9cb5d57ac9c3cb1d36bcc --- toolkit/content/tests/reftests/reftest.list | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/content/tests/reftests/reftest.list b/toolkit/content/tests/reftests/reftest.list index 3cb36a171b4d..820b8b30d710 100644 --- a/toolkit/content/tests/reftests/reftest.list +++ b/toolkit/content/tests/reftests/reftest.list @@ -1,4 +1,4 @@ -random-if(cocoaWidget||(/^Windows\x20NT\x206\.2/.test(http.oscpu)&&isDebugBuild)) == bug-442419-progressmeter-max.xul bug-442419-progressmeter-max-ref.xul # fails most of the time on Mac because progress meter animates +fails-if(/^Windows\x20NT\x206\.1/.test(http.oscpu)&&!isDebugBuild) random-if(cocoaWidget||/^Windows\x20NT\x206\.2/.test(http.oscpu)||(/^Windows\x20NT\x2010\.0/.test(http.oscpu)&&!isDebugBuild)) == bug-442419-progressmeter-max.xul bug-442419-progressmeter-max-ref.xul # fails most of the time on Mac because progress meter animates; Bug 1439988 != textbox-multiline-default-value.xul textbox-multiline-empty.xul == videocontrols-dynamically-add-cc.html videocontrols-dynamically-add-cc-ref.html == audio-with-bogus-url.html audio-with-bogus-url-ref.html From 1a67e265c1567e6ff936f2d2dd66008c49247b5a Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Wed, 21 Feb 2018 20:34:19 +0000 Subject: [PATCH 15/48] Bug 1439673 - Fix React 16 warnings r=nchevobbe I believe this fixes all warnings except for one. This warning appears the first time the memory panel is selected: "Warning: Failed prop type: Calling PropTypes validators directly is not supported by the `prop-types` package. Use `PropTypes.checkPropTypes()` to call them. Read more at http://fb.me/use-check-prop-types in MemoryApp (created by Connect(MemoryApp)) in Connect(MemoryApp) (created by bound createElementWithValidation) in bound createElementWithValidation in Provider" This appears to be an issue with `devtools/client/memory/app.js` but I will log a new bug for this. MozReview-Commit-ID: 341zdQyfgrN --HG-- extra : rebase_source : ce25569407b89a7547e4db688373b202d17d2475 --- .../dom/content/components/main-toolbar.js | 2 ++ .../framework/components/toolbox-tabs.js | 1 + .../boxmodel/components/BoxModelProperties.js | 3 ++- devtools/client/inspector/fonts/fonts.js | 8 +++++--- .../memory/components/SnapshotListItem.js | 1 - devtools/client/memory/components/Toolbar.js | 1 - .../netmonitor/src/components/MonitorPanel.js | 6 +++--- .../src/components/RequestListContent.js | 2 +- .../src/components/RequestListItem.js | 2 +- .../components/waterfall-header.js | 1 + devtools/client/shared/components/Frame.js | 2 +- .../shared/components/splitter/SplitBox.js | 20 +++++++++++++++---- .../components/ConsoleOutput.js | 2 +- .../new-console-output/components/Message.js | 3 +-- 14 files changed, 35 insertions(+), 19 deletions(-) diff --git a/devtools/client/dom/content/components/main-toolbar.js b/devtools/client/dom/content/components/main-toolbar.js index a92ba63f3458..9a73bcf4953e 100644 --- a/devtools/client/dom/content/components/main-toolbar.js +++ b/devtools/client/dom/content/components/main-toolbar.js @@ -52,12 +52,14 @@ class MainToolbar extends Component { return ( Toolbar({}, ToolbarButton({ + key: "refresh", className: "refresh devtools-button", id: "dom-refresh-button", title: l10n.getStr("dom.refresh"), onClick: this.onRefresh }), SearchBox({ + key: "filter", delay: 250, onChange: this.onSearch, placeholder: l10n.getStr("dom.filterDOMPanel"), diff --git a/devtools/client/framework/components/toolbox-tabs.js b/devtools/client/framework/components/toolbox-tabs.js index 521121c31d5c..e1605b49e051 100644 --- a/devtools/client/framework/components/toolbox-tabs.js +++ b/devtools/client/framework/components/toolbox-tabs.js @@ -94,6 +94,7 @@ class ToolboxTabs extends Component { } = this.props; let tabs = panelDefinitions.map(panelDefinition => ToolboxTab({ + key: panelDefinition.id, currentToolId, focusButton, focusedButton, diff --git a/devtools/client/inspector/boxmodel/components/BoxModelProperties.js b/devtools/client/inspector/boxmodel/components/BoxModelProperties.js index 4f0ce458cd52..41c671d5e6ca 100644 --- a/devtools/client/inspector/boxmodel/components/BoxModelProperties.js +++ b/devtools/client/inspector/boxmodel/components/BoxModelProperties.js @@ -63,10 +63,11 @@ class BoxModelProperties extends PureComponent { return {}; } - onToggleExpander() { + onToggleExpander(event) { this.setState({ isOpen: !this.state.isOpen, }); + event.stopPropagation(); } render() { diff --git a/devtools/client/inspector/fonts/fonts.js b/devtools/client/inspector/fonts/fonts.js index b36729942902..d2186895072b 100644 --- a/devtools/client/inspector/fonts/fonts.js +++ b/devtools/client/inspector/fonts/fonts.js @@ -38,6 +38,11 @@ class FontInspector { this.init(); } + componentWillMount() { + this.store.dispatch(updatePreviewText("")); + this.update(false, ""); + } + init() { if (!this.inspector) { return; @@ -62,9 +67,6 @@ class FontInspector { // Listen for theme changes as the color of the previews depend on the theme gDevTools.on("theme-switched", this.onThemeChanged); - - this.store.dispatch(updatePreviewText("")); - this.update(false, ""); } /** diff --git a/devtools/client/memory/components/SnapshotListItem.js b/devtools/client/memory/components/SnapshotListItem.js index 2f7d49c97c35..c8d1b7f6ea29 100644 --- a/devtools/client/memory/components/SnapshotListItem.js +++ b/devtools/client/memory/components/SnapshotListItem.js @@ -94,7 +94,6 @@ class SnapshotListItem extends Component { let deleteButton = !snapshot.path ? void 0 : dom.div({ onClick: () => onDelete(snapshot), className: "delete", - "aria-role": "button", title: L10N.getStr("snapshot.io.delete") }); diff --git a/devtools/client/memory/components/Toolbar.js b/devtools/client/memory/components/Toolbar.js index 6f9a8a0fc699..8cded90d9915 100644 --- a/devtools/client/memory/components/Toolbar.js +++ b/devtools/client/memory/components/Toolbar.js @@ -205,7 +205,6 @@ class Toolbar extends Component { { id: "select-view", onChange: e => onViewChange(e.target.value), - defaultValue: view, value: view.state, }, dom.option( diff --git a/devtools/client/netmonitor/src/components/MonitorPanel.js b/devtools/client/netmonitor/src/components/MonitorPanel.js index f59bfd9f76b8..40728620c0d9 100644 --- a/devtools/client/netmonitor/src/components/MonitorPanel.js +++ b/devtools/client/netmonitor/src/components/MonitorPanel.js @@ -41,7 +41,7 @@ class MonitorPanel extends Component { networkDetailsOpen: PropTypes.bool.isRequired, openNetworkDetails: PropTypes.func.isRequired, request: PropTypes.object, - selectedRequestVisible: PropTypes.func.isRequired, + selectedRequestVisible: PropTypes.bool.isRequired, sourceMapService: PropTypes.object, openLink: PropTypes.func, updateRequest: PropTypes.func.isRequired, @@ -113,8 +113,8 @@ class MonitorPanel extends Component { Toolbar({ connector }), SplitBox({ className: "devtools-responsive-container", - initialWidth: `${initialWidth}px`, - initialHeight: `${initialHeight}px`, + initialWidth: initialWidth, + initialHeight: initialHeight, minSize: "50px", maxSize: "80%", splitterSize: 1, diff --git a/devtools/client/netmonitor/src/components/RequestListContent.js b/devtools/client/netmonitor/src/components/RequestListContent.js index 2107f19cc334..7408262bc806 100644 --- a/devtools/client/netmonitor/src/components/RequestListContent.js +++ b/devtools/client/netmonitor/src/components/RequestListContent.js @@ -63,7 +63,7 @@ class RequestListContent extends Component { scale: PropTypes.number, selectedRequest: PropTypes.object, sortedRequests: PropTypes.array.isRequired, - requestFilterTypes: PropTypes.string.isRequired, + requestFilterTypes: PropTypes.object.isRequired, }; } diff --git a/devtools/client/netmonitor/src/components/RequestListItem.js b/devtools/client/netmonitor/src/components/RequestListItem.js index c164d0e1edaa..6de1cf32e691 100644 --- a/devtools/client/netmonitor/src/components/RequestListItem.js +++ b/devtools/client/netmonitor/src/components/RequestListItem.js @@ -138,7 +138,7 @@ class RequestListItem extends Component { onMouseDown: PropTypes.func.isRequired, onSecurityIconMouseDown: PropTypes.func.isRequired, onWaterfallMouseDown: PropTypes.func.isRequired, - requestFilterTypes: PropTypes.string.isRequired, + requestFilterTypes: PropTypes.object.isRequired, waterfallWidth: PropTypes.number, }; } diff --git a/devtools/client/performance/components/waterfall-header.js b/devtools/client/performance/components/waterfall-header.js index ab46ca6a3dc1..fa611fef0cb7 100644 --- a/devtools/client/performance/components/waterfall-header.js +++ b/devtools/client/performance/components/waterfall-header.js @@ -33,6 +33,7 @@ function WaterfallHeader(props) { let label = L10N.getFormatStr("timeline.tick", time); let node = dom.div({ + key: x, className: "plain waterfall-header-tick", style: { transform: `translateX(${left}px)` } }, label); diff --git a/devtools/client/shared/components/Frame.js b/devtools/client/shared/components/Frame.js index 027aa56e4f9f..2efac755ef76 100644 --- a/devtools/client/shared/components/Frame.js +++ b/devtools/client/shared/components/Frame.js @@ -25,7 +25,7 @@ class Frame extends Component { column: PropTypes.oneOfType([ PropTypes.string, PropTypes.number ]), }).isRequired, // Clicking on the frame link -- probably should link to the debugger. - onClick: PropTypes.func.isRequired, + onClick: PropTypes.func, // Option to display a function name before the source link. showFunctionName: PropTypes.bool, // Option to display a function name even if it's anonymous. diff --git a/devtools/client/shared/components/splitter/SplitBox.js b/devtools/client/shared/components/splitter/SplitBox.js index 68f0f395d3ce..553f3ee8b6b1 100644 --- a/devtools/client/shared/components/splitter/SplitBox.js +++ b/devtools/client/shared/components/splitter/SplitBox.js @@ -22,15 +22,27 @@ class SplitBox extends Component { // Initial size of controlled panel. initialSize: PropTypes.string, // Initial width of controlled panel. - initialWidth: PropTypes.string, + initialWidth: PropTypes.oneOfType([ + PropTypes.number, + PropTypes.string + ]), // Initial height of controlled panel. - initialHeight: PropTypes.string, + initialHeight: PropTypes.oneOfType([ + PropTypes.number, + PropTypes.string + ]), // Left/top panel startPanel: PropTypes.any, // Min panel size. - minSize: PropTypes.string, + minSize: PropTypes.oneOfType([ + PropTypes.number, + PropTypes.string + ]), // Max panel size. - maxSize: PropTypes.string, + maxSize: PropTypes.oneOfType([ + PropTypes.number, + PropTypes.string + ]), // Right/bottom panel endPanel: PropTypes.any, // True if the right/bottom panel should be controlled. diff --git a/devtools/client/webconsole/new-console-output/components/ConsoleOutput.js b/devtools/client/webconsole/new-console-output/components/ConsoleOutput.js index e95d3d9b64b3..6312759c90a5 100644 --- a/devtools/client/webconsole/new-console-output/components/ConsoleOutput.js +++ b/devtools/client/webconsole/new-console-output/components/ConsoleOutput.js @@ -31,7 +31,7 @@ class ConsoleOutput extends Component { return { initialized: PropTypes.bool.isRequired, messages: PropTypes.object.isRequired, - messagesUi: PropTypes.object.isRequired, + messagesUi: PropTypes.array.isRequired, serviceContainer: PropTypes.shape({ attachRefToHud: PropTypes.func.isRequired, openContextMenu: PropTypes.func.isRequired, diff --git a/devtools/client/webconsole/new-console-output/components/Message.js b/devtools/client/webconsole/new-console-output/components/Message.js index b4f61a7644a6..11056581172c 100644 --- a/devtools/client/webconsole/new-console-output/components/Message.js +++ b/devtools/client/webconsole/new-console-output/components/Message.js @@ -46,7 +46,6 @@ class Message extends Component { messageId: PropTypes.string, scrollToMessage: PropTypes.bool, exceptionDocURL: PropTypes.string, - parameters: PropTypes.object, request: PropTypes.object, dispatch: PropTypes.func, timeStamp: PropTypes.number, @@ -261,7 +260,7 @@ class Message extends Component { dom.span({ className: "message-body-wrapper" }, dom.span({ className: "message-flex-body", - onClick: collapsible && this.toggleMessage, + onClick: collapsible ? this.toggleMessage : undefined, }, // Add whitespaces for formatting when copying to the clipboard. timestampEl ? " " : null, From 1561bd419e7770f9876cabcca61f652dfe761995 Mon Sep 17 00:00:00 2001 From: Eugen Sawin Date: Tue, 20 Feb 2018 15:37:06 +0200 Subject: [PATCH 16/48] Bug 1410456 - Allow OMT access to Android system audio properties. r=esawin MozReview-Commit-ID: 4YkiuzNkNu5 --HG-- extra : rebase_source : 878486dabb87c8913fbb4c3b4002483158c467d4 --- .../src/main/java/org/mozilla/gecko/GeckoAppShell.java | 4 ++-- widget/android/GeneratedJNIWrappers.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java index d17c70913354..7dbb286cae04 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java @@ -1836,7 +1836,7 @@ public class GeckoAppShell return sScreenSize; } - @WrapForJNI(calledFrom = "gecko") + @WrapForJNI(calledFrom = "any") public static int getAudioOutputFramesPerBuffer() { if (SysInfo.getVersion() < 17) { return 0; @@ -1853,7 +1853,7 @@ public class GeckoAppShell return Integer.parseInt(prop); } - @WrapForJNI(calledFrom = "gecko") + @WrapForJNI(calledFrom = "any") public static int getAudioOutputSampleRate() { if (SysInfo.getVersion() < 17) { return 0; diff --git a/widget/android/GeneratedJNIWrappers.h b/widget/android/GeneratedJNIWrappers.h index 3eb178b0894e..46c780942619 100644 --- a/widget/android/GeneratedJNIWrappers.h +++ b/widget/android/GeneratedJNIWrappers.h @@ -814,7 +814,7 @@ public: static const mozilla::jni::ExceptionMode exceptionMode = mozilla::jni::ExceptionMode::ABORT; static const mozilla::jni::CallingThread callingThread = - mozilla::jni::CallingThread::GECKO; + mozilla::jni::CallingThread::ANY; static const mozilla::jni::DispatchTarget dispatchTarget = mozilla::jni::DispatchTarget::CURRENT; }; @@ -833,7 +833,7 @@ public: static const mozilla::jni::ExceptionMode exceptionMode = mozilla::jni::ExceptionMode::ABORT; static const mozilla::jni::CallingThread callingThread = - mozilla::jni::CallingThread::GECKO; + mozilla::jni::CallingThread::ANY; static const mozilla::jni::DispatchTarget dispatchTarget = mozilla::jni::DispatchTarget::CURRENT; }; From 816b44e1a0be9c38fd1e4ffcead2fb09450de326 Mon Sep 17 00:00:00 2001 From: Alex Chronopoulos Date: Tue, 20 Feb 2018 15:37:07 +0200 Subject: [PATCH 17/48] Bug 1410456 - use jni methods in place of removed cubeb methods. r=padenot MozReview-Commit-ID: 18fQVZeYAgk --HG-- extra : rebase_source : 67b9df3bdfb22d2baa05a15f6535af8d3d5c12d9 --- dom/media/CubebUtils.cpp | 50 +++++++++++++++++++++++++++++++++------ dom/media/CubebUtils.h | 7 +++++- dom/media/GraphDriver.cpp | 10 +------- 3 files changed, 50 insertions(+), 17 deletions(-) diff --git a/dom/media/CubebUtils.cpp b/dom/media/CubebUtils.cpp index 9c47c2f2ea55..4cda851197a7 100644 --- a/dom/media/CubebUtils.cpp +++ b/dom/media/CubebUtils.cpp @@ -25,6 +25,9 @@ #include "prdtoa.h" #include #include +#ifdef MOZ_WIDGET_ANDROID +#include "GeneratedJNIWrappers.h" +#endif #define PREF_VOLUME_SCALE "media.volume_scale" #define PREF_CUBEB_BACKEND "media.cubeb.backend" @@ -119,8 +122,8 @@ cubeb* sCubebContext; double sVolumeScale = 1.0; uint32_t sCubebPlaybackLatencyInMilliseconds = 100; uint32_t sCubebMSGLatencyInFrames = 512; -bool sCubebPlaybackLatencyPrefSet; -bool sCubebMSGLatencyPrefSet; +bool sCubebPlaybackLatencyPrefSet = false; +bool sCubebMSGLatencyPrefSet = false; bool sAudioStreamInitEverSucceeded = false; #ifdef MOZ_CUBEB_REMOTING bool sCubebSandbox; @@ -305,11 +308,15 @@ bool InitPreferredSampleRate() if (!context) { return false; } +#ifdef MOZ_WIDGET_ANDROID + sPreferredSampleRate = AndroidGetAudioOutputSampleRate(); +#else if (cubeb_get_preferred_sample_rate(context, &sPreferredSampleRate) != CUBEB_OK) { return false; } +#endif MOZ_ASSERT(sPreferredSampleRate); return true; } @@ -527,14 +534,28 @@ bool CubebMSGLatencyPrefSet() return sCubebMSGLatencyPrefSet; } -Maybe GetCubebMSGLatencyInFrames() +uint32_t GetCubebMSGLatencyInFrames(cubeb_stream_params * params) { StaticMutexAutoLock lock(sMutex); - if (!sCubebMSGLatencyPrefSet) { - return Maybe(); + if (sCubebMSGLatencyPrefSet) { + MOZ_ASSERT(sCubebMSGLatencyInFrames > 0); + return sCubebMSGLatencyInFrames; } - MOZ_ASSERT(sCubebMSGLatencyInFrames > 0); - return Some(sCubebMSGLatencyInFrames); + +#ifdef MOZ_WIDGET_ANDROID + return AndroidGetAudioOutputFramesPerBuffer(); +#else + cubeb* context = GetCubebContextUnlocked(); + if (!context) { + return sCubebMSGLatencyInFrames; // default 512 + } + uint32_t latency_frames = 0; + if (cubeb_get_min_latency(context, params, &latency_frames) != CUBEB_OK) { + NS_WARNING("Could not get minimal latency from cubeb."); + return sCubebMSGLatencyInFrames; // default 512 + } + return latency_frames; +#endif } void InitLibrary() @@ -741,5 +762,20 @@ void GetDeviceCollection(nsTArray>& aDeviceInfos, } } +#ifdef MOZ_WIDGET_ANDROID +uint32_t AndroidGetAudioOutputSampleRate() +{ + int32_t sample_rate = java::GeckoAppShell::GetAudioOutputSampleRate(); + MOZ_ASSERT(sample_rate > 0); + return sample_rate; +} +uint32_t AndroidGetAudioOutputFramesPerBuffer() +{ + int32_t frames = java::GeckoAppShell::GetAudioOutputFramesPerBuffer(); + MOZ_ASSERT(frames > 0); + return frames; +} +#endif + } // namespace CubebUtils } // namespace mozilla diff --git a/dom/media/CubebUtils.h b/dom/media/CubebUtils.h index 60e9eb1e477e..79f52d9714e6 100644 --- a/dom/media/CubebUtils.h +++ b/dom/media/CubebUtils.h @@ -44,7 +44,7 @@ cubeb* GetCubebContext(); void ReportCubebStreamInitFailure(bool aIsFirstStream); void ReportCubebBackendUsed(); uint32_t GetCubebPlaybackLatencyInMilliseconds(); -Maybe GetCubebMSGLatencyInFrames(); +uint32_t GetCubebMSGLatencyInFrames(cubeb_stream_params * params); bool CubebLatencyPrefSet(); cubeb_channel_layout ConvertChannelMapToCubebLayout(uint32_t aChannelMap); void GetCurrentBackend(nsAString& aBackend); @@ -52,6 +52,11 @@ void GetPreferredChannelLayout(nsAString& aLayout); void GetDeviceCollection(nsTArray>& aDeviceInfos, Side aSide); cubeb_channel_layout GetPreferredChannelLayoutOrSMPTE(cubeb* context, uint32_t aChannels); + +#ifdef MOZ_WIDGET_ANDROID +uint32_t AndroidGetAudioOutputSampleRate(); +uint32_t AndroidGetAudioOutputFramesPerBuffer(); +#endif } // namespace CubebUtils } // namespace mozilla diff --git a/dom/media/GraphDriver.cpp b/dom/media/GraphDriver.cpp index f3d8c05d6be5..62d10cce42c6 100644 --- a/dom/media/GraphDriver.cpp +++ b/dom/media/GraphDriver.cpp @@ -599,7 +599,6 @@ AudioCallbackDriver::Init() cubeb_stream_params output; cubeb_stream_params input; - uint32_t latency_frames; bool firstStream = CubebUtils::GetFirstStream(); MOZ_ASSERT(!NS_IsMainThread(), @@ -629,14 +628,7 @@ AudioCallbackDriver::Init() output.layout = CubebUtils::GetPreferredChannelLayoutOrSMPTE(cubebContext, mOutputChannels); output.prefs = CUBEB_STREAM_PREF_NONE; - Maybe latencyPref = CubebUtils::GetCubebMSGLatencyInFrames(); - if (latencyPref) { - latency_frames = latencyPref.value(); - } else { - if (cubeb_get_min_latency(cubebContext, &output, &latency_frames) != CUBEB_OK) { - NS_WARNING("Could not get minimal latency from cubeb."); - } - } + uint32_t latency_frames = CubebUtils::GetCubebMSGLatencyInFrames(&output); // Macbook and MacBook air don't have enough CPU to run very low latency // MediaStreamGraphs, cap the minimal latency to 512 frames int this case. From 26b0a8633138d80b19699ca638a8c50369b16c03 Mon Sep 17 00:00:00 2001 From: Alex Chronopoulos Date: Tue, 20 Feb 2018 15:37:08 +0200 Subject: [PATCH 18/48] Bug 1410456 - remove an unused variable that produces a compile warning. r=padenot MozReview-Commit-ID: Hf7Od37bnzX --HG-- extra : rebase_source : d53cbf4ef6f79ee9caac4e74a67f67a2e441669f --- media/libcubeb/src/cubeb_opensl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/media/libcubeb/src/cubeb_opensl.c b/media/libcubeb/src/cubeb_opensl.c index 8e9d4c73f2d6..98bf0e6178f8 100644 --- a/media/libcubeb/src/cubeb_opensl.c +++ b/media/libcubeb/src/cubeb_opensl.c @@ -236,7 +236,6 @@ static void play_callback(SLPlayItf caller, void * user_ptr, SLuint32 event) { cubeb_stream * stm = user_ptr; - int draining; assert(stm); switch (event) { case SL_PLAYEVENT_HEADATMARKER: From 3c5238ce8cb1dcae5811586c4a83c570f49505ed Mon Sep 17 00:00:00 2001 From: Alex Chronopoulos Date: Tue, 20 Feb 2018 15:37:09 +0200 Subject: [PATCH 19/48] Bug 1410456 - remove get min latency implementation because makes use of dlopen. r=padenot MozReview-Commit-ID: CD2FCiMobWm --HG-- extra : rebase_source : 17de7ffc8b93082473beaebabd8354c853f4f20f --- media/libcubeb/src/cubeb_opensl.c | 78 ++----------------------------- 1 file changed, 4 insertions(+), 74 deletions(-) diff --git a/media/libcubeb/src/cubeb_opensl.c b/media/libcubeb/src/cubeb_opensl.c index 98bf0e6178f8..4154b0d87818 100644 --- a/media/libcubeb/src/cubeb_opensl.c +++ b/media/libcubeb/src/cubeb_opensl.c @@ -61,6 +61,7 @@ #endif #define DEFAULT_SAMPLE_RATE 48000 +#define DEFAULT_NUM_OF_FRAMES 480 static struct cubeb_ops const opensl_ops; @@ -843,64 +844,6 @@ opensl_get_preferred_sample_rate(cubeb * ctx, uint32_t * rate) return CUBEB_OK; } -static int -opensl_get_min_latency(cubeb * ctx, cubeb_stream_params params, uint32_t * latency_frames) -{ - /* https://android.googlesource.com/platform/ndk.git/+/master/docs/opensles/index.html - * We don't want to deal with JNI here (and we don't have Java on b2g anyways), - * so we just dlopen the library and get the two symbols we need. */ - - int r; - void * libmedia; - size_t (*get_primary_output_frame_count)(void); - int (*get_output_frame_count)(size_t * frameCount, int streamType); - uint32_t primary_sampling_rate; - size_t primary_buffer_size; - - r = opensl_get_preferred_sample_rate(ctx, &primary_sampling_rate); - - if (r) { - return CUBEB_ERROR; - } - - libmedia = dlopen("libmedia.so", RTLD_LAZY); - if (!libmedia) { - return CUBEB_ERROR; - } - - /* JB variant */ - /* size_t AudioSystem::getPrimaryOutputFrameCount(void) */ - get_primary_output_frame_count = - dlsym(libmedia, "_ZN7android11AudioSystem26getPrimaryOutputFrameCountEv"); - if (!get_primary_output_frame_count) { - /* ICS variant */ - /* status_t AudioSystem::getOutputFrameCount(int* frameCount, int streamType) */ - get_output_frame_count = - dlsym(libmedia, "_ZN7android11AudioSystem19getOutputFrameCountEPii"); - if (!get_output_frame_count) { - dlclose(libmedia); - return CUBEB_ERROR; - } - } - - if (get_primary_output_frame_count) { - primary_buffer_size = get_primary_output_frame_count(); - } else { - if (get_output_frame_count(&primary_buffer_size, AUDIO_STREAM_TYPE_MUSIC) != 0) { - return CUBEB_ERROR; - } - } - - /* To get a fast track in Android's mixer, we need to be at the native - * samplerate, which is device dependant. Some devices might be able to - * resample when playing a fast track, but it's pretty rare. */ - *latency_frames = primary_buffer_size; - - dlclose(libmedia); - - return CUBEB_OK; -} - static void opensl_destroy(cubeb * ctx) { @@ -1150,20 +1093,7 @@ opensl_configure_playback(cubeb_stream * stm, cubeb_stream_params * params) { #endif assert(NELEMS(ids) == NELEMS(req)); - unsigned int latency_frames = stm->latency_frames; uint32_t preferred_sampling_rate = stm->inputrate; -#if defined(__ANDROID__) - if (get_android_version() >= ANDROID_VERSION_MARSHMALLOW) { - // Reset preferred samping rate to trigger fallback to native sampling rate. - preferred_sampling_rate = 0; - if (opensl_get_min_latency(stm->context, *params, &latency_frames) != CUBEB_OK) { - // Default to AudioFlinger's advertised fast track latency of 10ms. - latency_frames = 440; - } - stm->latency_frames = latency_frames; - } -#endif - SLresult res = SL_RESULT_CONTENT_UNSUPPORTED; if (preferred_sampling_rate) { res = (*stm->context->eng)->CreateAudioPlayer(stm->context->eng, @@ -1199,7 +1129,7 @@ opensl_configure_playback(cubeb_stream * stm, cubeb_stream_params * params) { stm->output_configured_rate = preferred_sampling_rate; stm->bytespersec = stm->output_configured_rate * stm->framesize; - stm->queuebuf_len = stm->framesize * latency_frames; + stm->queuebuf_len = stm->framesize * stm->latency_frames; // Calculate the capacity of input array stm->queuebuf_capacity = NBUFS; @@ -1336,7 +1266,7 @@ opensl_stream_init(cubeb * ctx, cubeb_stream ** stream, char const * stream_name stm->data_callback = data_callback; stm->state_callback = state_callback; stm->user_ptr = user_ptr; - stm->latency_frames = latency_frames; + stm->latency_frames = latency_frames ? latency_frames : DEFAULT_NUM_OF_FRAMES; stm->input_enabled = (input_stream_params) ? 1 : 0; stm->output_enabled = (output_stream_params) ? 1 : 0; stm->shutdown = 1; @@ -1704,7 +1634,7 @@ static struct cubeb_ops const opensl_ops = { .init = opensl_init, .get_backend_id = opensl_get_backend_id, .get_max_channel_count = opensl_get_max_channel_count, - .get_min_latency = opensl_get_min_latency, + .get_min_latency = NULL, .get_preferred_sample_rate = opensl_get_preferred_sample_rate, .get_preferred_channel_layout = NULL, .enumerate_devices = NULL, From b3ae975aa1d09ea95d5a6e264f3c0cf814488098 Mon Sep 17 00:00:00 2001 From: Alex Chronopoulos Date: Tue, 20 Feb 2018 15:37:10 +0200 Subject: [PATCH 20/48] Bug 1410456 - remove get latency implementation because makes use of dlopen. r=padenot MozReview-Commit-ID: 1Mc2dSk0hlc --HG-- extra : rebase_source : 00c83fb95b4ef5f184bc4c9b7511f53995ad82c4 --- media/libcubeb/src/cubeb_opensl.c | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/media/libcubeb/src/cubeb_opensl.c b/media/libcubeb/src/cubeb_opensl.c index 4154b0d87818..bbf8db7235f4 100644 --- a/media/libcubeb/src/cubeb_opensl.c +++ b/media/libcubeb/src/cubeb_opensl.c @@ -1581,24 +1581,6 @@ opensl_stream_get_position(cubeb_stream * stm, uint64_t * position) return CUBEB_OK; } -int -opensl_stream_get_latency(cubeb_stream * stm, uint32_t * latency) -{ - int r; - uint32_t mixer_latency; // The latency returned by AudioFlinger is in ms. - - /* audio_stream_type_t is an int, so this is okay. */ - r = stm->context->get_output_latency(&mixer_latency, AUDIO_STREAM_TYPE_MUSIC); - if (r) { - return CUBEB_ERROR; - } - - *latency = stm->latency_frames + // OpenSL latency - mixer_latency * stm->inputrate / 1000; // AudioFlinger latency - - return CUBEB_OK; -} - int opensl_stream_set_volume(cubeb_stream * stm, float volume) { @@ -1646,7 +1628,7 @@ static struct cubeb_ops const opensl_ops = { .stream_stop = opensl_stream_stop, .stream_reset_default_device = NULL, .stream_get_position = opensl_stream_get_position, - .stream_get_latency = opensl_stream_get_latency, + .stream_get_latency = NULL, .stream_set_volume = opensl_stream_set_volume, .stream_set_panning = NULL, .stream_get_current_device = NULL, From 2b7cbda791fc9b3a9811fecd43108388238dcb31 Mon Sep 17 00:00:00 2001 From: Alex Chronopoulos Date: Tue, 20 Feb 2018 15:37:11 +0200 Subject: [PATCH 21/48] Bug 1410456 - remove preferred sample rate implementation because makes use of dlopen. r=padenot MozReview-Commit-ID: jBZ3oewbjh --HG-- extra : rebase_source : e983510c4c3e2e046302ea3c0732b25a0cfd2cb1 --- media/libcubeb/src/cubeb_opensl.c | 91 ++++--------------------------- 1 file changed, 12 insertions(+), 79 deletions(-) diff --git a/media/libcubeb/src/cubeb_opensl.c b/media/libcubeb/src/cubeb_opensl.c index bbf8db7235f4..f7ef87672b1d 100644 --- a/media/libcubeb/src/cubeb_opensl.c +++ b/media/libcubeb/src/cubeb_opensl.c @@ -154,7 +154,7 @@ struct cubeb_stream { cubeb_state_callback state_callback; cubeb_resampler * resampler; - unsigned int inputrate; + unsigned int user_output_rate; unsigned int output_configured_rate; unsigned int latency_frames; int64_t lastPosition; @@ -784,66 +784,6 @@ opensl_get_max_channel_count(cubeb * ctx, uint32_t * max_channels) return CUBEB_OK; } -static int -opensl_get_preferred_sample_rate(cubeb * ctx, uint32_t * rate) -{ - /* https://android.googlesource.com/platform/ndk.git/+/master/docs/opensles/index.html - * We don't want to deal with JNI here (and we don't have Java on b2g anyways), - * so we just dlopen the library and get the two symbols we need. */ - int r; - void * libmedia; - uint32_t (*get_primary_output_samplingrate)(); - uint32_t (*get_output_samplingrate)(int * samplingRate, int streamType); - - libmedia = dlopen("libmedia.so", RTLD_LAZY); - if (!libmedia) { - return CUBEB_ERROR; - } - - /* uint32_t AudioSystem::getPrimaryOutputSamplingRate(void) */ - get_primary_output_samplingrate = - dlsym(libmedia, "_ZN7android11AudioSystem28getPrimaryOutputSamplingRateEv"); - if (!get_primary_output_samplingrate) { - /* fallback to - * status_t AudioSystem::getOutputSamplingRate(int* samplingRate, int streamType) - * if we cannot find getPrimaryOutputSamplingRate. */ - get_output_samplingrate = - dlsym(libmedia, "_ZN7android11AudioSystem21getOutputSamplingRateEPj19audio_stream_type_t"); - if (!get_output_samplingrate) { - /* Another signature exists, with a int instead of an audio_stream_type_t */ - get_output_samplingrate = - dlsym(libmedia, "_ZN7android11AudioSystem21getOutputSamplingRateEPii"); - if (!get_output_samplingrate) { - dlclose(libmedia); - return CUBEB_ERROR; - } - } - } - - if (get_primary_output_samplingrate) { - *rate = get_primary_output_samplingrate(); - } else { - /* We don't really know about the type, here, so we just pass music. */ - r = get_output_samplingrate((int *) rate, AUDIO_STREAM_TYPE_MUSIC); - if (r) { - dlclose(libmedia); - return CUBEB_ERROR; - } - } - - dlclose(libmedia); - - /* Depending on which method we called above, we can get a zero back, yet have - * a non-error return value, especially if the audio system is not - * ready/shutting down (i.e. when we can't get our hand on the AudioFlinger - * thread). */ - if (*rate == 0) { - return CUBEB_ERROR; - } - - return CUBEB_OK; -} - static void opensl_destroy(cubeb * ctx) { @@ -939,13 +879,9 @@ opensl_configure_capture(cubeb_stream * stm, cubeb_stream_params * params) // api for input device this is a safe choice. stm->input_device_rate = stm->output_configured_rate; } else { - // The output preferred rate is used for input only scenario. This is - // the correct rate to use to get a fast track for input only. - r = opensl_get_preferred_sample_rate(stm->context, &stm->input_device_rate); - if (r != CUBEB_OK) { - // If everything else fail use a safe choice for Android. - stm->input_device_rate = DEFAULT_SAMPLE_RATE; - } + // The output preferred rate is used for input only scenario. The + // default rate expected to supported from all android devices. + stm->input_device_rate = DEFAULT_SAMPLE_RATE; } lDataFormat.samplesPerSec = stm->input_device_rate * 1000; res = (*stm->context->eng)->CreateAudioRecorder(stm->context->eng, @@ -1056,7 +992,7 @@ opensl_configure_playback(cubeb_stream * stm, cubeb_stream_params * params) { assert(stm); assert(params); - stm->inputrate = params->rate; + stm->user_output_rate = params->rate; stm->framesize = params->channels * sizeof(int16_t); stm->lastPosition = -1; stm->lastPositionTimeStamp = 0; @@ -1093,7 +1029,7 @@ opensl_configure_playback(cubeb_stream * stm, cubeb_stream_params * params) { #endif assert(NELEMS(ids) == NELEMS(req)); - uint32_t preferred_sampling_rate = stm->inputrate; + uint32_t preferred_sampling_rate = stm->user_output_rate; SLresult res = SL_RESULT_CONTENT_UNSUPPORTED; if (preferred_sampling_rate) { res = (*stm->context->eng)->CreateAudioPlayer(stm->context->eng, @@ -1106,12 +1042,9 @@ opensl_configure_playback(cubeb_stream * stm, cubeb_stream_params * params) { } // Sample rate not supported? Try again with primary sample rate! - if (res == SL_RESULT_CONTENT_UNSUPPORTED) { - if (opensl_get_preferred_sample_rate(stm->context, &preferred_sampling_rate)) { - // If fail default is used - preferred_sampling_rate = DEFAULT_SAMPLE_RATE; - } - + if (res == SL_RESULT_CONTENT_UNSUPPORTED && + preferred_sampling_rate != DEFAULT_SAMPLE_RATE) { + preferred_sampling_rate = DEFAULT_SAMPLE_RATE; format.samplesPerSec = preferred_sampling_rate * 1000; res = (*stm->context->eng)->CreateAudioPlayer(stm->context->eng, &stm->playerObj, @@ -1550,7 +1483,7 @@ opensl_stream_get_position(cubeb_stream * stm, uint64_t * position) stm->lastPosition = msec; } - samplerate = stm->inputrate; + samplerate = stm->user_output_rate; r = stm->context->get_output_latency(&mixer_latency, AUDIO_STREAM_TYPE_MUSIC); if (r) { @@ -1558,7 +1491,7 @@ opensl_stream_get_position(cubeb_stream * stm, uint64_t * position) } pthread_mutex_lock(&stm->mutex); - int64_t maximum_position = stm->written * (int64_t)stm->inputrate / stm->output_configured_rate; + int64_t maximum_position = stm->written * (int64_t)stm->user_output_rate / stm->output_configured_rate; pthread_mutex_unlock(&stm->mutex); assert(maximum_position >= 0); @@ -1617,7 +1550,7 @@ static struct cubeb_ops const opensl_ops = { .get_backend_id = opensl_get_backend_id, .get_max_channel_count = opensl_get_max_channel_count, .get_min_latency = NULL, - .get_preferred_sample_rate = opensl_get_preferred_sample_rate, + .get_preferred_sample_rate = NULL, .get_preferred_channel_layout = NULL, .enumerate_devices = NULL, .device_collection_destroy = NULL, From 967d8be756ece5ff90404478e04655a0b655b474 Mon Sep 17 00:00:00 2001 From: Alex Chronopoulos Date: Tue, 20 Feb 2018 15:37:12 +0200 Subject: [PATCH 22/48] Bug 1410456 - get mixer latency from JNI instead of system library. r=jchen,padenot,snorp MozReview-Commit-ID: 3YciuVu25JO --HG-- extra : rebase_source : e5b3ad2f4b38f4b6454a56434022fce7f2a649cb --- media/libcubeb/src/cubeb-jni-instances.h | 34 +++++++++ media/libcubeb/src/cubeb-jni.cpp | 88 ++++++++++++++++++++++++ media/libcubeb/src/cubeb-jni.h | 10 +++ media/libcubeb/src/cubeb_opensl.c | 57 ++++++--------- media/libcubeb/src/moz.build | 1 + 5 files changed, 153 insertions(+), 37 deletions(-) create mode 100644 media/libcubeb/src/cubeb-jni-instances.h create mode 100644 media/libcubeb/src/cubeb-jni.cpp create mode 100644 media/libcubeb/src/cubeb-jni.h diff --git a/media/libcubeb/src/cubeb-jni-instances.h b/media/libcubeb/src/cubeb-jni-instances.h new file mode 100644 index 000000000000..dad62e4d3357 --- /dev/null +++ b/media/libcubeb/src/cubeb-jni-instances.h @@ -0,0 +1,34 @@ +#ifndef _CUBEB_JNI_INSTANCES_H_ +#define _CUBEB_JNI_INSTANCES_H_ + +#include "GeneratedJNIWrappers.h" +#include "mozilla/jni/Utils.h" + +/* + * The methods in this file offer a way to pass in the required + * JNI instances in the cubeb library. By default they return NULL. + * In this case part of the cubeb API that depends on JNI + * will return CUBEB_ERROR_NOT_SUPPORTED. Currently only one + * method depends on that: + * + * cubeb_stream_get_position() + * + * Users that want to use that cubeb API method must "override" + * the methods bellow to return a valid instance of JavaVM + * and application's Context object. + * */ + +JavaVM * +cubeb_jni_get_java_vm() +{ + return mozilla::jni::GetVM(); +} + +jobject +cubeb_jni_get_context_instance() +{ + auto context = mozilla::java::GeckoAppShell::GetApplicationContext(); + return context.Forget(); +} + +#endif //_CUBEB_JNI_INSTANCES_H_ diff --git a/media/libcubeb/src/cubeb-jni.cpp b/media/libcubeb/src/cubeb-jni.cpp new file mode 100644 index 000000000000..3eba97d74dcc --- /dev/null +++ b/media/libcubeb/src/cubeb-jni.cpp @@ -0,0 +1,88 @@ +#include "jni.h" +#include +#include "cubeb-jni-instances.h" + +#define AUDIO_STREAM_TYPE_MUSIC 3 + +JNIEnv * +cubeb_jni_get_env_for_thread(JavaVM * java_vm) +{ + JNIEnv * env = nullptr; + if (!java_vm->AttachCurrentThread(&env, nullptr)) { + assert(env); + return env; + } + + assert(false && "Failed to get JNIEnv for thread"); + return nullptr; // unreachable +} + +struct cubeb_jni { + JavaVM * s_java_vm = nullptr; + jobject s_audio_manager_obj = nullptr; + jclass s_audio_manager_class = nullptr; + jmethodID s_get_output_latency_id = nullptr; +}; + +extern "C" +cubeb_jni * +cubeb_jni_init() +{ + JavaVM * javaVM = cubeb_jni_get_java_vm(); + jobject ctx_obj = cubeb_jni_get_context_instance(); + + if (!javaVM || !ctx_obj) { + return nullptr; + } + + JNIEnv * jni_env = cubeb_jni_get_env_for_thread(javaVM); + assert(jni_env); + + cubeb_jni * cubeb_jni_ptr = new cubeb_jni; + assert(cubeb_jni_ptr); + + cubeb_jni_ptr->s_java_vm = javaVM; + + // Find the audio manager object and make it global to call it from another method + jclass context_class = jni_env->FindClass("android/content/Context"); + jfieldID audio_service_field = jni_env->GetStaticFieldID(context_class, "AUDIO_SERVICE", "Ljava/lang/String;"); + jstring jstr = (jstring)jni_env->GetStaticObjectField(context_class, audio_service_field); + jmethodID get_system_service_id = jni_env->GetMethodID(context_class, "getSystemService", "(Ljava/lang/String;)Ljava/lang/Object;"); + jobject audio_manager_obj = jni_env->CallObjectMethod(ctx_obj, get_system_service_id, jstr); + cubeb_jni_ptr->s_audio_manager_obj = reinterpret_cast(jni_env->NewGlobalRef(audio_manager_obj)); + + // Make the audio manager class a global reference in order to preserve method id + jclass audio_manager_class = jni_env->FindClass("android/media/AudioManager"); + cubeb_jni_ptr->s_audio_manager_class = reinterpret_cast(jni_env->NewGlobalRef(audio_manager_class)); + cubeb_jni_ptr->s_get_output_latency_id = jni_env->GetMethodID (audio_manager_class, "getOutputLatency", "(I)I"); + + jni_env->DeleteLocalRef(ctx_obj); + jni_env->DeleteLocalRef(context_class); + jni_env->DeleteLocalRef(jstr); + jni_env->DeleteLocalRef(audio_manager_obj); + jni_env->DeleteLocalRef(audio_manager_class); + + return cubeb_jni_ptr; +} + +extern "C" +int cubeb_get_output_latency_from_jni(cubeb_jni * cubeb_jni_ptr) +{ + assert(cubeb_jni_ptr); + JNIEnv * jni_env = cubeb_jni_get_env_for_thread(cubeb_jni_ptr->s_java_vm); + return jni_env->CallIntMethod(cubeb_jni_ptr->s_audio_manager_obj, cubeb_jni_ptr->s_get_output_latency_id, AUDIO_STREAM_TYPE_MUSIC); //param: AudioManager.STREAM_MUSIC +} + +extern "C" +void cubeb_jni_destroy(cubeb_jni * cubeb_jni_ptr) +{ + assert(cubeb_jni_ptr); + + JNIEnv * jni_env = cubeb_jni_get_env_for_thread(cubeb_jni_ptr->s_java_vm); + assert(jni_env); + + jni_env->DeleteGlobalRef(cubeb_jni_ptr->s_audio_manager_obj); + jni_env->DeleteGlobalRef(cubeb_jni_ptr->s_audio_manager_class); + + delete cubeb_jni_ptr; +} diff --git a/media/libcubeb/src/cubeb-jni.h b/media/libcubeb/src/cubeb-jni.h new file mode 100644 index 000000000000..8c7ddb6acf4d --- /dev/null +++ b/media/libcubeb/src/cubeb-jni.h @@ -0,0 +1,10 @@ +#ifndef _CUBEB_JNI_H_ +#define _CUBEB_JNI_H_ + +typedef struct cubeb_jni cubeb_jni; + +cubeb_jni * cubeb_jni_init(); +int cubeb_get_output_latency_from_jni(cubeb_jni * cubeb_jni_ptr); +void cubeb_jni_destroy(cubeb_jni * cubeb_jni_ptr); + +#endif // _CUBEB_JNI_H_ diff --git a/media/libcubeb/src/cubeb_opensl.c b/media/libcubeb/src/cubeb_opensl.c index f7ef87672b1d..2963fcabd61e 100644 --- a/media/libcubeb/src/cubeb_opensl.c +++ b/media/libcubeb/src/cubeb_opensl.c @@ -26,6 +26,7 @@ #include "cubeb_resampler.h" #include "cubeb-sles.h" #include "cubeb_array_queue.h" +#include "cubeb-jni.h" #if defined(__ANDROID__) #ifdef LOG @@ -68,8 +69,6 @@ static struct cubeb_ops const opensl_ops; struct cubeb { struct cubeb_ops const * ops; void * lib; - void * libmedia; - int32_t (* get_output_latency)(uint32_t * latency, int stream_type); SLInterfaceID SL_IID_BUFFERQUEUE; SLInterfaceID SL_IID_PLAY; #if defined(__ANDROID__) @@ -81,11 +80,11 @@ struct cubeb { SLObjectItf engObj; SLEngineItf eng; SLObjectItf outmixObj; + cubeb_jni * jni_obj; }; #define NELEMS(A) (sizeof(A) / sizeof A[0]) #define NBUFS 4 -#define AUDIO_STREAM_TYPE_MUSIC 3 struct cubeb_stream { /* Note: Must match cubeb_stream layout in cubeb.c. */ @@ -668,30 +667,11 @@ opensl_init(cubeb ** context, char const * context_name) ctx->ops = &opensl_ops; ctx->lib = dlopen("libOpenSLES.so", RTLD_LAZY); - ctx->libmedia = dlopen("libmedia.so", RTLD_LAZY); - if (!ctx->lib || !ctx->libmedia) { + if (!ctx->lib) { free(ctx); return CUBEB_ERROR; } - /* Get the latency, in ms, from AudioFlinger */ - /* status_t AudioSystem::getOutputLatency(uint32_t* latency, - * audio_stream_type_t streamType) */ - /* First, try the most recent signature. */ - ctx->get_output_latency = - dlsym(ctx->libmedia, "_ZN7android11AudioSystem16getOutputLatencyEPj19audio_stream_type_t"); - if (!ctx->get_output_latency) { - /* in case of failure, try the legacy version. */ - /* status_t AudioSystem::getOutputLatency(uint32_t* latency, - * int streamType) */ - ctx->get_output_latency = - dlsym(ctx->libmedia, "_ZN7android11AudioSystem16getOutputLatencyEPji"); - if (!ctx->get_output_latency) { - opensl_destroy(ctx); - return CUBEB_ERROR; - } - } - typedef SLresult (*slCreateEngine_t)(SLObjectItf *, SLuint32, const SLEngineOption *, @@ -761,6 +741,11 @@ opensl_init(cubeb ** context, char const * context_name) return CUBEB_ERROR; } + ctx->jni_obj = cubeb_jni_init(); + if (!ctx->jni_obj) { + LOG("Warning: jni is not initialized, cubeb_stream_get_position() is not supported"); + } + *context = ctx; LOG("Cubeb init (%p) success", ctx); @@ -792,7 +777,8 @@ opensl_destroy(cubeb * ctx) if (ctx->engObj) cubeb_destroy_sles_engine(&ctx->engObj); dlclose(ctx->lib); - dlclose(ctx->libmedia); + if (ctx->jni_obj) + cubeb_jni_destroy(ctx->jni_obj); free(ctx); } @@ -879,8 +865,8 @@ opensl_configure_capture(cubeb_stream * stm, cubeb_stream_params * params) // api for input device this is a safe choice. stm->input_device_rate = stm->output_configured_rate; } else { - // The output preferred rate is used for input only scenario. The - // default rate expected to supported from all android devices. + // The output preferred rate is used for an input only scenario. + // The default rate expected to be supported from all android devices. stm->input_device_rate = DEFAULT_SAMPLE_RATE; } lDataFormat.samplesPerSec = stm->input_device_rate * 1000; @@ -1043,7 +1029,7 @@ opensl_configure_playback(cubeb_stream * stm, cubeb_stream_params * params) { // Sample rate not supported? Try again with primary sample rate! if (res == SL_RESULT_CONTENT_UNSUPPORTED && - preferred_sampling_rate != DEFAULT_SAMPLE_RATE) { + preferred_sampling_rate != DEFAULT_SAMPLE_RATE) { preferred_sampling_rate = DEFAULT_SAMPLE_RATE; format.samplesPerSec = preferred_sampling_rate * 1000; res = (*stm->context->eng)->CreateAudioPlayer(stm->context->eng, @@ -1463,11 +1449,12 @@ static int opensl_stream_get_position(cubeb_stream * stm, uint64_t * position) { SLmillisecond msec; - uint64_t samplerate; - SLresult res; - int r; - uint32_t mixer_latency; uint32_t compensation_msec = 0; + SLresult res; + + if (!stm->context->jni_obj) { + return CUBEB_ERROR_NOT_SUPPORTED; + } res = (*stm->play)->GetPosition(stm->play, &msec); if (res != SL_RESULT_SUCCESS) @@ -1483,12 +1470,8 @@ opensl_stream_get_position(cubeb_stream * stm, uint64_t * position) stm->lastPosition = msec; } - samplerate = stm->user_output_rate; - - r = stm->context->get_output_latency(&mixer_latency, AUDIO_STREAM_TYPE_MUSIC); - if (r) { - return CUBEB_ERROR; - } + uint64_t samplerate = stm->user_output_rate; + uint32_t mixer_latency = cubeb_get_output_latency_from_jni(stm->context->jni_obj); pthread_mutex_lock(&stm->mutex); int64_t maximum_position = stm->written * (int64_t)stm->user_output_rate / stm->output_configured_rate; diff --git a/media/libcubeb/src/moz.build b/media/libcubeb/src/moz.build index 6484e61fa915..15071f147ab8 100644 --- a/media/libcubeb/src/moz.build +++ b/media/libcubeb/src/moz.build @@ -77,6 +77,7 @@ if CONFIG['OS_TARGET'] == 'WINNT': if CONFIG['OS_TARGET'] == 'Android': SOURCES += ['cubeb_opensl.c'] SOURCES += ['cubeb_resampler.cpp'] + SOURCES += ['cubeb-jni.cpp'] DEFINES['USE_OPENSL'] = True SOURCES += [ 'cubeb_audiotrack.c', From 0d15e892588e6c1ee90719fc545b31d8575d4c99 Mon Sep 17 00:00:00 2001 From: Alex Chronopoulos Date: Mon, 26 Feb 2018 13:46:13 +0200 Subject: [PATCH 23/48] Bug 1410456 - Keep the old mechanism for the older versions that jni method is not available. r=padenot MozReview-Commit-ID: IsAOhDkNjHp --HG-- extra : rebase_source : 375b7db8bc5d0c433aa55f229d8f81c971a6fbde --- .../src/android/cubeb-output-latency.h | 75 +++++++++++++++++++ .../src/android/cubeb_media_library.h | 62 +++++++++++++++ media/libcubeb/src/cubeb_opensl.c | 18 ++--- 3 files changed, 146 insertions(+), 9 deletions(-) create mode 100644 media/libcubeb/src/android/cubeb-output-latency.h create mode 100644 media/libcubeb/src/android/cubeb_media_library.h diff --git a/media/libcubeb/src/android/cubeb-output-latency.h b/media/libcubeb/src/android/cubeb-output-latency.h new file mode 100644 index 000000000000..e8eb8ae23ea8 --- /dev/null +++ b/media/libcubeb/src/android/cubeb-output-latency.h @@ -0,0 +1,75 @@ +#ifndef _CUBEB_OUTPUT_LATENCY_H_ +#define _CUBEB_OUTPUT_LATENCY_H_ + +#include "cubeb_media_library.h" +#include "../cubeb-jni.h" + +struct output_latency_function { + media_lib * from_lib; + cubeb_jni * from_jni; + int version; +}; + +typedef struct output_latency_function output_latency_function; + +const int ANDROID_JELLY_BEAN_MR1_4_2 = 17; + +output_latency_function * +cubeb_output_latency_load_method(int version) +{ + output_latency_function * ol = NULL; + ol = calloc(1, sizeof(output_latency_function)); + + ol->version = version; + + if (ol->version > ANDROID_JELLY_BEAN_MR1_4_2){ + ol->from_jni = cubeb_jni_init(); + return ol; + } + + ol->from_lib = cubeb_load_media_library(); + return ol; +} + +bool +cubeb_output_latency_method_is_loaded(output_latency_function * ol) +{ + assert(ol && (ol->from_jni || ol->from_lib)); + if (ol->version > ANDROID_JELLY_BEAN_MR1_4_2){ + return !!ol->from_jni; + } + + return !!ol->from_lib; +} + +void +cubeb_output_latency_unload_method(output_latency_function * ol) +{ + if (!ol) { + return; + } + + if (ol->version > ANDROID_JELLY_BEAN_MR1_4_2 && ol->from_jni) { + cubeb_jni_destroy(ol->from_jni); + } + + if (ol->version <= ANDROID_JELLY_BEAN_MR1_4_2 && ol->from_lib) { + cubeb_close_media_library(ol->from_lib); + } + + free(ol); +} + +uint32_t +cubeb_get_output_latency(output_latency_function * ol) +{ + assert(cubeb_output_latency_method_is_loaded(ol)); + + if (ol->version > ANDROID_JELLY_BEAN_MR1_4_2){ + return cubeb_get_output_latency_from_jni(ol->from_jni); + } + + return cubeb_get_output_latency_from_media_library(ol->from_lib); +} + +#endif // _CUBEB_OUTPUT_LATENCY_H_ diff --git a/media/libcubeb/src/android/cubeb_media_library.h b/media/libcubeb/src/android/cubeb_media_library.h new file mode 100644 index 000000000000..ab21b779dfad --- /dev/null +++ b/media/libcubeb/src/android/cubeb_media_library.h @@ -0,0 +1,62 @@ +#ifndef _CUBEB_MEDIA_LIBRARY_H_ +#define _CUBEB_MEDIA_LIBRARY_H_ + +struct media_lib { + void * libmedia; + int32_t (* get_output_latency)(uint32_t * latency, int stream_type); +}; + +typedef struct media_lib media_lib; + +media_lib * +cubeb_load_media_library() +{ + media_lib ml = {0}; + ml.libmedia = dlopen("libmedia.so", RTLD_LAZY); + if (!ml.libmedia) { + return NULL; + } + + // Get the latency, in ms, from AudioFlinger. First, try the most recent signature. + // status_t AudioSystem::getOutputLatency(uint32_t* latency, audio_stream_type_t streamType) + ml.get_output_latency = + dlsym(ml.libmedia, "_ZN7android11AudioSystem16getOutputLatencyEPj19audio_stream_type_t"); + if (!ml.get_output_latency) { + // In case of failure, try the signature from legacy version. + // status_t AudioSystem::getOutputLatency(uint32_t* latency, int streamType) + ml.get_output_latency = + dlsym(ml.libmedia, "_ZN7android11AudioSystem16getOutputLatencyEPji"); + if (!ml.get_output_latency) { + return NULL; + } + } + + media_lib * rv = NULL; + rv = calloc(1, sizeof(media_lib)); + assert(rv); + *rv = ml; + return rv; +} + +void +cubeb_close_media_library(media_lib * ml) +{ + dlclose(ml->libmedia); + ml->libmedia = NULL; + ml->get_output_latency = NULL; + free(ml); +} + +uint32_t +cubeb_get_output_latency_from_media_library(media_lib * ml) +{ + uint32_t latency = 0; + const int audio_stream_type_music = 3; + int32_t r = ml->get_output_latency(&latency, audio_stream_type_music); + if (r) { + return 0; + } + return latency; +} + +#endif // _CUBEB_MEDIA_LIBRARY_H_ diff --git a/media/libcubeb/src/cubeb_opensl.c b/media/libcubeb/src/cubeb_opensl.c index 2963fcabd61e..a760fd8111ef 100644 --- a/media/libcubeb/src/cubeb_opensl.c +++ b/media/libcubeb/src/cubeb_opensl.c @@ -26,7 +26,7 @@ #include "cubeb_resampler.h" #include "cubeb-sles.h" #include "cubeb_array_queue.h" -#include "cubeb-jni.h" +#include "android/cubeb-output-latency.h" #if defined(__ANDROID__) #ifdef LOG @@ -80,7 +80,7 @@ struct cubeb { SLObjectItf engObj; SLEngineItf eng; SLObjectItf outmixObj; - cubeb_jni * jni_obj; + output_latency_function * p_output_latency_function; }; #define NELEMS(A) (sizeof(A) / sizeof A[0]) @@ -741,9 +741,9 @@ opensl_init(cubeb ** context, char const * context_name) return CUBEB_ERROR; } - ctx->jni_obj = cubeb_jni_init(); - if (!ctx->jni_obj) { - LOG("Warning: jni is not initialized, cubeb_stream_get_position() is not supported"); + ctx->p_output_latency_function = cubeb_output_latency_load_method(android_version); + if (!ctx->p_output_latency_function) { + LOG("Warning: output latency is not available, cubeb_stream_get_position() is not supported"); } *context = ctx; @@ -777,8 +777,8 @@ opensl_destroy(cubeb * ctx) if (ctx->engObj) cubeb_destroy_sles_engine(&ctx->engObj); dlclose(ctx->lib); - if (ctx->jni_obj) - cubeb_jni_destroy(ctx->jni_obj); + if (ctx->p_output_latency_function) + cubeb_output_latency_unload_method(ctx->p_output_latency_function); free(ctx); } @@ -1452,7 +1452,7 @@ opensl_stream_get_position(cubeb_stream * stm, uint64_t * position) uint32_t compensation_msec = 0; SLresult res; - if (!stm->context->jni_obj) { + if (!cubeb_output_latency_method_is_loaded(stm->context->p_output_latency_function)) { return CUBEB_ERROR_NOT_SUPPORTED; } @@ -1471,7 +1471,7 @@ opensl_stream_get_position(cubeb_stream * stm, uint64_t * position) } uint64_t samplerate = stm->user_output_rate; - uint32_t mixer_latency = cubeb_get_output_latency_from_jni(stm->context->jni_obj); + uint32_t mixer_latency = cubeb_get_output_latency(stm->context->p_output_latency_function); pthread_mutex_lock(&stm->mutex); int64_t maximum_position = stm->written * (int64_t)stm->user_output_rate / stm->output_configured_rate; From 2eb404212e630874dcd086c16cc669df9ec58633 Mon Sep 17 00:00:00 2001 From: Alex Chronopoulos Date: Mon, 26 Feb 2018 13:52:36 +0200 Subject: [PATCH 24/48] Bug 1410456 - Add new files in update.sh script. r=padenot MozReview-Commit-ID: GD8CHTM1pV8 --HG-- extra : rebase_source : 27a9e05db3831729dc0d6c7f43ba4edca2d39bdc --- media/libcubeb/update.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/media/libcubeb/update.sh b/media/libcubeb/update.sh index 827a0db37c50..0b33550a4923 100755 --- a/media/libcubeb/update.sh +++ b/media/libcubeb/update.sh @@ -20,6 +20,10 @@ cp $1/src/cubeb_log.h src cp $1/src/cubeb_mixer.cpp src cp $1/src/cubeb_mixer.h src cp $1/src/cubeb_opensl.c src +cp $1/src/cubeb-jni.cpp src +cp $1/src/cubeb-jni.h src +cp $1/src/android/cubeb-output-latency.h src/android +cp $1/src/android/cubeb_media_library.h src/android cp $1/src/cubeb_osx_run_loop.h src cp $1/src/cubeb_panner.cpp src cp $1/src/cubeb_panner.h src From 1697d603ea45ecad4d80f9b5501a6ea787f7db1c Mon Sep 17 00:00:00 2001 From: Alex Chronopoulos Date: Mon, 26 Feb 2018 14:03:36 +0200 Subject: [PATCH 25/48] Bug 1410456 - Update cubeb from upstream to b1ee1ce. r=padenot MozReview-Commit-ID: 5PmGUcxB6jv --HG-- extra : rebase_source : 15e4d33a4d80d5cfedb6b064d9320ae711db73f4 --- media/libcubeb/README_MOZILLA | 2 +- media/libcubeb/src/android/cubeb-output-latency.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/media/libcubeb/README_MOZILLA b/media/libcubeb/README_MOZILLA index e18180d14fb4..828f2cae534f 100644 --- a/media/libcubeb/README_MOZILLA +++ b/media/libcubeb/README_MOZILLA @@ -5,4 +5,4 @@ Makefile.in build files for the Mozilla build system. The cubeb git repository is: git://github.com/kinetiknz/cubeb.git -The git commit ID used was 1d53c3a3779cbeb860b16aa38cc7f51e196b9745 (2018-02-13 12:30:46 +1000) +The git commit ID used was b1ee1ce7fa7bfb9675e529663f230685125fb694 (2018-02-26 13:40:08 +0200) diff --git a/media/libcubeb/src/android/cubeb-output-latency.h b/media/libcubeb/src/android/cubeb-output-latency.h index e8eb8ae23ea8..a824fc1c2453 100644 --- a/media/libcubeb/src/android/cubeb-output-latency.h +++ b/media/libcubeb/src/android/cubeb-output-latency.h @@ -1,6 +1,7 @@ #ifndef _CUBEB_OUTPUT_LATENCY_H_ #define _CUBEB_OUTPUT_LATENCY_H_ +#include #include "cubeb_media_library.h" #include "../cubeb-jni.h" From add628d70cbfd637050871fc6105957e7b3b79e0 Mon Sep 17 00:00:00 2001 From: Kartikaya Gupta Date: Sun, 25 Feb 2018 01:04:48 -0500 Subject: [PATCH 26/48] Bug 1440968 - Use a different method for skipping talos tests on windows. r=jmaher Having both .*-qr/.* and windows.* as patterns in the run-on-projects test platform list means that a platform like windows10-64-qr matches both and the parser complains. Instead of doing this, we can remove the windows patterns and take the talos tests out of the windows-talos test-set which accomplishes the same thing. MozReview-Commit-ID: 9kMooiNiHTb --HG-- extra : rebase_source : e58d74c43531cb1f4b625b188be696d1e951eda0 --- taskcluster/ci/test/talos.yml | 2 -- taskcluster/ci/test/test-sets.yml | 2 -- 2 files changed, 4 deletions(-) diff --git a/taskcluster/ci/test/talos.yml b/taskcluster/ci/test/talos.yml index 6d678785832f..0631e5bde740 100644 --- a/taskcluster/ci/test/talos.yml +++ b/taskcluster/ci/test/talos.yml @@ -492,7 +492,6 @@ talos-tp6: run-on-projects: by-test-platform: .*-qr/.*: ['mozilla-central', 'try'] - windows.*: [] default: ['mozilla-beta', 'mozilla-central', 'mozilla-inbound', 'autoland', 'try'] max-run-time: 1200 mozharness: @@ -523,7 +522,6 @@ talos-tp6-stylo-threads: by-test-platform: .*-qr/.*: ['mozilla-central', 'try'] macosx.*: ['mozilla-beta', 'autoland', 'try'] - windows.*: [] default: ['mozilla-beta', 'mozilla-central', 'mozilla-inbound', 'autoland', 'try'] mozharness: extra-options: diff --git a/taskcluster/ci/test/test-sets.yml b/taskcluster/ci/test/test-sets.yml index 713c7f21d609..87b6a203be5a 100644 --- a/taskcluster/ci/test/test-sets.yml +++ b/taskcluster/ci/test/test-sets.yml @@ -208,8 +208,6 @@ windows-talos: - talos-svgr - talos-tp5o - talos-xperf - - talos-tp6 - - talos-tp6-stylo-threads - talos-speedometer - talos-motionmark - talos-h1 From e84ffff0722e7ad5cd8f2b6037f58c2a49260ea8 Mon Sep 17 00:00:00 2001 From: Kartikaya Gupta Date: Sun, 25 Feb 2018 01:05:49 -0500 Subject: [PATCH 27/48] Bug 1440968 - Turn on some of the talos tests for QuantumRender on windows. r=jmaher MozReview-Commit-ID: JGXW63ohn8W --HG-- extra : rebase_source : e67cd5793a2ddbba38f02705f2c59f07084aa4c7 --- taskcluster/ci/test/test-platforms.yml | 1 + taskcluster/ci/test/test-sets.yml | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/taskcluster/ci/test/test-platforms.yml b/taskcluster/ci/test/test-platforms.yml index bf91250b44ee..cb364625b359 100644 --- a/taskcluster/ci/test/test-platforms.yml +++ b/taskcluster/ci/test/test-platforms.yml @@ -243,6 +243,7 @@ windows10-64-qr/opt: build-platform: win64/opt test-sets: - windows-qr-tests + - windows-qr-talos ## diff --git a/taskcluster/ci/test/test-sets.yml b/taskcluster/ci/test/test-sets.yml index 87b6a203be5a..cf06e86243ca 100644 --- a/taskcluster/ci/test/test-sets.yml +++ b/taskcluster/ci/test/test-sets.yml @@ -159,6 +159,18 @@ windows-qr-tests: - mochitest-gpu - mochitest-media +windows-qr-talos: + - talos-chrome + - talos-dromaeojs + - talos-g1 + - talos-g4 + - talos-perf-reftest + - talos-perf-reftest-singletons + - talos-svgr + - talos-tp5o + - talos-speedometer + - talos-motionmark + jsdcov-code-coverage-tests: - mochitest - mochitest-browser-chrome From c597c8df87f65bb420714e0f1cf19bb628601c14 Mon Sep 17 00:00:00 2001 From: Andreas Pehrson Date: Mon, 26 Feb 2018 09:41:20 +0100 Subject: [PATCH 28/48] Bug 1440252 - Implement MediaEngineWebRTCMicrophoneSource::GetSettings. r=padenot MozReview-Commit-ID: IVbax9Xxs8R --HG-- extra : rebase_source : 6d42af31705860100ba40b0d750ddce2b513467d --- dom/media/webrtc/MediaEngineWebRTC.h | 6 ++++++ dom/media/webrtc/MediaEngineWebRTCAudio.cpp | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/dom/media/webrtc/MediaEngineWebRTC.h b/dom/media/webrtc/MediaEngineWebRTC.h index f8379515cdc6..ca29230e63bd 100644 --- a/dom/media/webrtc/MediaEngineWebRTC.h +++ b/dom/media/webrtc/MediaEngineWebRTC.h @@ -416,6 +416,12 @@ public: const nsString& aDeviceId, const char** aOutBadConstraint) override; + /** + * Assigns the current settings of the capture to aOutSettings. + * Main thread only. + */ + void GetSettings(dom::MediaTrackSettings& aOutSettings) const override; + void Pull(const RefPtr& aHandle, const RefPtr& aStream, TrackID aTrackID, diff --git a/dom/media/webrtc/MediaEngineWebRTCAudio.cpp b/dom/media/webrtc/MediaEngineWebRTCAudio.cpp index 2dd0f1a2de4c..68c2c9164570 100644 --- a/dom/media/webrtc/MediaEngineWebRTCAudio.cpp +++ b/dom/media/webrtc/MediaEngineWebRTCAudio.cpp @@ -754,6 +754,13 @@ MediaEngineWebRTCMicrophoneSource::Stop(const RefPtr& aH return NS_OK; } +void +MediaEngineWebRTCMicrophoneSource::GetSettings(dom::MediaTrackSettings& aOutSettings) const +{ + MOZ_ASSERT(NS_IsMainThread()); + aOutSettings = *mSettings; +} + void MediaEngineWebRTCMicrophoneSource::Pull(const RefPtr& aHandle, const RefPtr& aStream, From 5968b0947fe3b53ccd01c6ce3beffefef3ae59a2 Mon Sep 17 00:00:00 2001 From: Narcis Beleuzu Date: Mon, 26 Feb 2018 15:58:20 +0200 Subject: [PATCH 29/48] Backed out 10 changesets (bug 1410456) for Android mochitest crashes, e.g. in test_postMessages.html. CLOSED TREE Backed out changeset 7ec175446efd (bug 1410456) Backed out changeset 94457911bb24 (bug 1410456) Backed out changeset 5248a21216ae (bug 1410456) Backed out changeset f182ab7885db (bug 1410456) Backed out changeset e482347bdae3 (bug 1410456) Backed out changeset f7b646045e06 (bug 1410456) Backed out changeset 6a8ed4bf5d2f (bug 1410456) Backed out changeset 1a9c687ec277 (bug 1410456) Backed out changeset 82f6667c6758 (bug 1410456) Backed out changeset 7bf358e3e01b (bug 1410456) --- dom/media/CubebUtils.cpp | 50 +--- dom/media/CubebUtils.h | 7 +- dom/media/GraphDriver.cpp | 10 +- media/libcubeb/README_MOZILLA | 2 +- .../src/android/cubeb-output-latency.h | 76 ------ .../src/android/cubeb_media_library.h | 62 ----- media/libcubeb/src/cubeb-jni-instances.h | 34 --- media/libcubeb/src/cubeb-jni.cpp | 88 ------- media/libcubeb/src/cubeb-jni.h | 10 - media/libcubeb/src/cubeb_opensl.c | 239 +++++++++++++++--- media/libcubeb/src/moz.build | 1 - media/libcubeb/update.sh | 4 - .../java/org/mozilla/gecko/GeckoAppShell.java | 4 +- widget/android/GeneratedJNIWrappers.h | 4 +- 14 files changed, 228 insertions(+), 363 deletions(-) delete mode 100644 media/libcubeb/src/android/cubeb-output-latency.h delete mode 100644 media/libcubeb/src/android/cubeb_media_library.h delete mode 100644 media/libcubeb/src/cubeb-jni-instances.h delete mode 100644 media/libcubeb/src/cubeb-jni.cpp delete mode 100644 media/libcubeb/src/cubeb-jni.h diff --git a/dom/media/CubebUtils.cpp b/dom/media/CubebUtils.cpp index 4cda851197a7..9c47c2f2ea55 100644 --- a/dom/media/CubebUtils.cpp +++ b/dom/media/CubebUtils.cpp @@ -25,9 +25,6 @@ #include "prdtoa.h" #include #include -#ifdef MOZ_WIDGET_ANDROID -#include "GeneratedJNIWrappers.h" -#endif #define PREF_VOLUME_SCALE "media.volume_scale" #define PREF_CUBEB_BACKEND "media.cubeb.backend" @@ -122,8 +119,8 @@ cubeb* sCubebContext; double sVolumeScale = 1.0; uint32_t sCubebPlaybackLatencyInMilliseconds = 100; uint32_t sCubebMSGLatencyInFrames = 512; -bool sCubebPlaybackLatencyPrefSet = false; -bool sCubebMSGLatencyPrefSet = false; +bool sCubebPlaybackLatencyPrefSet; +bool sCubebMSGLatencyPrefSet; bool sAudioStreamInitEverSucceeded = false; #ifdef MOZ_CUBEB_REMOTING bool sCubebSandbox; @@ -308,15 +305,11 @@ bool InitPreferredSampleRate() if (!context) { return false; } -#ifdef MOZ_WIDGET_ANDROID - sPreferredSampleRate = AndroidGetAudioOutputSampleRate(); -#else if (cubeb_get_preferred_sample_rate(context, &sPreferredSampleRate) != CUBEB_OK) { return false; } -#endif MOZ_ASSERT(sPreferredSampleRate); return true; } @@ -534,28 +527,14 @@ bool CubebMSGLatencyPrefSet() return sCubebMSGLatencyPrefSet; } -uint32_t GetCubebMSGLatencyInFrames(cubeb_stream_params * params) +Maybe GetCubebMSGLatencyInFrames() { StaticMutexAutoLock lock(sMutex); - if (sCubebMSGLatencyPrefSet) { - MOZ_ASSERT(sCubebMSGLatencyInFrames > 0); - return sCubebMSGLatencyInFrames; + if (!sCubebMSGLatencyPrefSet) { + return Maybe(); } - -#ifdef MOZ_WIDGET_ANDROID - return AndroidGetAudioOutputFramesPerBuffer(); -#else - cubeb* context = GetCubebContextUnlocked(); - if (!context) { - return sCubebMSGLatencyInFrames; // default 512 - } - uint32_t latency_frames = 0; - if (cubeb_get_min_latency(context, params, &latency_frames) != CUBEB_OK) { - NS_WARNING("Could not get minimal latency from cubeb."); - return sCubebMSGLatencyInFrames; // default 512 - } - return latency_frames; -#endif + MOZ_ASSERT(sCubebMSGLatencyInFrames > 0); + return Some(sCubebMSGLatencyInFrames); } void InitLibrary() @@ -762,20 +741,5 @@ void GetDeviceCollection(nsTArray>& aDeviceInfos, } } -#ifdef MOZ_WIDGET_ANDROID -uint32_t AndroidGetAudioOutputSampleRate() -{ - int32_t sample_rate = java::GeckoAppShell::GetAudioOutputSampleRate(); - MOZ_ASSERT(sample_rate > 0); - return sample_rate; -} -uint32_t AndroidGetAudioOutputFramesPerBuffer() -{ - int32_t frames = java::GeckoAppShell::GetAudioOutputFramesPerBuffer(); - MOZ_ASSERT(frames > 0); - return frames; -} -#endif - } // namespace CubebUtils } // namespace mozilla diff --git a/dom/media/CubebUtils.h b/dom/media/CubebUtils.h index 79f52d9714e6..60e9eb1e477e 100644 --- a/dom/media/CubebUtils.h +++ b/dom/media/CubebUtils.h @@ -44,7 +44,7 @@ cubeb* GetCubebContext(); void ReportCubebStreamInitFailure(bool aIsFirstStream); void ReportCubebBackendUsed(); uint32_t GetCubebPlaybackLatencyInMilliseconds(); -uint32_t GetCubebMSGLatencyInFrames(cubeb_stream_params * params); +Maybe GetCubebMSGLatencyInFrames(); bool CubebLatencyPrefSet(); cubeb_channel_layout ConvertChannelMapToCubebLayout(uint32_t aChannelMap); void GetCurrentBackend(nsAString& aBackend); @@ -52,11 +52,6 @@ void GetPreferredChannelLayout(nsAString& aLayout); void GetDeviceCollection(nsTArray>& aDeviceInfos, Side aSide); cubeb_channel_layout GetPreferredChannelLayoutOrSMPTE(cubeb* context, uint32_t aChannels); - -#ifdef MOZ_WIDGET_ANDROID -uint32_t AndroidGetAudioOutputSampleRate(); -uint32_t AndroidGetAudioOutputFramesPerBuffer(); -#endif } // namespace CubebUtils } // namespace mozilla diff --git a/dom/media/GraphDriver.cpp b/dom/media/GraphDriver.cpp index 62d10cce42c6..f3d8c05d6be5 100644 --- a/dom/media/GraphDriver.cpp +++ b/dom/media/GraphDriver.cpp @@ -599,6 +599,7 @@ AudioCallbackDriver::Init() cubeb_stream_params output; cubeb_stream_params input; + uint32_t latency_frames; bool firstStream = CubebUtils::GetFirstStream(); MOZ_ASSERT(!NS_IsMainThread(), @@ -628,7 +629,14 @@ AudioCallbackDriver::Init() output.layout = CubebUtils::GetPreferredChannelLayoutOrSMPTE(cubebContext, mOutputChannels); output.prefs = CUBEB_STREAM_PREF_NONE; - uint32_t latency_frames = CubebUtils::GetCubebMSGLatencyInFrames(&output); + Maybe latencyPref = CubebUtils::GetCubebMSGLatencyInFrames(); + if (latencyPref) { + latency_frames = latencyPref.value(); + } else { + if (cubeb_get_min_latency(cubebContext, &output, &latency_frames) != CUBEB_OK) { + NS_WARNING("Could not get minimal latency from cubeb."); + } + } // Macbook and MacBook air don't have enough CPU to run very low latency // MediaStreamGraphs, cap the minimal latency to 512 frames int this case. diff --git a/media/libcubeb/README_MOZILLA b/media/libcubeb/README_MOZILLA index 828f2cae534f..e18180d14fb4 100644 --- a/media/libcubeb/README_MOZILLA +++ b/media/libcubeb/README_MOZILLA @@ -5,4 +5,4 @@ Makefile.in build files for the Mozilla build system. The cubeb git repository is: git://github.com/kinetiknz/cubeb.git -The git commit ID used was b1ee1ce7fa7bfb9675e529663f230685125fb694 (2018-02-26 13:40:08 +0200) +The git commit ID used was 1d53c3a3779cbeb860b16aa38cc7f51e196b9745 (2018-02-13 12:30:46 +1000) diff --git a/media/libcubeb/src/android/cubeb-output-latency.h b/media/libcubeb/src/android/cubeb-output-latency.h deleted file mode 100644 index a824fc1c2453..000000000000 --- a/media/libcubeb/src/android/cubeb-output-latency.h +++ /dev/null @@ -1,76 +0,0 @@ -#ifndef _CUBEB_OUTPUT_LATENCY_H_ -#define _CUBEB_OUTPUT_LATENCY_H_ - -#include -#include "cubeb_media_library.h" -#include "../cubeb-jni.h" - -struct output_latency_function { - media_lib * from_lib; - cubeb_jni * from_jni; - int version; -}; - -typedef struct output_latency_function output_latency_function; - -const int ANDROID_JELLY_BEAN_MR1_4_2 = 17; - -output_latency_function * -cubeb_output_latency_load_method(int version) -{ - output_latency_function * ol = NULL; - ol = calloc(1, sizeof(output_latency_function)); - - ol->version = version; - - if (ol->version > ANDROID_JELLY_BEAN_MR1_4_2){ - ol->from_jni = cubeb_jni_init(); - return ol; - } - - ol->from_lib = cubeb_load_media_library(); - return ol; -} - -bool -cubeb_output_latency_method_is_loaded(output_latency_function * ol) -{ - assert(ol && (ol->from_jni || ol->from_lib)); - if (ol->version > ANDROID_JELLY_BEAN_MR1_4_2){ - return !!ol->from_jni; - } - - return !!ol->from_lib; -} - -void -cubeb_output_latency_unload_method(output_latency_function * ol) -{ - if (!ol) { - return; - } - - if (ol->version > ANDROID_JELLY_BEAN_MR1_4_2 && ol->from_jni) { - cubeb_jni_destroy(ol->from_jni); - } - - if (ol->version <= ANDROID_JELLY_BEAN_MR1_4_2 && ol->from_lib) { - cubeb_close_media_library(ol->from_lib); - } - - free(ol); -} - -uint32_t -cubeb_get_output_latency(output_latency_function * ol) -{ - assert(cubeb_output_latency_method_is_loaded(ol)); - - if (ol->version > ANDROID_JELLY_BEAN_MR1_4_2){ - return cubeb_get_output_latency_from_jni(ol->from_jni); - } - - return cubeb_get_output_latency_from_media_library(ol->from_lib); -} - -#endif // _CUBEB_OUTPUT_LATENCY_H_ diff --git a/media/libcubeb/src/android/cubeb_media_library.h b/media/libcubeb/src/android/cubeb_media_library.h deleted file mode 100644 index ab21b779dfad..000000000000 --- a/media/libcubeb/src/android/cubeb_media_library.h +++ /dev/null @@ -1,62 +0,0 @@ -#ifndef _CUBEB_MEDIA_LIBRARY_H_ -#define _CUBEB_MEDIA_LIBRARY_H_ - -struct media_lib { - void * libmedia; - int32_t (* get_output_latency)(uint32_t * latency, int stream_type); -}; - -typedef struct media_lib media_lib; - -media_lib * -cubeb_load_media_library() -{ - media_lib ml = {0}; - ml.libmedia = dlopen("libmedia.so", RTLD_LAZY); - if (!ml.libmedia) { - return NULL; - } - - // Get the latency, in ms, from AudioFlinger. First, try the most recent signature. - // status_t AudioSystem::getOutputLatency(uint32_t* latency, audio_stream_type_t streamType) - ml.get_output_latency = - dlsym(ml.libmedia, "_ZN7android11AudioSystem16getOutputLatencyEPj19audio_stream_type_t"); - if (!ml.get_output_latency) { - // In case of failure, try the signature from legacy version. - // status_t AudioSystem::getOutputLatency(uint32_t* latency, int streamType) - ml.get_output_latency = - dlsym(ml.libmedia, "_ZN7android11AudioSystem16getOutputLatencyEPji"); - if (!ml.get_output_latency) { - return NULL; - } - } - - media_lib * rv = NULL; - rv = calloc(1, sizeof(media_lib)); - assert(rv); - *rv = ml; - return rv; -} - -void -cubeb_close_media_library(media_lib * ml) -{ - dlclose(ml->libmedia); - ml->libmedia = NULL; - ml->get_output_latency = NULL; - free(ml); -} - -uint32_t -cubeb_get_output_latency_from_media_library(media_lib * ml) -{ - uint32_t latency = 0; - const int audio_stream_type_music = 3; - int32_t r = ml->get_output_latency(&latency, audio_stream_type_music); - if (r) { - return 0; - } - return latency; -} - -#endif // _CUBEB_MEDIA_LIBRARY_H_ diff --git a/media/libcubeb/src/cubeb-jni-instances.h b/media/libcubeb/src/cubeb-jni-instances.h deleted file mode 100644 index dad62e4d3357..000000000000 --- a/media/libcubeb/src/cubeb-jni-instances.h +++ /dev/null @@ -1,34 +0,0 @@ -#ifndef _CUBEB_JNI_INSTANCES_H_ -#define _CUBEB_JNI_INSTANCES_H_ - -#include "GeneratedJNIWrappers.h" -#include "mozilla/jni/Utils.h" - -/* - * The methods in this file offer a way to pass in the required - * JNI instances in the cubeb library. By default they return NULL. - * In this case part of the cubeb API that depends on JNI - * will return CUBEB_ERROR_NOT_SUPPORTED. Currently only one - * method depends on that: - * - * cubeb_stream_get_position() - * - * Users that want to use that cubeb API method must "override" - * the methods bellow to return a valid instance of JavaVM - * and application's Context object. - * */ - -JavaVM * -cubeb_jni_get_java_vm() -{ - return mozilla::jni::GetVM(); -} - -jobject -cubeb_jni_get_context_instance() -{ - auto context = mozilla::java::GeckoAppShell::GetApplicationContext(); - return context.Forget(); -} - -#endif //_CUBEB_JNI_INSTANCES_H_ diff --git a/media/libcubeb/src/cubeb-jni.cpp b/media/libcubeb/src/cubeb-jni.cpp deleted file mode 100644 index 3eba97d74dcc..000000000000 --- a/media/libcubeb/src/cubeb-jni.cpp +++ /dev/null @@ -1,88 +0,0 @@ -#include "jni.h" -#include -#include "cubeb-jni-instances.h" - -#define AUDIO_STREAM_TYPE_MUSIC 3 - -JNIEnv * -cubeb_jni_get_env_for_thread(JavaVM * java_vm) -{ - JNIEnv * env = nullptr; - if (!java_vm->AttachCurrentThread(&env, nullptr)) { - assert(env); - return env; - } - - assert(false && "Failed to get JNIEnv for thread"); - return nullptr; // unreachable -} - -struct cubeb_jni { - JavaVM * s_java_vm = nullptr; - jobject s_audio_manager_obj = nullptr; - jclass s_audio_manager_class = nullptr; - jmethodID s_get_output_latency_id = nullptr; -}; - -extern "C" -cubeb_jni * -cubeb_jni_init() -{ - JavaVM * javaVM = cubeb_jni_get_java_vm(); - jobject ctx_obj = cubeb_jni_get_context_instance(); - - if (!javaVM || !ctx_obj) { - return nullptr; - } - - JNIEnv * jni_env = cubeb_jni_get_env_for_thread(javaVM); - assert(jni_env); - - cubeb_jni * cubeb_jni_ptr = new cubeb_jni; - assert(cubeb_jni_ptr); - - cubeb_jni_ptr->s_java_vm = javaVM; - - // Find the audio manager object and make it global to call it from another method - jclass context_class = jni_env->FindClass("android/content/Context"); - jfieldID audio_service_field = jni_env->GetStaticFieldID(context_class, "AUDIO_SERVICE", "Ljava/lang/String;"); - jstring jstr = (jstring)jni_env->GetStaticObjectField(context_class, audio_service_field); - jmethodID get_system_service_id = jni_env->GetMethodID(context_class, "getSystemService", "(Ljava/lang/String;)Ljava/lang/Object;"); - jobject audio_manager_obj = jni_env->CallObjectMethod(ctx_obj, get_system_service_id, jstr); - cubeb_jni_ptr->s_audio_manager_obj = reinterpret_cast(jni_env->NewGlobalRef(audio_manager_obj)); - - // Make the audio manager class a global reference in order to preserve method id - jclass audio_manager_class = jni_env->FindClass("android/media/AudioManager"); - cubeb_jni_ptr->s_audio_manager_class = reinterpret_cast(jni_env->NewGlobalRef(audio_manager_class)); - cubeb_jni_ptr->s_get_output_latency_id = jni_env->GetMethodID (audio_manager_class, "getOutputLatency", "(I)I"); - - jni_env->DeleteLocalRef(ctx_obj); - jni_env->DeleteLocalRef(context_class); - jni_env->DeleteLocalRef(jstr); - jni_env->DeleteLocalRef(audio_manager_obj); - jni_env->DeleteLocalRef(audio_manager_class); - - return cubeb_jni_ptr; -} - -extern "C" -int cubeb_get_output_latency_from_jni(cubeb_jni * cubeb_jni_ptr) -{ - assert(cubeb_jni_ptr); - JNIEnv * jni_env = cubeb_jni_get_env_for_thread(cubeb_jni_ptr->s_java_vm); - return jni_env->CallIntMethod(cubeb_jni_ptr->s_audio_manager_obj, cubeb_jni_ptr->s_get_output_latency_id, AUDIO_STREAM_TYPE_MUSIC); //param: AudioManager.STREAM_MUSIC -} - -extern "C" -void cubeb_jni_destroy(cubeb_jni * cubeb_jni_ptr) -{ - assert(cubeb_jni_ptr); - - JNIEnv * jni_env = cubeb_jni_get_env_for_thread(cubeb_jni_ptr->s_java_vm); - assert(jni_env); - - jni_env->DeleteGlobalRef(cubeb_jni_ptr->s_audio_manager_obj); - jni_env->DeleteGlobalRef(cubeb_jni_ptr->s_audio_manager_class); - - delete cubeb_jni_ptr; -} diff --git a/media/libcubeb/src/cubeb-jni.h b/media/libcubeb/src/cubeb-jni.h deleted file mode 100644 index 8c7ddb6acf4d..000000000000 --- a/media/libcubeb/src/cubeb-jni.h +++ /dev/null @@ -1,10 +0,0 @@ -#ifndef _CUBEB_JNI_H_ -#define _CUBEB_JNI_H_ - -typedef struct cubeb_jni cubeb_jni; - -cubeb_jni * cubeb_jni_init(); -int cubeb_get_output_latency_from_jni(cubeb_jni * cubeb_jni_ptr); -void cubeb_jni_destroy(cubeb_jni * cubeb_jni_ptr); - -#endif // _CUBEB_JNI_H_ diff --git a/media/libcubeb/src/cubeb_opensl.c b/media/libcubeb/src/cubeb_opensl.c index a760fd8111ef..8e9d4c73f2d6 100644 --- a/media/libcubeb/src/cubeb_opensl.c +++ b/media/libcubeb/src/cubeb_opensl.c @@ -26,7 +26,6 @@ #include "cubeb_resampler.h" #include "cubeb-sles.h" #include "cubeb_array_queue.h" -#include "android/cubeb-output-latency.h" #if defined(__ANDROID__) #ifdef LOG @@ -62,13 +61,14 @@ #endif #define DEFAULT_SAMPLE_RATE 48000 -#define DEFAULT_NUM_OF_FRAMES 480 static struct cubeb_ops const opensl_ops; struct cubeb { struct cubeb_ops const * ops; void * lib; + void * libmedia; + int32_t (* get_output_latency)(uint32_t * latency, int stream_type); SLInterfaceID SL_IID_BUFFERQUEUE; SLInterfaceID SL_IID_PLAY; #if defined(__ANDROID__) @@ -80,11 +80,11 @@ struct cubeb { SLObjectItf engObj; SLEngineItf eng; SLObjectItf outmixObj; - output_latency_function * p_output_latency_function; }; #define NELEMS(A) (sizeof(A) / sizeof A[0]) #define NBUFS 4 +#define AUDIO_STREAM_TYPE_MUSIC 3 struct cubeb_stream { /* Note: Must match cubeb_stream layout in cubeb.c. */ @@ -153,7 +153,7 @@ struct cubeb_stream { cubeb_state_callback state_callback; cubeb_resampler * resampler; - unsigned int user_output_rate; + unsigned int inputrate; unsigned int output_configured_rate; unsigned int latency_frames; int64_t lastPosition; @@ -236,6 +236,7 @@ static void play_callback(SLPlayItf caller, void * user_ptr, SLuint32 event) { cubeb_stream * stm = user_ptr; + int draining; assert(stm); switch (event) { case SL_PLAYEVENT_HEADATMARKER: @@ -667,11 +668,30 @@ opensl_init(cubeb ** context, char const * context_name) ctx->ops = &opensl_ops; ctx->lib = dlopen("libOpenSLES.so", RTLD_LAZY); - if (!ctx->lib) { + ctx->libmedia = dlopen("libmedia.so", RTLD_LAZY); + if (!ctx->lib || !ctx->libmedia) { free(ctx); return CUBEB_ERROR; } + /* Get the latency, in ms, from AudioFlinger */ + /* status_t AudioSystem::getOutputLatency(uint32_t* latency, + * audio_stream_type_t streamType) */ + /* First, try the most recent signature. */ + ctx->get_output_latency = + dlsym(ctx->libmedia, "_ZN7android11AudioSystem16getOutputLatencyEPj19audio_stream_type_t"); + if (!ctx->get_output_latency) { + /* in case of failure, try the legacy version. */ + /* status_t AudioSystem::getOutputLatency(uint32_t* latency, + * int streamType) */ + ctx->get_output_latency = + dlsym(ctx->libmedia, "_ZN7android11AudioSystem16getOutputLatencyEPji"); + if (!ctx->get_output_latency) { + opensl_destroy(ctx); + return CUBEB_ERROR; + } + } + typedef SLresult (*slCreateEngine_t)(SLObjectItf *, SLuint32, const SLEngineOption *, @@ -741,11 +761,6 @@ opensl_init(cubeb ** context, char const * context_name) return CUBEB_ERROR; } - ctx->p_output_latency_function = cubeb_output_latency_load_method(android_version); - if (!ctx->p_output_latency_function) { - LOG("Warning: output latency is not available, cubeb_stream_get_position() is not supported"); - } - *context = ctx; LOG("Cubeb init (%p) success", ctx); @@ -769,6 +784,124 @@ opensl_get_max_channel_count(cubeb * ctx, uint32_t * max_channels) return CUBEB_OK; } +static int +opensl_get_preferred_sample_rate(cubeb * ctx, uint32_t * rate) +{ + /* https://android.googlesource.com/platform/ndk.git/+/master/docs/opensles/index.html + * We don't want to deal with JNI here (and we don't have Java on b2g anyways), + * so we just dlopen the library and get the two symbols we need. */ + int r; + void * libmedia; + uint32_t (*get_primary_output_samplingrate)(); + uint32_t (*get_output_samplingrate)(int * samplingRate, int streamType); + + libmedia = dlopen("libmedia.so", RTLD_LAZY); + if (!libmedia) { + return CUBEB_ERROR; + } + + /* uint32_t AudioSystem::getPrimaryOutputSamplingRate(void) */ + get_primary_output_samplingrate = + dlsym(libmedia, "_ZN7android11AudioSystem28getPrimaryOutputSamplingRateEv"); + if (!get_primary_output_samplingrate) { + /* fallback to + * status_t AudioSystem::getOutputSamplingRate(int* samplingRate, int streamType) + * if we cannot find getPrimaryOutputSamplingRate. */ + get_output_samplingrate = + dlsym(libmedia, "_ZN7android11AudioSystem21getOutputSamplingRateEPj19audio_stream_type_t"); + if (!get_output_samplingrate) { + /* Another signature exists, with a int instead of an audio_stream_type_t */ + get_output_samplingrate = + dlsym(libmedia, "_ZN7android11AudioSystem21getOutputSamplingRateEPii"); + if (!get_output_samplingrate) { + dlclose(libmedia); + return CUBEB_ERROR; + } + } + } + + if (get_primary_output_samplingrate) { + *rate = get_primary_output_samplingrate(); + } else { + /* We don't really know about the type, here, so we just pass music. */ + r = get_output_samplingrate((int *) rate, AUDIO_STREAM_TYPE_MUSIC); + if (r) { + dlclose(libmedia); + return CUBEB_ERROR; + } + } + + dlclose(libmedia); + + /* Depending on which method we called above, we can get a zero back, yet have + * a non-error return value, especially if the audio system is not + * ready/shutting down (i.e. when we can't get our hand on the AudioFlinger + * thread). */ + if (*rate == 0) { + return CUBEB_ERROR; + } + + return CUBEB_OK; +} + +static int +opensl_get_min_latency(cubeb * ctx, cubeb_stream_params params, uint32_t * latency_frames) +{ + /* https://android.googlesource.com/platform/ndk.git/+/master/docs/opensles/index.html + * We don't want to deal with JNI here (and we don't have Java on b2g anyways), + * so we just dlopen the library and get the two symbols we need. */ + + int r; + void * libmedia; + size_t (*get_primary_output_frame_count)(void); + int (*get_output_frame_count)(size_t * frameCount, int streamType); + uint32_t primary_sampling_rate; + size_t primary_buffer_size; + + r = opensl_get_preferred_sample_rate(ctx, &primary_sampling_rate); + + if (r) { + return CUBEB_ERROR; + } + + libmedia = dlopen("libmedia.so", RTLD_LAZY); + if (!libmedia) { + return CUBEB_ERROR; + } + + /* JB variant */ + /* size_t AudioSystem::getPrimaryOutputFrameCount(void) */ + get_primary_output_frame_count = + dlsym(libmedia, "_ZN7android11AudioSystem26getPrimaryOutputFrameCountEv"); + if (!get_primary_output_frame_count) { + /* ICS variant */ + /* status_t AudioSystem::getOutputFrameCount(int* frameCount, int streamType) */ + get_output_frame_count = + dlsym(libmedia, "_ZN7android11AudioSystem19getOutputFrameCountEPii"); + if (!get_output_frame_count) { + dlclose(libmedia); + return CUBEB_ERROR; + } + } + + if (get_primary_output_frame_count) { + primary_buffer_size = get_primary_output_frame_count(); + } else { + if (get_output_frame_count(&primary_buffer_size, AUDIO_STREAM_TYPE_MUSIC) != 0) { + return CUBEB_ERROR; + } + } + + /* To get a fast track in Android's mixer, we need to be at the native + * samplerate, which is device dependant. Some devices might be able to + * resample when playing a fast track, but it's pretty rare. */ + *latency_frames = primary_buffer_size; + + dlclose(libmedia); + + return CUBEB_OK; +} + static void opensl_destroy(cubeb * ctx) { @@ -777,8 +910,7 @@ opensl_destroy(cubeb * ctx) if (ctx->engObj) cubeb_destroy_sles_engine(&ctx->engObj); dlclose(ctx->lib); - if (ctx->p_output_latency_function) - cubeb_output_latency_unload_method(ctx->p_output_latency_function); + dlclose(ctx->libmedia); free(ctx); } @@ -865,9 +997,13 @@ opensl_configure_capture(cubeb_stream * stm, cubeb_stream_params * params) // api for input device this is a safe choice. stm->input_device_rate = stm->output_configured_rate; } else { - // The output preferred rate is used for an input only scenario. - // The default rate expected to be supported from all android devices. - stm->input_device_rate = DEFAULT_SAMPLE_RATE; + // The output preferred rate is used for input only scenario. This is + // the correct rate to use to get a fast track for input only. + r = opensl_get_preferred_sample_rate(stm->context, &stm->input_device_rate); + if (r != CUBEB_OK) { + // If everything else fail use a safe choice for Android. + stm->input_device_rate = DEFAULT_SAMPLE_RATE; + } } lDataFormat.samplesPerSec = stm->input_device_rate * 1000; res = (*stm->context->eng)->CreateAudioRecorder(stm->context->eng, @@ -978,7 +1114,7 @@ opensl_configure_playback(cubeb_stream * stm, cubeb_stream_params * params) { assert(stm); assert(params); - stm->user_output_rate = params->rate; + stm->inputrate = params->rate; stm->framesize = params->channels * sizeof(int16_t); stm->lastPosition = -1; stm->lastPositionTimeStamp = 0; @@ -1015,7 +1151,20 @@ opensl_configure_playback(cubeb_stream * stm, cubeb_stream_params * params) { #endif assert(NELEMS(ids) == NELEMS(req)); - uint32_t preferred_sampling_rate = stm->user_output_rate; + unsigned int latency_frames = stm->latency_frames; + uint32_t preferred_sampling_rate = stm->inputrate; +#if defined(__ANDROID__) + if (get_android_version() >= ANDROID_VERSION_MARSHMALLOW) { + // Reset preferred samping rate to trigger fallback to native sampling rate. + preferred_sampling_rate = 0; + if (opensl_get_min_latency(stm->context, *params, &latency_frames) != CUBEB_OK) { + // Default to AudioFlinger's advertised fast track latency of 10ms. + latency_frames = 440; + } + stm->latency_frames = latency_frames; + } +#endif + SLresult res = SL_RESULT_CONTENT_UNSUPPORTED; if (preferred_sampling_rate) { res = (*stm->context->eng)->CreateAudioPlayer(stm->context->eng, @@ -1028,9 +1177,12 @@ opensl_configure_playback(cubeb_stream * stm, cubeb_stream_params * params) { } // Sample rate not supported? Try again with primary sample rate! - if (res == SL_RESULT_CONTENT_UNSUPPORTED && - preferred_sampling_rate != DEFAULT_SAMPLE_RATE) { - preferred_sampling_rate = DEFAULT_SAMPLE_RATE; + if (res == SL_RESULT_CONTENT_UNSUPPORTED) { + if (opensl_get_preferred_sample_rate(stm->context, &preferred_sampling_rate)) { + // If fail default is used + preferred_sampling_rate = DEFAULT_SAMPLE_RATE; + } + format.samplesPerSec = preferred_sampling_rate * 1000; res = (*stm->context->eng)->CreateAudioPlayer(stm->context->eng, &stm->playerObj, @@ -1048,7 +1200,7 @@ opensl_configure_playback(cubeb_stream * stm, cubeb_stream_params * params) { stm->output_configured_rate = preferred_sampling_rate; stm->bytespersec = stm->output_configured_rate * stm->framesize; - stm->queuebuf_len = stm->framesize * stm->latency_frames; + stm->queuebuf_len = stm->framesize * latency_frames; // Calculate the capacity of input array stm->queuebuf_capacity = NBUFS; @@ -1185,7 +1337,7 @@ opensl_stream_init(cubeb * ctx, cubeb_stream ** stream, char const * stream_name stm->data_callback = data_callback; stm->state_callback = state_callback; stm->user_ptr = user_ptr; - stm->latency_frames = latency_frames ? latency_frames : DEFAULT_NUM_OF_FRAMES; + stm->latency_frames = latency_frames; stm->input_enabled = (input_stream_params) ? 1 : 0; stm->output_enabled = (output_stream_params) ? 1 : 0; stm->shutdown = 1; @@ -1449,12 +1601,11 @@ static int opensl_stream_get_position(cubeb_stream * stm, uint64_t * position) { SLmillisecond msec; - uint32_t compensation_msec = 0; + uint64_t samplerate; SLresult res; - - if (!cubeb_output_latency_method_is_loaded(stm->context->p_output_latency_function)) { - return CUBEB_ERROR_NOT_SUPPORTED; - } + int r; + uint32_t mixer_latency; + uint32_t compensation_msec = 0; res = (*stm->play)->GetPosition(stm->play, &msec); if (res != SL_RESULT_SUCCESS) @@ -1470,11 +1621,15 @@ opensl_stream_get_position(cubeb_stream * stm, uint64_t * position) stm->lastPosition = msec; } - uint64_t samplerate = stm->user_output_rate; - uint32_t mixer_latency = cubeb_get_output_latency(stm->context->p_output_latency_function); + samplerate = stm->inputrate; + + r = stm->context->get_output_latency(&mixer_latency, AUDIO_STREAM_TYPE_MUSIC); + if (r) { + return CUBEB_ERROR; + } pthread_mutex_lock(&stm->mutex); - int64_t maximum_position = stm->written * (int64_t)stm->user_output_rate / stm->output_configured_rate; + int64_t maximum_position = stm->written * (int64_t)stm->inputrate / stm->output_configured_rate; pthread_mutex_unlock(&stm->mutex); assert(maximum_position >= 0); @@ -1497,6 +1652,24 @@ opensl_stream_get_position(cubeb_stream * stm, uint64_t * position) return CUBEB_OK; } +int +opensl_stream_get_latency(cubeb_stream * stm, uint32_t * latency) +{ + int r; + uint32_t mixer_latency; // The latency returned by AudioFlinger is in ms. + + /* audio_stream_type_t is an int, so this is okay. */ + r = stm->context->get_output_latency(&mixer_latency, AUDIO_STREAM_TYPE_MUSIC); + if (r) { + return CUBEB_ERROR; + } + + *latency = stm->latency_frames + // OpenSL latency + mixer_latency * stm->inputrate / 1000; // AudioFlinger latency + + return CUBEB_OK; +} + int opensl_stream_set_volume(cubeb_stream * stm, float volume) { @@ -1532,8 +1705,8 @@ static struct cubeb_ops const opensl_ops = { .init = opensl_init, .get_backend_id = opensl_get_backend_id, .get_max_channel_count = opensl_get_max_channel_count, - .get_min_latency = NULL, - .get_preferred_sample_rate = NULL, + .get_min_latency = opensl_get_min_latency, + .get_preferred_sample_rate = opensl_get_preferred_sample_rate, .get_preferred_channel_layout = NULL, .enumerate_devices = NULL, .device_collection_destroy = NULL, @@ -1544,7 +1717,7 @@ static struct cubeb_ops const opensl_ops = { .stream_stop = opensl_stream_stop, .stream_reset_default_device = NULL, .stream_get_position = opensl_stream_get_position, - .stream_get_latency = NULL, + .stream_get_latency = opensl_stream_get_latency, .stream_set_volume = opensl_stream_set_volume, .stream_set_panning = NULL, .stream_get_current_device = NULL, diff --git a/media/libcubeb/src/moz.build b/media/libcubeb/src/moz.build index 15071f147ab8..6484e61fa915 100644 --- a/media/libcubeb/src/moz.build +++ b/media/libcubeb/src/moz.build @@ -77,7 +77,6 @@ if CONFIG['OS_TARGET'] == 'WINNT': if CONFIG['OS_TARGET'] == 'Android': SOURCES += ['cubeb_opensl.c'] SOURCES += ['cubeb_resampler.cpp'] - SOURCES += ['cubeb-jni.cpp'] DEFINES['USE_OPENSL'] = True SOURCES += [ 'cubeb_audiotrack.c', diff --git a/media/libcubeb/update.sh b/media/libcubeb/update.sh index 0b33550a4923..827a0db37c50 100755 --- a/media/libcubeb/update.sh +++ b/media/libcubeb/update.sh @@ -20,10 +20,6 @@ cp $1/src/cubeb_log.h src cp $1/src/cubeb_mixer.cpp src cp $1/src/cubeb_mixer.h src cp $1/src/cubeb_opensl.c src -cp $1/src/cubeb-jni.cpp src -cp $1/src/cubeb-jni.h src -cp $1/src/android/cubeb-output-latency.h src/android -cp $1/src/android/cubeb_media_library.h src/android cp $1/src/cubeb_osx_run_loop.h src cp $1/src/cubeb_panner.cpp src cp $1/src/cubeb_panner.h src diff --git a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java index 7dbb286cae04..d17c70913354 100644 --- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java +++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java @@ -1836,7 +1836,7 @@ public class GeckoAppShell return sScreenSize; } - @WrapForJNI(calledFrom = "any") + @WrapForJNI(calledFrom = "gecko") public static int getAudioOutputFramesPerBuffer() { if (SysInfo.getVersion() < 17) { return 0; @@ -1853,7 +1853,7 @@ public class GeckoAppShell return Integer.parseInt(prop); } - @WrapForJNI(calledFrom = "any") + @WrapForJNI(calledFrom = "gecko") public static int getAudioOutputSampleRate() { if (SysInfo.getVersion() < 17) { return 0; diff --git a/widget/android/GeneratedJNIWrappers.h b/widget/android/GeneratedJNIWrappers.h index 46c780942619..3eb178b0894e 100644 --- a/widget/android/GeneratedJNIWrappers.h +++ b/widget/android/GeneratedJNIWrappers.h @@ -814,7 +814,7 @@ public: static const mozilla::jni::ExceptionMode exceptionMode = mozilla::jni::ExceptionMode::ABORT; static const mozilla::jni::CallingThread callingThread = - mozilla::jni::CallingThread::ANY; + mozilla::jni::CallingThread::GECKO; static const mozilla::jni::DispatchTarget dispatchTarget = mozilla::jni::DispatchTarget::CURRENT; }; @@ -833,7 +833,7 @@ public: static const mozilla::jni::ExceptionMode exceptionMode = mozilla::jni::ExceptionMode::ABORT; static const mozilla::jni::CallingThread callingThread = - mozilla::jni::CallingThread::ANY; + mozilla::jni::CallingThread::GECKO; static const mozilla::jni::DispatchTarget dispatchTarget = mozilla::jni::DispatchTarget::CURRENT; }; From f943b48940ad167546da4c94f97215c65bb90fae Mon Sep 17 00:00:00 2001 From: Johann Hofmann Date: Thu, 22 Feb 2018 00:31:16 +0100 Subject: [PATCH 30/48] Bug 1431027 - Update disk space warning to reflect the latest preferences. r=nhnt11 This changes the disk space notification to show the correct preferences path and to use the correct openPreferences arguments. MozReview-Commit-ID: BuKAUvDjp9T --HG-- extra : rebase_source : 678f52801d100a980f529fc565d1009c38320ae4 --- browser/base/content/browser.js | 6 +++--- browser/components/preferences/in-content/privacy.xul | 2 +- .../chrome/browser/preferences/preferences.properties | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index ac7675c3252e..384e0c984008 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -537,11 +537,11 @@ const gStoragePressureObserver = { } else { // The firefox-used space >= 5GB, then guide users to about:preferences // to clear some data stored on firefox by websites. - let descriptionStringID = "spaceAlert.over5GB.message"; + let descriptionStringID = "spaceAlert.over5GB.message1"; let prefButtonLabelStringID = "spaceAlert.over5GB.prefButton.label"; let prefButtonAccesskeyStringID = "spaceAlert.over5GB.prefButton.accesskey"; if (AppConstants.platform == "win") { - descriptionStringID = "spaceAlert.over5GB.messageWin"; + descriptionStringID = "spaceAlert.over5GB.messageWin1"; prefButtonLabelStringID = "spaceAlert.over5GB.prefButtonWin.label"; prefButtonAccesskeyStringID = "spaceAlert.over5GB.prefButtonWin.accesskey"; } @@ -553,7 +553,7 @@ const gStoragePressureObserver = { // The advanced subpanes are only supported in the old organization, which will // be removed by bug 1349689. let win = gBrowser.ownerGlobal; - win.openPreferences("panePrivacy", { origin: "storagePressure" }); + win.openPreferences("privacy-sitedata", { origin: "storagePressure" }); } }); } diff --git a/browser/components/preferences/in-content/privacy.xul b/browser/components/preferences/in-content/privacy.xul index 0802e4a4d4cf..be6a24949da8 100644 --- a/browser/components/preferences/in-content/privacy.xul +++ b/browser/components/preferences/in-content/privacy.xul @@ -170,7 +170,7 @@

""".format(direction)) + + class TestVisibility(MarionetteTestCase): def testShouldAllowTheUserToTellIfAnElementIsDisplayedOrNot(self): @@ -103,21 +123,17 @@ class TestVisibility(MarionetteTestCase): self.assertFalse(child.is_displayed()) def testShouldClickOnELementPartiallyOffLeft(self): - test_html = self.marionette.absolute_url("element_left.html") - self.marionette.navigate(test_html) + test_html = self.marionette.navigate(element_direction_doc("left")) self.marionette.find_element(By.CSS_SELECTOR, '.element').click() def testShouldClickOnELementPartiallyOffRight(self): - test_html = self.marionette.absolute_url("element_right.html") - self.marionette.navigate(test_html) + test_html = self.marionette.navigate(element_direction_doc("right")) self.marionette.find_element(By.CSS_SELECTOR, '.element').click() def testShouldClickOnELementPartiallyOffTop(self): - test_html = self.marionette.absolute_url("element_top.html") - self.marionette.navigate(test_html) + test_html = self.marionette.navigate(element_direction_doc("top")) self.marionette.find_element(By.CSS_SELECTOR, '.element').click() def testShouldClickOnELementPartiallyOffBottom(self): - test_html = self.marionette.absolute_url("element_bottom.html") - self.marionette.navigate(test_html) + test_html = self.marionette.navigate(element_direction_doc("bottom")) self.marionette.find_element(By.CSS_SELECTOR, '.element').click() diff --git a/testing/marionette/harness/marionette_harness/www/element_bottom.html b/testing/marionette/harness/marionette_harness/www/element_bottom.html deleted file mode 100644 index 470199a078e9..000000000000 --- a/testing/marionette/harness/marionette_harness/www/element_bottom.html +++ /dev/null @@ -1,12 +0,0 @@ - - -
- diff --git a/testing/marionette/harness/marionette_harness/www/element_left.html b/testing/marionette/harness/marionette_harness/www/element_left.html deleted file mode 100644 index c98b945f9448..000000000000 --- a/testing/marionette/harness/marionette_harness/www/element_left.html +++ /dev/null @@ -1,12 +0,0 @@ - - -
- diff --git a/testing/marionette/harness/marionette_harness/www/element_right.html b/testing/marionette/harness/marionette_harness/www/element_right.html deleted file mode 100644 index ff8774ab6d33..000000000000 --- a/testing/marionette/harness/marionette_harness/www/element_right.html +++ /dev/null @@ -1,12 +0,0 @@ - - -
- diff --git a/testing/marionette/harness/marionette_harness/www/element_top.html b/testing/marionette/harness/marionette_harness/www/element_top.html deleted file mode 100644 index b975e34c46f7..000000000000 --- a/testing/marionette/harness/marionette_harness/www/element_top.html +++ /dev/null @@ -1,12 +0,0 @@ - - -
- From 9f797a66bf21fe1bf9d09ff1a8973c78c8986053 Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Mon, 26 Feb 2018 11:59:23 +0000 Subject: [PATCH 47/48] Bug 1441113 - Can only update a mounted or mounting component when closing Memory Tool r=nchevobbe The onclick handler of the toolbox close button looked like this... spot the deliberate mistake: ``` onClick: () => { closeToolbox(); focusButton(closeButtonId); }, ``` So we were closing the toolbox and then trying to focus the toolbox's close button. There is also an obvious race condition with setState inside the toolbox controller not using a callback even though the next line calls a function that uses this.state, which could cause intermittent issues. MozReview-Commit-ID: 9VRcZw4RvE5 --HG-- extra : rebase_source : 17c21aafe5a72042c23472342c53fd730784a5ae --- .../components/toolbox-controller.js | 28 ++++++++----------- .../framework/components/toolbox-toolbar.js | 1 - 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/devtools/client/framework/components/toolbox-controller.js b/devtools/client/framework/components/toolbox-controller.js index c9fe249efc9b..ecf21ac779cd 100644 --- a/devtools/client/framework/components/toolbox-controller.js +++ b/devtools/client/framework/components/toolbox-controller.js @@ -102,19 +102,18 @@ class ToolboxController extends Component { } setCurrentToolId(currentToolId) { - this.setState({currentToolId}); - // Also set the currently focused button to this tool. - this.setFocusedButton(currentToolId); + this.setState({currentToolId}, () => { + // Also set the currently focused button to this tool. + this.setFocusedButton(currentToolId); + }); } setCanRender() { - this.setState({ canRender: true }); - this.updateButtonIds(); + this.setState({ canRender: true }, this.updateButtonIds); } setOptionsPanel(optionsPanel) { - this.setState({ optionsPanel }); - this.updateButtonIds(); + this.setState({ optionsPanel }, this.updateButtonIds); } highlightTool(highlightedTool) { @@ -132,23 +131,19 @@ class ToolboxController extends Component { } setDockButtonsEnabled(areDockButtonsEnabled) { - this.setState({ areDockButtonsEnabled }); - this.updateButtonIds(); + this.setState({ areDockButtonsEnabled }, this.updateButtonIds); } setHostTypes(hostTypes) { - this.setState({ hostTypes }); - this.updateButtonIds(); + this.setState({ hostTypes }, this.updateButtonIds); } setCanCloseToolbox(canCloseToolbox) { - this.setState({ canCloseToolbox }); - this.updateButtonIds(); + this.setState({ canCloseToolbox }, this.updateButtonIds); } setPanelDefinitions(panelDefinitions) { - this.setState({ panelDefinitions }); - this.updateButtonIds(); + this.setState({ panelDefinitions }, this.updateButtonIds); } get panelDefinitions() { @@ -164,8 +159,7 @@ class ToolboxController extends Component { button.on("updatechecked", this.state.checkedButtonsUpdated); }); - this.setState({ toolboxButtons }); - this.updateButtonIds(); + this.setState({ toolboxButtons }, this.updateButtonIds); } setCanMinimize(canMinimize) { diff --git a/devtools/client/framework/components/toolbox-toolbar.js b/devtools/client/framework/components/toolbox-toolbar.js index 3759c35f3a55..b20cacfa8946 100644 --- a/devtools/client/framework/components/toolbox-toolbar.js +++ b/devtools/client/framework/components/toolbox-toolbar.js @@ -230,7 +230,6 @@ function renderDockButtons(props) { title: L10N.getStr("toolbox.closebutton.tooltip"), onClick: () => { closeToolbox(); - focusButton(closeButtonId); }, tabIndex: focusedButton === "toolbox-close" ? "0" : "-1", }) From 6429bea50f608f3e904f63ade2216ea4c42f973a Mon Sep 17 00:00:00 2001 From: Jonathan Kingston Date: Mon, 26 Feb 2018 16:07:24 +0000 Subject: [PATCH 48/48] Bug 1439651 - Fix uriloader/exthandler/tests/mochitest/test_web_protocol_handlers.html for builds that support not secure registerProtocolHandler. r=baku MozReview-Commit-ID: Cw5iRzxQR3X --HG-- extra : rebase_source : 5cc4789e57433b70b45dfa2fbf5220d0127c527b --- .../tests/mochitest/test_web_protocol_handlers.html | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/uriloader/exthandler/tests/mochitest/test_web_protocol_handlers.html b/uriloader/exthandler/tests/mochitest/test_web_protocol_handlers.html index 2fef553a26c1..2af885f97673 100644 --- a/uriloader/exthandler/tests/mochitest/test_web_protocol_handlers.html +++ b/uriloader/exthandler/tests/mochitest/test_web_protocol_handlers.html @@ -10,8 +10,14 @@