From 268ef43bd751afa4c349e0b2731ec4bac305d7a9 Mon Sep 17 00:00:00 2001 From: Brad Werth Date: Wed, 5 Dec 2018 17:16:34 +0000 Subject: [PATCH 01/34] Bug 1504659 Part 1: Make RefreshVisualViewportSize allow non-APZ zooming, and call it during RefreshViewportSize. r=botond Differential Revision: https://phabricator.services.mozilla.com/D13173 --HG-- extra : moz-landing-system : lando --- layout/base/MobileViewportManager.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/layout/base/MobileViewportManager.cpp b/layout/base/MobileViewportManager.cpp index 61316c980440..6c5b08bed1b6 100644 --- a/layout/base/MobileViewportManager.cpp +++ b/layout/base/MobileViewportManager.cpp @@ -397,10 +397,6 @@ void MobileViewportManager::RefreshVisualViewportSize() { // This function is a subset of RefreshViewportSize, and only updates the // visual viewport size. - if (!gfxPrefs::APZAllowZooming()) { - return; - } - ScreenIntSize displaySize = ViewAs( mDisplaySize, PixelCastJustification::LayoutDeviceIsScreenForBounds); @@ -476,6 +472,10 @@ void MobileViewportManager::RefreshViewportSize(bool aForceAdjustResolution) { if (gfxPrefs::APZAllowZooming()) { UpdateResolution(viewportInfo, displaySize, viewport, displayWidthChangeRatio, UpdateType::ViewportSize); + } else { + // Even without zoom, we need to update that the visual viewport size + // has changed. + RefreshVisualViewportSize(); } if (gfxPlatform::AsyncPanZoomEnabled()) { UpdateDisplayPortMargins(); From cd794d1181e5caab1c57a8ce29ece1b43672c861 Mon Sep 17 00:00:00 2001 From: Brad Werth Date: Wed, 5 Dec 2018 17:16:36 +0000 Subject: [PATCH 02/34] Bug 1504659 Part 2: Add tests of viewport RDM scroll behavior, with and without touch simulation. r=gl Differential Revision: https://phabricator.services.mozilla.com/D13174 --HG-- extra : moz-landing-system : lando --- .../responsive.html/test/browser/browser.ini | 1 + .../test/browser/browser_scroll.js | 75 +++++++++++++++++++ .../responsive.html/test/browser/head.js | 11 +++ .../test/unit/test_resize_viewport.js | 10 ++- 4 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 devtools/client/responsive.html/test/browser/browser_scroll.js diff --git a/devtools/client/responsive.html/test/browser/browser.ini b/devtools/client/responsive.html/test/browser/browser.ini index 2af02083e196..4339e292d775 100644 --- a/devtools/client/responsive.html/test/browser/browser.ini +++ b/devtools/client/responsive.html/test/browser/browser.ini @@ -52,6 +52,7 @@ tags = devtools geolocation skip-if = true # Bug 1413765 [browser_preloaded_newtab.js] [browser_screenshot_button.js] +[browser_scroll.js] [browser_state_restore.js] [browser_tab_close.js] [browser_tab_remoteness_change.js] diff --git a/devtools/client/responsive.html/test/browser/browser_scroll.js b/devtools/client/responsive.html/test/browser/browser_scroll.js new file mode 100644 index 000000000000..a33c5b0da6fb --- /dev/null +++ b/devtools/client/responsive.html/test/browser/browser_scroll.js @@ -0,0 +1,75 @@ +/* Any copyright is dedicated to the Public Domain. +http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +/** + * This test is checking that keyboard scrolling of content in RDM + * behaves correctly, both with and without touch simulation enabled. + */ + +const TEST_URL = "data:text/html;charset=utf-8," + + "
"; + +addRDMTask(TEST_URL, async function({ ui, manager }) { + info("Turning off keyboard APZ for this test."); + await SpecialPowers.pushPrefEnv({ + set: [["apz.keyboard.enabled", false]], + }); + + await setViewportSize(ui, manager, 100, 100); + const browser = ui.getViewportBrowser(); + + info("Setting focus on the browser."); + browser.focus(); + + info("Testing scroll behavior with touch simulation OFF."); + await testScrollingOfContent(ui); + + // Run the tests again with touch simulation on. + const reloadNeeded = await ui.updateTouchSimulation(true); + if (reloadNeeded) { + info("Reload is needed -- waiting for it."); + const reload = waitForViewportLoad(ui); + browser.reload(); + await reload; + + await ContentTask.spawn(browser, null, () => { + content.scrollTo(0, 0); + }); + } + + info("Testing scroll behavior with touch simulation ON."); + await testScrollingOfContent(ui); +}); + +async function testScrollingOfContent(ui) { + let scroll; + + info("Checking initial scroll conditions."); + const viewportScroll = await getViewportScroll(ui); + is(viewportScroll.x, 0, "Content should load with scrollX 0."); + is(viewportScroll.y, 0, "Content should load with scrollY 0."); + + /** + * Here we're going to send off some arrow key events to trigger scrolling. + * What we would like to be able to do is to await the scroll event and then + * check the scroll position to confirm the amount of scrolling that has + * happened. Unfortunately, APZ makes the scrolling happen asynchronously on + * the compositor thread, and it's very difficult to await the end state of + * the APZ animation -- see the tests in /gfx/layers/apz/test/mochitest for + * an example. For our purposes, it's sufficient to test that the scroll + * event is fired at all, and not worry about the amount of scrolling that + * has occurred at the time of the event. If the key events don't trigger + * scrolling, then no event will be fired and the test will time out. + */ + scroll = waitForViewportScroll(ui); + EventUtils.synthesizeKey("KEY_ArrowDown"); + await scroll; + info("Scroll event was fired after arrow key down."); + + scroll = waitForViewportScroll(ui); + EventUtils.synthesizeKey("KEY_ArrowRight"); + await scroll; + info("Scroll event was fired after arrow key right."); +} diff --git a/devtools/client/responsive.html/test/browser/head.js b/devtools/client/responsive.html/test/browser/head.js index a4fdff613e33..b9f7156e355d 100644 --- a/devtools/client/responsive.html/test/browser/head.js +++ b/devtools/client/responsive.html/test/browser/head.js @@ -332,6 +332,13 @@ function getContentSize(ui) { })); } +function getViewportScroll(ui) { + return spawnViewportTask(ui, {}, () => ({ + x: content.scrollX, + y: content.scrollY, + })); +} + async function waitForPageShow(browser) { const tab = gBrowser.getTabForBrowser(browser); const ui = ResponsiveUIManager.getResponsiveUIForTab(tab); @@ -349,6 +356,10 @@ function waitForViewportLoad(ui) { return BrowserTestUtils.waitForContentEvent(ui.getViewportBrowser(), "load", true); } +function waitForViewportScroll(ui) { + return BrowserTestUtils.waitForContentEvent(ui.getViewportBrowser(), "scroll", true); +} + function load(browser, url) { const loaded = BrowserTestUtils.browserLoaded(browser, false, url); BrowserTestUtils.loadURI(browser, url); diff --git a/devtools/client/responsive.html/test/unit/test_resize_viewport.js b/devtools/client/responsive.html/test/unit/test_resize_viewport.js index d983f3a0c09f..e6503f7b4825 100644 --- a/devtools/client/responsive.html/test/unit/test_resize_viewport.js +++ b/devtools/client/responsive.html/test/unit/test_resize_viewport.js @@ -7,6 +7,7 @@ const { addViewport, resizeViewport } = require("devtools/client/responsive.html/actions/viewports"); +const { toggleTouchSimulation } = require("devtools/client/responsive.html/actions/ui"); add_task(async function() { const store = Store(); @@ -15,7 +16,14 @@ add_task(async function() { dispatch(addViewport()); dispatch(resizeViewport(0, 500, 500)); - const viewport = getState().viewports[0]; + let viewport = getState().viewports[0]; equal(viewport.width, 500, "Resized width of 500"); equal(viewport.height, 500, "Resized height of 500"); + + dispatch(toggleTouchSimulation(true)); + dispatch(resizeViewport(0, 400, 400)); + + viewport = getState().viewports[0]; + equal(viewport.width, 400, "Resized width of 400 (with touch simulation on)"); + equal(viewport.height, 400, "Resized height of 400 (with touch simulation on)"); }); From e3c6d70315dd3d80100f36c43947acd131d2911a Mon Sep 17 00:00:00 2001 From: Dave Townsend Date: Wed, 5 Dec 2018 19:03:36 +0000 Subject: [PATCH 03/34] Bug 1511465: Fix race condition in browser_oversized.js. r=mak Seems that when run as the first test in a test run there is a race for whether the favicon for the initial tab has already been seen or not. Rarely we fail the race and end up seeing a successful favicon load. This makes us ignore any favicons other than the one we're interested in. Differential Revision: https://phabricator.services.mozilla.com/D13838 --HG-- extra : moz-landing-system : lando --- browser/base/content/test/favicons/browser_oversized.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/base/content/test/favicons/browser_oversized.js b/browser/base/content/test/favicons/browser_oversized.js index 5262eee73302..855e09f5dab0 100644 --- a/browser/base/content/test/favicons/browser_oversized.js +++ b/browser/base/content/test/favicons/browser_oversized.js @@ -5,7 +5,7 @@ const ROOT = "http://mochi.test:8888/browser/browser/base/content/test/favicons/ add_task(async () => { await BrowserTestUtils.withNewTab({ gBrowser, url: "about:blank" }, async (browser) => { - let faviconPromise = waitForFaviconMessage(); + let faviconPromise = waitForFaviconMessage(true, `${ROOT}large.png`); BrowserTestUtils.loadURI(browser, ROOT + "large_favicon.html"); await BrowserTestUtils.browserLoaded(browser); From e5975f34baab669828a51e96d07e869567a331e6 Mon Sep 17 00:00:00 2001 From: Eitan Isaacson Date: Wed, 5 Dec 2018 19:06:40 +0000 Subject: [PATCH 04/34] Bug 1511398 - Check if the retrieved accessible child is actually valid when filling the bundle's data, r=yzen Differential Revision: https://phabricator.services.mozilla.com/D13611 --HG-- extra : moz-landing-system : lando --- accessible/android/SessionAccessibility.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/accessible/android/SessionAccessibility.cpp b/accessible/android/SessionAccessibility.cpp index 75f580a8150f..a45b6b68dfa5 100644 --- a/accessible/android/SessionAccessibility.cpp +++ b/accessible/android/SessionAccessibility.cpp @@ -3,6 +3,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include "Accessible-inl.h" #include "SessionAccessibility.h" #include "AndroidUiThread.h" #include "DocAccessibleParent.h" @@ -371,9 +372,15 @@ void SessionAccessibility::UpdateCachedBounds( auto infos = jni::ObjectArray::New(aAccessibles.Length()); for (size_t i = 0; i < aAccessibles.Length(); i++) { AccessibleWrap* acc = aAccessibles.ElementAt(i); + if (!acc || acc->IsDefunct()) { + MOZ_ASSERT_UNREACHABLE("Updated accessible is gone."); + continue; + } + if (aData.Length() == aAccessibles.Length()) { const BatchData& data = aData.ElementAt(i); - auto bundle = acc->ToSmallBundle(data.State(), data.Bounds(), data.ActionCount()); + auto bundle = + acc->ToSmallBundle(data.State(), data.Bounds(), data.ActionCount()); infos->SetElement(i, bundle); } else { infos->SetElement(i, acc->ToSmallBundle()); From a88fd1acef78fcbeb83ba7af92733dab9ef11c9e Mon Sep 17 00:00:00 2001 From: Botond Ballo Date: Wed, 5 Dec 2018 16:29:18 +0000 Subject: [PATCH 05/34] Bug 1511137 - Track more accurately when the main thread originates a resolution change. r=kats The tracking is done using nsAtom origins, similarly to how updates to the scroll offset are tracked. Currently, APZ still uses some heuristics to deduce that the main thread originated a resolution change in some cases, but the intention is to try to remove those and rely only on this mechanism in the future. Differential Revision: https://phabricator.services.mozilla.com/D13741 --HG-- extra : moz-landing-system : lando --- dom/base/nsDOMWindowUtils.cpp | 2 +- dom/base/nsDocument.cpp | 5 +++-- gfx/layers/FrameMetrics.h | 9 +++++++++ gfx/layers/apz/src/AsyncPanZoomController.cpp | 4 +++- gfx/layers/apz/util/APZCCallbackHelper.cpp | 2 +- gfx/layers/ipc/LayersMessageUtils.h | 3 +++ layout/base/MobileViewportManager.cpp | 2 +- layout/base/PresShell.cpp | 7 ++++++- layout/base/PresShell.h | 20 +++++++++++++++---- layout/base/nsIPresShell.h | 14 ++++++++++++- layout/base/nsLayoutUtils.cpp | 7 +++++++ layout/generic/nsGfxScrollFrame.cpp | 3 ++- 12 files changed, 65 insertions(+), 13 deletions(-) diff --git a/dom/base/nsDOMWindowUtils.cpp b/dom/base/nsDOMWindowUtils.cpp index 3a2e38bb90d9..b8fface5826b 100644 --- a/dom/base/nsDOMWindowUtils.cpp +++ b/dom/base/nsDOMWindowUtils.cpp @@ -560,7 +560,7 @@ nsDOMWindowUtils::SetResolutionAndScaleTo(float aResolution) { return NS_ERROR_FAILURE; } - presShell->SetResolutionAndScaleTo(aResolution); + presShell->SetResolutionAndScaleTo(aResolution, nsGkAtoms::other); return NS_OK; } diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 25d4b4d9d14d..d72cd48842b7 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -10179,7 +10179,7 @@ void nsIDocument::CleanupFullscreenState() { // Restore the zoom level that was in place prior to entering fullscreen. if (nsIPresShell* shell = GetShell()) { if (shell->GetMobileViewportManager()) { - shell->SetResolutionAndScaleTo(mSavedResolution); + shell->SetResolutionAndScaleTo(mSavedResolution, nsGkAtoms::restore); } } @@ -10576,7 +10576,8 @@ bool nsIDocument::ApplyFullscreen(UniquePtr aRequest) { shell->GetMobileViewportManager()) { // Save the previous resolution so it can be restored. child->mSavedResolution = shell->GetResolution(); - shell->SetResolutionAndScaleTo(manager->ComputeIntrinsicResolution()); + shell->SetResolutionAndScaleTo(manager->ComputeIntrinsicResolution(), + nsGkAtoms::other); } } diff --git a/gfx/layers/FrameMetrics.h b/gfx/layers/FrameMetrics.h index 5d6ee1ca1a28..bb757c140dd8 100644 --- a/gfx/layers/FrameMetrics.h +++ b/gfx/layers/FrameMetrics.h @@ -812,6 +812,7 @@ struct ScrollMetadata { mIsAutoDirRootContentRTL(false), mUsesContainerScrolling(false), mForceDisableApz(false), + mResolutionUpdated(false), mOverscrollBehavior() {} bool operator==(const ScrollMetadata& aOther) const { @@ -827,6 +828,7 @@ struct ScrollMetadata { mIsAutoDirRootContentRTL == aOther.mIsAutoDirRootContentRTL && mUsesContainerScrolling == aOther.mUsesContainerScrolling && mForceDisableApz == aOther.mForceDisableApz && + mResolutionUpdated == aOther.mResolutionUpdated && mDisregardedDirection == aOther.mDisregardedDirection && mOverscrollBehavior == aOther.mOverscrollBehavior; } @@ -907,6 +909,8 @@ struct ScrollMetadata { mForceDisableApz = aForceDisable; } bool IsApzForceDisabled() const { return mForceDisableApz; } + void SetResolutionUpdated(bool aUpdated) { mResolutionUpdated = aUpdated; } + bool IsResolutionUpdated() const { return mResolutionUpdated; } // For more details about the concept of a disregarded direction, refer to the // code which defines mDisregardedDirection. @@ -983,6 +987,11 @@ struct ScrollMetadata { // scrollframe. bool mForceDisableApz : 1; + // Whether the pres shell resolution stored in mMetrics reflects a change + // originated by the main thread. Plays a similar role for the resolution as + // FrameMetrics::mScrollUpdateType) does for the scroll offset. + bool mResolutionUpdated : 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, diff --git a/gfx/layers/apz/src/AsyncPanZoomController.cpp b/gfx/layers/apz/src/AsyncPanZoomController.cpp index 111d2ced6703..4d707ef6488f 100644 --- a/gfx/layers/apz/src/AsyncPanZoomController.cpp +++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp @@ -4418,11 +4418,13 @@ void AsyncPanZoomController::NotifyLayersUpdated( // in some things into our local Metrics() because these things are // determined by Gecko and our copy in Metrics() may be stale. + // TODO: Rely entirely on |aScrollMetadata.IsResolutionUpdated()| to + // determine which branch to take, and drop the other conditions. if (FuzzyEqualsAdditive(Metrics().GetCompositionBounds().Width(), aLayerMetrics.GetCompositionBounds().Width()) && Metrics().GetDevPixelsPerCSSPixel() == aLayerMetrics.GetDevPixelsPerCSSPixel() && - !viewportUpdated) { + !viewportUpdated && !aScrollMetadata.IsResolutionUpdated()) { // Any change to the pres shell resolution was requested by APZ and is // already included in our zoom; however, other components of the // cumulative resolution (a parent document's pres-shell resolution, or diff --git a/gfx/layers/apz/util/APZCCallbackHelper.cpp b/gfx/layers/apz/util/APZCCallbackHelper.cpp index 37c45a003eac..09046c94a70e 100644 --- a/gfx/layers/apz/util/APZCCallbackHelper.cpp +++ b/gfx/layers/apz/util/APZCCallbackHelper.cpp @@ -317,7 +317,7 @@ void APZCCallbackHelper::UpdateRootFrame(const RepaintRequest& aRequest) { // last paint. presShellResolution = aRequest.GetPresShellResolution() * aRequest.GetAsyncZoom().scale; - shell->SetResolutionAndScaleTo(presShellResolution); + shell->SetResolutionAndScaleTo(presShellResolution, nsGkAtoms::apz); } // Do this as late as possible since scrolling can flush layout. It also diff --git a/gfx/layers/ipc/LayersMessageUtils.h b/gfx/layers/ipc/LayersMessageUtils.h index 0aaa63afb761..89ca16c582e8 100644 --- a/gfx/layers/ipc/LayersMessageUtils.h +++ b/gfx/layers/ipc/LayersMessageUtils.h @@ -339,6 +339,7 @@ struct ParamTraits WriteParam(aMsg, aParam.mIsAutoDirRootContentRTL); WriteParam(aMsg, aParam.mUsesContainerScrolling); WriteParam(aMsg, aParam.mForceDisableApz); + WriteParam(aMsg, aParam.mResolutionUpdated); WriteParam(aMsg, aParam.mDisregardedDirection); WriteParam(aMsg, aParam.mOverscrollBehavior); } @@ -373,6 +374,8 @@ struct ParamTraits ¶mType::SetUsesContainerScrolling) && ReadBoolForBitfield(aMsg, aIter, aResult, ¶mType::SetForceDisableApz) && + ReadBoolForBitfield(aMsg, aIter, aResult, + ¶mType::SetResolutionUpdated) && ReadParam(aMsg, aIter, &aResult->mDisregardedDirection) && ReadParam(aMsg, aIter, &aResult->mOverscrollBehavior)); } diff --git a/layout/base/MobileViewportManager.cpp b/layout/base/MobileViewportManager.cpp index 6c5b08bed1b6..e9b107af4ad8 100644 --- a/layout/base/MobileViewportManager.cpp +++ b/layout/base/MobileViewportManager.cpp @@ -329,7 +329,7 @@ void MobileViewportManager::UpdateResolution( if (newZoom) { LayoutDeviceToLayerScale resolution = ZoomToResolution(*newZoom, cssToDev); MVM_LOG("%p: setting resolution %f\n", this, resolution.scale); - mPresShell->SetResolutionAndScaleTo(resolution.scale); + mPresShell->SetResolutionAndScaleTo(resolution.scale, nsGkAtoms::other); MVM_LOG("%p: New zoom is %f\n", this, newZoom->scale); } diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index db6986eb9b1d..7f3713f1abc0 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -781,6 +781,7 @@ PresShell::PresShell() mHasReceivedPaintMessage(false), mIsLastKeyDownCanceled(false), mHasHandledUserInput(false), + mResolutionUpdated(false), mForceDispatchKeyPressEventsForNonPrintableKeys(false), mForceUseLegacyKeyCodeAndCharCodeValues(false), mInitializedWithKeyPressEventDispatchingBlacklist(false) { @@ -5106,7 +5107,8 @@ void PresShell::SetIgnoreViewportScrolling(bool aIgnore) { } nsresult PresShell::SetResolutionImpl(float aResolution, - bool aScaleToResolution) { + bool aScaleToResolution, + nsAtom* aOrigin) { if (!(aResolution > 0.0)) { return NS_ERROR_ILLEGAL_VALUE; } @@ -5121,6 +5123,9 @@ nsresult PresShell::SetResolutionImpl(float aResolution, if (mMobileViewportManager) { mMobileViewportManager->ResolutionUpdated(); } + if (aOrigin != nsGkAtoms::apz) { + mResolutionUpdated = true; + } return NS_OK; } diff --git a/layout/base/PresShell.h b/layout/base/PresShell.h index b8ef46eedd3e..30e9e48c32e7 100644 --- a/layout/base/PresShell.h +++ b/layout/base/PresShell.h @@ -195,12 +195,19 @@ class PresShell final : public nsIPresShell, void SetIgnoreViewportScrolling(bool aIgnore) override; nsresult SetResolution(float aResolution) override { - return SetResolutionImpl(aResolution, /* aScaleToResolution = */ false); + return SetResolutionImpl(aResolution, /* aScaleToResolution = */ false, + nsGkAtoms::other); } - nsresult SetResolutionAndScaleTo(float aResolution) override { - return SetResolutionImpl(aResolution, /* aScaleToResolution = */ true); + nsresult SetResolutionAndScaleTo(float aResolution, + nsAtom* aOrigin) override { + return SetResolutionImpl(aResolution, /* aScaleToResolution = */ true, + aOrigin); } bool ScaleToResolution() const override; + bool IsResolutionUpdated() const override { return mResolutionUpdated; } + void SetResolutionUpdated(bool aUpdated) override { + mResolutionUpdated = aUpdated; + } float GetCumulativeResolution() override; float GetCumulativeNonRootScaleResolution() override; void SetRestoreResolution(float aResolution, @@ -711,7 +718,8 @@ class PresShell final : public nsIPresShell, // that we last did an approximate frame visibility update. VisibleFrames mApproximatelyVisibleFrames; - nsresult SetResolutionImpl(float aResolution, bool aScaleToResolution); + nsresult SetResolutionImpl(float aResolution, bool aScaleToResolution, + nsAtom* aOrigin); nsIContent* GetOverrideClickTarget(WidgetGUIEvent* aEvent, nsIFrame* aFrame); #ifdef DEBUG @@ -828,6 +836,10 @@ class PresShell final : public nsIPresShell, // Whether we have ever handled a user input event bool mHasHandledUserInput : 1; + // Whether the most recent change to the pres shell resolution was + // originated by the main thread. + bool mResolutionUpdated : 1; + // Whether we should dispatch keypress events even for non-printable keys // for keeping backward compatibility. bool mForceDispatchKeyPressEventsForNonPrintableKeys : 1; diff --git a/layout/base/nsIPresShell.h b/layout/base/nsIPresShell.h index 054616ff6b1b..9ff5ec511821 100644 --- a/layout/base/nsIPresShell.h +++ b/layout/base/nsIPresShell.h @@ -1371,6 +1371,13 @@ class nsIPresShell : public nsStubDocumentObserver { float GetResolution() const { return mResolution.valueOr(1.0); } virtual float GetCumulativeResolution() = 0; + /** + * Accessors for a flag that tracks whether the most recent change to + * the pres shell's resolution was originated by the main thread. + */ + virtual bool IsResolutionUpdated() const = 0; + virtual void SetResolutionUpdated(bool aUpdated) = 0; + /** * Calculate the cumulative scale resolution from this document up to * but not including the root document. @@ -1385,8 +1392,13 @@ class nsIPresShell : public nsStubDocumentObserver { /** * Similar to SetResolution() but also increases the scale of the content * by the same amount. + * |aOrigin| specifies who originated the resolution change. For changes + * sent by APZ, pass nsGkAtoms::apz. For changes sent by the main thread, + * use pass nsGkAtoms::other or nsGkAtoms::restore (similar to the |aOrigin| + * parameter of nsIScrollableFrame::ScrollToCSSPixels()). */ - virtual nsresult SetResolutionAndScaleTo(float aResolution) = 0; + virtual nsresult SetResolutionAndScaleTo(float aResolution, + nsAtom* aOrigin) = 0; /** * Return whether we are scaling to the set resolution. diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index efdb409bb2a2..eebba00d35cb 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -8820,6 +8820,13 @@ static void MaybeReflowForInflationScreenSizeChange( } else { metrics.SetPresShellResolution(1.0f); } + + if (presShell->IsResolutionUpdated()) { + metadata.SetResolutionUpdated(true); + // We only need to tell APZ about the resolution update once. + presShell->SetResolutionUpdated(false); + } + // The cumulative resolution is the resolution at which the scroll frame's // content is actually rendered. It includes the pres shell resolutions of // all the pres shells from here up to the root, as well as any css-driven diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp index 058a1c7f01c6..3583fde04328 100644 --- a/layout/generic/nsGfxScrollFrame.cpp +++ b/layout/generic/nsGfxScrollFrame.cpp @@ -6145,7 +6145,8 @@ void ScrollFrameHelper::RestoreState(PresState* aState) { if (mIsRoot) { nsIPresShell* presShell = mOuter->PresShell(); if (aState->scaleToResolution()) { - presShell->SetResolutionAndScaleTo(aState->resolution()); + presShell->SetResolutionAndScaleTo(aState->resolution(), + nsGkAtoms::restore); } else { presShell->SetResolution(aState->resolution()); } From ec33fda472b002e8de0df9c1a64ac024e13402d1 Mon Sep 17 00:00:00 2001 From: Eitan Isaacson Date: Wed, 5 Dec 2018 19:14:54 +0000 Subject: [PATCH 06/34] Bug 1511759 - Null-check focus path cached accessible. r=yzen Differential Revision: https://phabricator.services.mozilla.com/D13729 --HG-- extra : moz-landing-system : lando --- accessible/android/DocAccessibleWrap.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/accessible/android/DocAccessibleWrap.cpp b/accessible/android/DocAccessibleWrap.cpp index 466e51a23384..a9009931528e 100644 --- a/accessible/android/DocAccessibleWrap.cpp +++ b/accessible/android/DocAccessibleWrap.cpp @@ -4,6 +4,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include "Accessible-inl.h" #include "DocAccessibleWrap.h" #include "nsIDocShell.h" #include "nsLayoutUtils.h" @@ -229,6 +230,11 @@ void DocAccessibleWrap::UpdateFocusPathBounds() { nsTArray boundsData(mFocusPath.Count()); for (auto iter = mFocusPath.Iter(); !iter.Done(); iter.Next()) { Accessible* accessible = iter.Data(); + if (!accessible || accessible->IsDefunct()) { + MOZ_ASSERT_UNREACHABLE("Focus path cached accessible is gone."); + continue; + } + auto uid = accessible->IsDoc() && accessible->AsDoc()->IPCDoc() ? 0 : reinterpret_cast(accessible->UniqueID()); From 4380b3f45bd10e4fff16fd6dd3515c973c61dfb7 Mon Sep 17 00:00:00 2001 From: Kim Moir Date: Wed, 5 Dec 2018 19:49:38 +0000 Subject: [PATCH 07/34] Bug 1510602 - update build documentation to mention artifact builds r=firefox-build-system-reviewers,gps Differential Revision: https://phabricator.services.mozilla.com/D13435 --HG-- extra : moz-landing-system : lando --- python/mozboot/mozboot/bootstrap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/mozboot/mozboot/bootstrap.py b/python/mozboot/mozboot/bootstrap.py index 2846b4c661ce..f5af5f8af4ef 100644 --- a/python/mozboot/mozboot/bootstrap.py +++ b/python/mozboot/mozboot/bootstrap.py @@ -40,7 +40,7 @@ APPLICATION_CHOICE = ''' Note on Artifact Mode: Artifact builds download prebuilt C++ components rather than building -them locally. +them locally. Artifact builds are faster! Artifact builds are recommended for people working on Firefox or Firefox for Android frontends. They are unsuitable for those working From a708b9e3d6eaee087fa67af020daa88156b0c67d Mon Sep 17 00:00:00 2001 From: Daniel Holbert Date: Wed, 5 Dec 2018 02:03:04 +0000 Subject: [PATCH 08/34] Bug 1512054 part 1: Add some needed "Inlines" includes to .cpp files in layout, to address some -Wundefined-inline build warnings in non-unified builds. r=TYLin I'm also adding the non-"inlines" version of the added "inlines" includes, too, since it's nice to have them included as a pair. (It's not strictly necessary, since in these cases we were already indirectly including the non-"inlines" header, but it seems like a good practice.) This patch fixes build warnings in non-unified builds for the following calls: - The call to IsColumnSpan() in layout/generic/ColumnSetWrapperFrame.cpp - The call to Type() in layout/style/MappedDeclarations.cpp - The call to IsInAnonymousSubtree() in layout/svg/SVGObserverUtils.cpp Differential Revision: https://phabricator.services.mozilla.com/D13743 --HG-- extra : moz-landing-system : lando --- layout/generic/ColumnSetWrapperFrame.cpp | 2 ++ layout/style/MappedDeclarations.cpp | 1 + layout/svg/SVGObserverUtils.cpp | 2 ++ 3 files changed, 5 insertions(+) diff --git a/layout/generic/ColumnSetWrapperFrame.cpp b/layout/generic/ColumnSetWrapperFrame.cpp index 364d0896b2b0..eecbcd13ff81 100644 --- a/layout/generic/ColumnSetWrapperFrame.cpp +++ b/layout/generic/ColumnSetWrapperFrame.cpp @@ -7,6 +7,8 @@ #include "ColumnSetWrapperFrame.h" #include "nsContentUtils.h" +#include "nsIFrame.h" +#include "nsIFrameInlines.h" using namespace mozilla; diff --git a/layout/style/MappedDeclarations.cpp b/layout/style/MappedDeclarations.cpp index c7eb25605938..74e192ad149f 100644 --- a/layout/style/MappedDeclarations.cpp +++ b/layout/style/MappedDeclarations.cpp @@ -7,6 +7,7 @@ #include "MappedDeclarations.h" #include "nsAttrValue.h" +#include "nsAttrValueInlines.h" #include "nsIDocument.h" #include "nsPresContext.h" diff --git a/layout/svg/SVGObserverUtils.cpp b/layout/svg/SVGObserverUtils.cpp index c3750168152e..e87f3e01861b 100644 --- a/layout/svg/SVGObserverUtils.cpp +++ b/layout/svg/SVGObserverUtils.cpp @@ -11,6 +11,8 @@ #include "mozilla/dom/CanvasRenderingContext2D.h" #include "mozilla/RestyleManager.h" #include "nsCSSFrameConstructor.h" +#include "nsIContent.h" +#include "nsIContentInlines.h" #include "nsISupportsImpl.h" #include "nsSVGClipPathFrame.h" #include "nsSVGMarkerFrame.h" From e590a79e704c117f35cc96317be6b1407b2ebbd0 Mon Sep 17 00:00:00 2001 From: Daniel Holbert Date: Wed, 5 Dec 2018 15:20:58 +0000 Subject: [PATCH 09/34] Bug 1512054 part 2: Fix alphabetical sorting of SVGObserverUtils.cpp's includes. r=TYLin (Also, include the full export path when including 'ImageLoader.h', for consistency with how we include it everywhere else outside of its own directory.) Depends on D13743 Differential Revision: https://phabricator.services.mozilla.com/D13773 --HG-- extra : moz-landing-system : lando --- layout/svg/SVGObserverUtils.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/layout/svg/SVGObserverUtils.cpp b/layout/svg/SVGObserverUtils.cpp index e87f3e01861b..c35529479184 100644 --- a/layout/svg/SVGObserverUtils.cpp +++ b/layout/svg/SVGObserverUtils.cpp @@ -8,24 +8,24 @@ #include "SVGObserverUtils.h" // Keep others in (case-insensitive) order: +#include "mozilla/css/ImageLoader.h" #include "mozilla/dom/CanvasRenderingContext2D.h" +#include "mozilla/net/ReferrerPolicy.h" #include "mozilla/RestyleManager.h" #include "nsCSSFrameConstructor.h" +#include "nsCycleCollectionParticipant.h" #include "nsIContent.h" #include "nsIContentInlines.h" +#include "nsIReflowCallback.h" #include "nsISupportsImpl.h" #include "nsSVGClipPathFrame.h" -#include "nsSVGMarkerFrame.h" -#include "nsSVGPaintServerFrame.h" #include "nsSVGFilterFrame.h" +#include "nsSVGMarkerFrame.h" #include "nsSVGMaskFrame.h" -#include "nsIReflowCallback.h" -#include "nsCycleCollectionParticipant.h" +#include "nsSVGPaintServerFrame.h" #include "SVGGeometryElement.h" #include "SVGTextPathElement.h" #include "SVGUseElement.h" -#include "ImageLoader.h" -#include "mozilla/net/ReferrerPolicy.h" using namespace mozilla; using namespace mozilla::dom; From 85e0545e3fc34452b96486debfa4911808b72f87 Mon Sep 17 00:00:00 2001 From: Michael Ratcliffe Date: Wed, 5 Dec 2018 19:54:56 +0000 Subject: [PATCH 10/34] Bug 1509460 - Disable / re-enable and delete/bring back should not bring the flexbox highlighter back r=gl Differential Revision: https://phabricator.services.mozilla.com/D12778 --HG-- extra : moz-landing-system : lando --- .../inspector/shared/highlighters-overlay.js | 31 +++++++++++++++++++ .../server/actors/highlighters/flexbox.js | 12 ------- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/devtools/client/inspector/shared/highlighters-overlay.js b/devtools/client/inspector/shared/highlighters-overlay.js index 4c02fd88ed99..bbf72b8287ca 100644 --- a/devtools/client/inspector/shared/highlighters-overlay.js +++ b/devtools/client/inspector/shared/highlighters-overlay.js @@ -70,6 +70,7 @@ class HighlightersOverlay { this.shapesHighlighterShown = null; this.onClick = this.onClick.bind(this); + this.onDisplayChange = this.onDisplayChange.bind(this); this.onMarkupMutation = this.onMarkupMutation.bind(this); this.onMouseMove = this.onMouseMove.bind(this); this.onMouseOut = this.onMouseOut.bind(this); @@ -88,6 +89,7 @@ class HighlightersOverlay { // Add inspector events, not specific to a given view. this.inspector.on("markupmutation", this.onMarkupMutation); + this.inspector.walker.on("display-change", this.onDisplayChange); this.inspector.target.on("will-navigate", this.onWillNavigate); EventEmitter.decorate(this); @@ -1038,6 +1040,35 @@ class HighlightersOverlay { } } + /** + * Handler for "display-change" events from the walker. Hides the flexbox or + * grid highlighter if their respective node is no longer a flex container or + * grid container. + * + * @param {Array} nodes + * An array of nodeFronts + */ + async onDisplayChange(nodes) { + for (const node of nodes) { + const display = node.displayType; + + // Hide the flexbox highlighter if the node is no longer a flexbox + // container. + if (display !== "flex" && display !== "inline-flex" && + node == this.flexboxHighlighterShown) { + await this.hideFlexboxHighlighter(node); + return; + } + + // Hide the grid highlighter if the node is no longer a grid container. + if (display !== "grid" && display !== "inline-grid" && + this.gridHighlighters.has(node)) { + await this.hideGridHighlighter(node); + return; + } + } + } + onMouseMove(event) { // Bail out if the target is the same as for the last mousemove. if (event.target === this._lastHovered) { diff --git a/devtools/server/actors/highlighters/flexbox.js b/devtools/server/actors/highlighters/flexbox.js index 77ae1c0486b1..8f6691da5418 100644 --- a/devtools/server/actors/highlighters/flexbox.js +++ b/devtools/server/actors/highlighters/flexbox.js @@ -757,18 +757,6 @@ class FlexboxHighlighter extends AutoRefreshHighlighter { } _update() { - // If this.currentNode is not a flex container we have nothing to highlight. - // We can't simply use getAsFlexContainer() here because this fails for - // text fields. This will be removed by https://bugzil.la/1509460. - if (!this.computedStyle) { - this.computedStyle = getComputedStyle(this.currentNode); - } - - if (this.computedStyle.display !== "flex" && - this.computedStyle.display !== "inline-flex") { - return false; - } - setIgnoreLayoutChanges(true); const root = this.getElement("root"); From 0211b2854b5470fcdedb6dd0d11c906f42c4a6d0 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Wed, 5 Dec 2018 19:48:01 +0000 Subject: [PATCH 11/34] Bug 1512276 - Remove the waitForSecurityChange() defined in browser_trackingUI_animation_2.js r=johannh Differential Revision: https://phabricator.services.mozilla.com/D13850 --HG-- extra : moz-landing-system : lando --- .../browser_trackingUI_animation_2.js | 35 +++++-------------- browser/base/content/test/trackingUI/head.js | 9 +++-- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/browser/base/content/test/trackingUI/browser_trackingUI_animation_2.js b/browser/base/content/test/trackingUI/browser_trackingUI_animation_2.js index 54fff9180396..3853da982687 100644 --- a/browser/base/content/test/trackingUI/browser_trackingUI_animation_2.js +++ b/browser/base/content/test/trackingUI/browser_trackingUI_animation_2.js @@ -22,23 +22,6 @@ registerCleanupFunction(function() { Services.prefs.clearUserPref(ContentBlocking.prefIntroCount); }); -function waitForSecurityChange(tabbrowser, numChanges = 1) { - return new Promise(resolve => { - let n = 0; - let listener = { - onSecurityChange() { - n = n + 1; - info("Received onSecurityChange event " + n + " of " + numChanges); - if (n >= numChanges) { - tabbrowser.removeProgressListener(listener); - resolve(n); - } - }, - }; - tabbrowser.addProgressListener(listener); - }); -} - async function testTrackingProtectionAnimation(tabbrowser) { info("Load a test page not containing tracking elements"); let benignTab = await BrowserTestUtils.openNewForegroundTab(tabbrowser, BENIGN_PAGE); @@ -62,7 +45,7 @@ async function testTrackingProtectionAnimation(tabbrowser) { await BrowserTestUtils.waitForEvent(ContentBlocking.animatedIcon, "animationend"); info("Switch from tracking cookie -> benign tab"); - let securityChanged = waitForSecurityChange(tabbrowser); + let securityChanged = waitForSecurityChange(1, tabbrowser.ownerGlobal); tabbrowser.selectedTab = benignTab; await securityChanged; @@ -70,7 +53,7 @@ async function testTrackingProtectionAnimation(tabbrowser) { ok(!ContentBlocking.iconBox.hasAttribute("animate"), "iconBox not animating"); info("Switch from benign -> tracking tab"); - securityChanged = waitForSecurityChange(tabbrowser); + securityChanged = waitForSecurityChange(1, tabbrowser.ownerGlobal); tabbrowser.selectedTab = trackingTab; await securityChanged; @@ -78,7 +61,7 @@ async function testTrackingProtectionAnimation(tabbrowser) { ok(!ContentBlocking.iconBox.hasAttribute("animate"), "iconBox not animating"); info("Switch from tracking -> tracking cookies tab"); - securityChanged = waitForSecurityChange(tabbrowser); + securityChanged = waitForSecurityChange(1, tabbrowser.ownerGlobal); tabbrowser.selectedTab = trackingCookiesTab; await securityChanged; @@ -86,7 +69,7 @@ async function testTrackingProtectionAnimation(tabbrowser) { ok(!ContentBlocking.iconBox.hasAttribute("animate"), "iconBox not animating"); info("Reload tracking cookies tab"); - securityChanged = waitForSecurityChange(tabbrowser, 3); + securityChanged = waitForSecurityChange(3, tabbrowser.ownerGlobal); tabbrowser.reload(); await securityChanged; @@ -95,7 +78,7 @@ async function testTrackingProtectionAnimation(tabbrowser) { await BrowserTestUtils.waitForEvent(ContentBlocking.animatedIcon, "animationend"); info("Reload tracking tab"); - securityChanged = waitForSecurityChange(tabbrowser, 4); + securityChanged = waitForSecurityChange(4, tabbrowser.ownerGlobal); tabbrowser.selectedTab = trackingTab; tabbrowser.reload(); await securityChanged; @@ -105,7 +88,7 @@ async function testTrackingProtectionAnimation(tabbrowser) { await BrowserTestUtils.waitForEvent(ContentBlocking.animatedIcon, "animationend"); info("Inject tracking cookie inside tracking tab"); - securityChanged = waitForSecurityChange(tabbrowser); + securityChanged = waitForSecurityChange(1, tabbrowser.ownerGlobal); let timeoutPromise = new Promise(resolve => setTimeout(resolve, 500)); await ContentTask.spawn(tabbrowser.selectedBrowser, {}, function() { @@ -118,7 +101,7 @@ async function testTrackingProtectionAnimation(tabbrowser) { ok(!ContentBlocking.iconBox.hasAttribute("animate"), "iconBox not animating"); info("Inject tracking element inside tracking tab"); - securityChanged = waitForSecurityChange(tabbrowser); + securityChanged = waitForSecurityChange(1, tabbrowser.ownerGlobal); timeoutPromise = new Promise(resolve => setTimeout(resolve, 500)); await ContentTask.spawn(tabbrowser.selectedBrowser, {}, function() { @@ -133,7 +116,7 @@ async function testTrackingProtectionAnimation(tabbrowser) { tabbrowser.selectedTab = trackingCookiesTab; info("Inject tracking cookie inside tracking cookies tab"); - securityChanged = waitForSecurityChange(tabbrowser); + securityChanged = waitForSecurityChange(1, tabbrowser.ownerGlobal); timeoutPromise = new Promise(resolve => setTimeout(resolve, 500)); await ContentTask.spawn(tabbrowser.selectedBrowser, {}, function() { @@ -146,7 +129,7 @@ async function testTrackingProtectionAnimation(tabbrowser) { ok(!ContentBlocking.iconBox.hasAttribute("animate"), "iconBox not animating"); info("Inject tracking element inside tracking cookies tab"); - securityChanged = waitForSecurityChange(tabbrowser); + securityChanged = waitForSecurityChange(1, tabbrowser.ownerGlobal); timeoutPromise = new Promise(resolve => setTimeout(resolve, 500)); await ContentTask.spawn(tabbrowser.selectedBrowser, {}, function() { diff --git a/browser/base/content/test/trackingUI/head.js b/browser/base/content/test/trackingUI/head.js index d7fe859d77a1..fd2be3f8ab78 100644 --- a/browser/base/content/test/trackingUI/head.js +++ b/browser/base/content/test/trackingUI/head.js @@ -40,7 +40,10 @@ function openIdentityPopup() { return viewShown; } -function waitForSecurityChange(numChanges = 1) { +function waitForSecurityChange(numChanges = 1, win = null) { + if (!win) { + win = window; + } return new Promise(resolve => { let n = 0; let listener = { @@ -48,11 +51,11 @@ function waitForSecurityChange(numChanges = 1) { n = n + 1; info("Received onSecurityChange event " + n + " of " + numChanges); if (n >= numChanges) { - gBrowser.removeProgressListener(listener); + win.gBrowser.removeProgressListener(listener); resolve(n); } }, }; - gBrowser.addProgressListener(listener); + win.gBrowser.addProgressListener(listener); }); } From 7c3b31a25d0b278f9e446faf4a6099b2b9a1a16e Mon Sep 17 00:00:00 2001 From: Daniel Holbert Date: Wed, 5 Dec 2018 18:55:59 +0000 Subject: [PATCH 12/34] Bug 1512112: Remove redundant includes from source files in layout. r=TYLin All of the removed includes are redundant (i.e. they're #included elsewhere in the same file). In most cases, I'm removing the second (redundant) copy of the #include, except when that copy makes more sense (i.e. if it's in better sorted order, or if it's paired alongside a closely-associated header while the earlier copy is not). Here's the script that I used to generate candidates here -- I ran this in every subdirectory of layout, on my linux machine (warning, this writes two files to your /tmp directory): for FILE in *.h *.cpp; do nonunique=$(grep \#include $FILE | grep -v List\.h | cut -f2 -d'"' | cut -f2- -d'/'| cut -f2- -d'/' | sort | wc -l) unique=$( grep \#include $FILE | grep -v List\.h | cut -f2 -d'"' | cut -f2- -d'/'| cut -f2- -d'/' | sort | uniq | wc -l) if [[ "$unique" != "$nonunique" ]]; then echo "$FILE: $nonunique / $unique" grep \#include $FILE | cut -f2 -d'"' | grep -v List\.h | cut -f2- -d'/'| cut -f2- -d'/' | sort > /tmp/nonunique.txt grep \#include $FILE | cut -f2 -d'"' | grep -v List\.h | cut -f2- -d'/'| cut -f2- -d'/' | sort | uniq > /tmp/unique.txt diff /tmp/nonunique.txt /tmp/unique.txt echo fi done Depends on D13773 Differential Revision: https://phabricator.services.mozilla.com/D13774 --HG-- extra : moz-landing-system : lando --- layout/base/PresShell.cpp | 1 - layout/base/RestyleManager.cpp | 2 -- layout/base/nsCSSFrameConstructor.cpp | 1 - layout/base/nsDocumentViewer.cpp | 1 - layout/base/nsLayoutUtils.cpp | 2 -- layout/base/nsLayoutUtils.h | 1 - layout/base/nsRefreshDriver.h | 1 - layout/forms/nsDateTimeControlFrame.cpp | 1 - layout/generic/nsBlockFrame.cpp | 1 - layout/generic/nsFrame.cpp | 4 ---- layout/generic/nsFrameSelection.cpp | 1 - layout/generic/nsImageFrame.cpp | 1 - layout/generic/nsPageFrame.cpp | 1 - layout/generic/nsTextFrame.cpp | 1 - layout/inspector/inDeepTreeWalker.cpp | 1 - layout/painting/nsCSSRendering.cpp | 1 - layout/painting/nsCSSRenderingBorders.cpp | 1 - layout/painting/nsDisplayList.h | 2 -- layout/painting/nsImageRenderer.cpp | 1 - layout/printing/nsPrintJob.cpp | 2 -- layout/style/Loader.cpp | 2 -- layout/style/StyleAnimationValue.cpp | 1 - layout/style/nsCSSProps.cpp | 1 - layout/style/nsLayoutStylesheetCache.cpp | 1 - layout/style/nsStyleStruct.cpp | 1 - layout/svg/SVGContextPaint.cpp | 1 - layout/svg/nsFilterInstance.cpp | 1 - layout/tables/nsCellMap.h | 1 - 28 files changed, 36 deletions(-) diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index 7f3713f1abc0..1145ca67382b 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -12,7 +12,6 @@ #include "mozilla/ArrayUtils.h" #include "mozilla/Attributes.h" #include "mozilla/AutoRestore.h" -#include "mozilla/StyleSheetInlines.h" #include "mozilla/EventDispatcher.h" #include "mozilla/EventStateManager.h" #include "mozilla/EventStates.h" diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp index 94fb3f478648..76bb67c45b82 100644 --- a/layout/base/RestyleManager.cpp +++ b/layout/base/RestyleManager.cpp @@ -23,7 +23,6 @@ #include "mozilla/dom/HTMLBodyElement.h" #include "Layers.h" -#include "LayerAnimationInfo.h" // For LayerAnimationInfo::sRecords #include "nsAnimationManager.h" #include "nsBlockFrame.h" #include "nsBulletFrame.h" @@ -44,7 +43,6 @@ #include "StickyScrollContainer.h" #include "mozilla/EffectSet.h" #include "mozilla/IntegerRange.h" -#include "mozilla/ViewportFrame.h" #include "SVGObserverUtils.h" #include "SVGTextFrame.h" #include "ActiveLayerTracker.h" diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index 8bf0c01731e9..71e8b2e58c31 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -104,7 +104,6 @@ #include "nsBackdropFrame.h" #include "nsTransitionManager.h" #include "DetailsFrame.h" -#include "nsStyleConsts.h" #ifdef MOZ_XUL #include "nsIPopupContainer.h" diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index 8f14dc3d1f08..efe606a1ffc2 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/layout/base/nsDocumentViewer.cpp @@ -49,7 +49,6 @@ #include "nsIPageSequenceFrame.h" #include "nsNetUtil.h" #include "nsIContentViewerEdit.h" -#include "mozilla/StyleSheetInlines.h" #include "mozilla/css/Loader.h" #include "nsIInterfaceRequestor.h" #include "nsIInterfaceRequestorUtils.h" diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index eebba00d35cb..10af5c03dab9 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -51,7 +51,6 @@ #include "ImageRegion.h" #include "gfxRect.h" #include "gfxContext.h" -#include "gfxContext.h" #include "nsIInterfaceRequestorUtils.h" #include "nsCSSRendering.h" #include "nsTextFragment.h" @@ -117,7 +116,6 @@ #include "mozilla/layers/APZUtils.h" // for apz::CalculatePendingDisplayPort #include "mozilla/layers/CompositorBridgeChild.h" #include "mozilla/Telemetry.h" -#include "mozilla/EventDispatcher.h" #include "mozilla/StyleAnimationValue.h" #include "mozilla/ServoStyleSet.h" #include "mozilla/WheelHandlingHelper.h" // for WheelHandlingUtils diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h index d323f71695ac..59f5f0084320 100644 --- a/layout/base/nsLayoutUtils.h +++ b/layout/base/nsLayoutUtils.h @@ -32,7 +32,6 @@ #include "mozilla/ReflowOutput.h" #include "ImageContainer.h" #include "gfx2DGlue.h" -#include "nsStyleConsts.h" #include "SVGImageContext.h" #include #include diff --git a/layout/base/nsRefreshDriver.h b/layout/base/nsRefreshDriver.h index 647763b20c26..f39f22967da6 100644 --- a/layout/base/nsRefreshDriver.h +++ b/layout/base/nsRefreshDriver.h @@ -20,7 +20,6 @@ #include "nsTObserverArray.h" #include "nsTArray.h" #include "nsTHashtable.h" -#include "nsTObserverArray.h" #include "nsClassHashtable.h" #include "nsHashKeys.h" #include "mozilla/Attributes.h" diff --git a/layout/forms/nsDateTimeControlFrame.cpp b/layout/forms/nsDateTimeControlFrame.cpp index 2b12fb995b68..61d9ab24d899 100644 --- a/layout/forms/nsDateTimeControlFrame.cpp +++ b/layout/forms/nsDateTimeControlFrame.cpp @@ -14,7 +14,6 @@ #include "nsContentUtils.h" #include "nsCheckboxRadioFrame.h" #include "nsGkAtoms.h" -#include "nsContentUtils.h" #include "nsContentCreatorFunctions.h" #include "mozilla/AsyncEventDispatcher.h" #include "mozilla/dom/HTMLInputElement.h" diff --git a/layout/generic/nsBlockFrame.cpp b/layout/generic/nsBlockFrame.cpp index 97a55b2264c6..f50e28df62b3 100644 --- a/layout/generic/nsBlockFrame.cpp +++ b/layout/generic/nsBlockFrame.cpp @@ -35,7 +35,6 @@ #include "nsIPresShell.h" #include "nsHTMLParts.h" #include "nsGkAtoms.h" -#include "nsGenericHTMLElement.h" #include "nsAttrValueInlines.h" #include "mozilla/Sprintf.h" #include "nsFloatManager.h" diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index 80b4871c96ba..9ec819eca30b 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -47,7 +47,6 @@ #include "nsStyleConsts.h" #include "nsIPresShell.h" #include "mozilla/Logging.h" -#include "mozilla/Sprintf.h" #include "nsLayoutUtils.h" #include "LayoutLogging.h" #include "mozilla/RestyleManager.h" @@ -116,9 +115,6 @@ #include "ActiveLayerTracker.h" #include "nsITheme.h" -#include "nsStyleConsts.h" - -#include "ImageLoader.h" using namespace mozilla; using namespace mozilla::css; diff --git a/layout/generic/nsFrameSelection.cpp b/layout/generic/nsFrameSelection.cpp index 7064bd586547..e5223a50a0df 100644 --- a/layout/generic/nsFrameSelection.cpp +++ b/layout/generic/nsFrameSelection.cpp @@ -44,7 +44,6 @@ static NS_DEFINE_CID(kFrameTraversalCID, NS_FRAMETRAVERSAL_CID); #include "nsTextFrame.h" -#include "nsContentUtils.h" #include "nsThreadUtils.h" #include "mozilla/Preferences.h" diff --git a/layout/generic/nsImageFrame.cpp b/layout/generic/nsImageFrame.cpp index e63d98f6fea5..94b0a24293ad 100644 --- a/layout/generic/nsImageFrame.cpp +++ b/layout/generic/nsImageFrame.cpp @@ -56,7 +56,6 @@ #include "nsLayoutUtils.h" #include "nsDisplayList.h" #include "nsIContent.h" -#include "nsIDocument.h" #include "FrameLayerBuilder.h" #include "mozilla/dom/Selection.h" #include "nsIURIMutator.h" diff --git a/layout/generic/nsPageFrame.cpp b/layout/generic/nsPageFrame.cpp index 75b958840893..ea561769ebed 100644 --- a/layout/generic/nsPageFrame.cpp +++ b/layout/generic/nsPageFrame.cpp @@ -16,7 +16,6 @@ #include "nsIPresShell.h" #include "nsPageContentFrame.h" #include "nsDisplayList.h" -#include "nsLayoutUtils.h" // for function BinarySearchForPosition #include "nsSimplePageSequenceFrame.h" // for nsSharedPageData #include "nsTextFormatter.h" // for page number localization formatting #include "nsBidiUtils.h" diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp index e1d2367d8c73..76724a2bf6ad 100644 --- a/layout/generic/nsTextFrame.cpp +++ b/layout/generic/nsTextFrame.cpp @@ -82,7 +82,6 @@ #include "nsPrintfCString.h" -#include "gfxContext.h" #include "mozilla/gfx/DrawTargetRecording.h" #include "mozilla/UniquePtr.h" diff --git a/layout/inspector/inDeepTreeWalker.cpp b/layout/inspector/inDeepTreeWalker.cpp index 01ad890856e4..60af7129f8b3 100644 --- a/layout/inspector/inDeepTreeWalker.cpp +++ b/layout/inspector/inDeepTreeWalker.cpp @@ -8,7 +8,6 @@ #include "inLayoutUtils.h" #include "BindingStyleRule.h" -#include "InspectorUtils.h" #include "nsString.h" #include "nsIDocument.h" #include "nsServiceManagerUtils.h" diff --git a/layout/painting/nsCSSRendering.cpp b/layout/painting/nsCSSRendering.cpp index ac95c7a2b28a..c9d3dae92575 100644 --- a/layout/painting/nsCSSRendering.cpp +++ b/layout/painting/nsCSSRendering.cpp @@ -39,7 +39,6 @@ #include "nsCSSRendering.h" #include "nsCSSColorUtils.h" #include "nsITheme.h" -#include "nsStyleConsts.h" #include "nsLayoutUtils.h" #include "nsBlockFrame.h" #include "nsStyleStructInlines.h" diff --git a/layout/painting/nsCSSRenderingBorders.cpp b/layout/painting/nsCSSRenderingBorders.cpp index e63b35cac9d2..ea599e3b5cf5 100644 --- a/layout/painting/nsCSSRenderingBorders.cpp +++ b/layout/painting/nsCSSRenderingBorders.cpp @@ -27,7 +27,6 @@ #include "nsClassHashtable.h" #include "nsPresContext.h" #include "nsStyleStruct.h" -#include "mozilla/gfx/2D.h" #include "gfx2DGlue.h" #include "gfxGradientCache.h" #include "mozilla/layers/StackingContextHelper.h" diff --git a/layout/painting/nsDisplayList.h b/layout/painting/nsDisplayList.h index 736133cd11d2..be01f56b8bdf 100644 --- a/layout/painting/nsDisplayList.h +++ b/layout/painting/nsDisplayList.h @@ -17,7 +17,6 @@ #include "gfxContext.h" #include "mozilla/ArenaAllocator.h" #include "mozilla/Assertions.h" -#include "mozilla/Attributes.h" #include "mozilla/Array.h" #include "mozilla/DebugOnly.h" #include "mozilla/EnumSet.h" @@ -37,7 +36,6 @@ #include "ImgDrawResult.h" #include "mozilla/EffectCompositor.h" #include "mozilla/EnumeratedArray.h" -#include "mozilla/Maybe.h" #include "mozilla/UniquePtr.h" #include "mozilla/TimeStamp.h" #include "mozilla/gfx/UserData.h" diff --git a/layout/painting/nsImageRenderer.cpp b/layout/painting/nsImageRenderer.cpp index 359ae0d9c5eb..519b929b18c6 100644 --- a/layout/painting/nsImageRenderer.cpp +++ b/layout/painting/nsImageRenderer.cpp @@ -25,7 +25,6 @@ #include "nsSVGDisplayableFrame.h" #include "SVGObserverUtils.h" #include "nsSVGIntegrationUtils.h" -#include "mozilla/layers/WebRenderLayerManager.h" using namespace mozilla; using namespace mozilla::gfx; diff --git a/layout/printing/nsPrintJob.cpp b/layout/printing/nsPrintJob.cpp index fda780a623b9..612c1453acdd 100644 --- a/layout/printing/nsPrintJob.cpp +++ b/layout/printing/nsPrintJob.cpp @@ -47,7 +47,6 @@ static const char sPrintSettingsServiceContractID[] = // Print Preview #include "imgIContainer.h" // image animation mode constants -#include "nsIWebBrowserPrint.h" // needed for PrintPreview Navigation constants // Print Progress #include "nsIPrintProgress.h" @@ -94,7 +93,6 @@ static const char kPrintingPromptService[] = #include "nsIDeviceContextSpec.h" #include "nsDeviceContextSpecProxy.h" #include "nsViewManager.h" -#include "nsView.h" #include "nsIPageSequenceFrame.h" #include "nsIURL.h" diff --git a/layout/style/Loader.cpp b/layout/style/Loader.cpp index a259782fcd01..0cf1be673bec 100644 --- a/layout/style/Loader.cpp +++ b/layout/style/Loader.cpp @@ -15,7 +15,6 @@ #include "mozilla/LoadInfo.h" #include "mozilla/Logging.h" #include "mozilla/MemoryReporting.h" -#include "mozilla/StyleSheetInlines.h" #include "mozilla/SystemGroup.h" #include "mozilla/ResultExtensions.h" #include "mozilla/URLPreloader.h" @@ -65,7 +64,6 @@ #include "mozilla/dom/SRICheck.h" #include "mozilla/Encoding.h" -#include "mozilla/Logging.h" using namespace mozilla::dom; diff --git a/layout/style/StyleAnimationValue.cpp b/layout/style/StyleAnimationValue.cpp index b5984dbd554f..be047c0e0afd 100644 --- a/layout/style/StyleAnimationValue.cpp +++ b/layout/style/StyleAnimationValue.cpp @@ -10,7 +10,6 @@ #include "mozilla/ArrayUtils.h" #include "mozilla/MathAlgorithms.h" -#include "mozilla/ServoBindings.h" #include "mozilla/ServoStyleSet.h" #include "mozilla/Tuple.h" #include "mozilla/UniquePtr.h" diff --git a/layout/style/nsCSSProps.cpp b/layout/style/nsCSSProps.cpp index 5070f53d4e6b..f7f05173aff8 100644 --- a/layout/style/nsCSSProps.cpp +++ b/layout/style/nsCSSProps.cpp @@ -16,7 +16,6 @@ #include "nsCSSKeywords.h" #include "nsLayoutUtils.h" -#include "nsStyleConsts.h" #include "nsIWidget.h" #include "nsStyleConsts.h" // For system widget appearance types diff --git a/layout/style/nsLayoutStylesheetCache.cpp b/layout/style/nsLayoutStylesheetCache.cpp index a898b81fa53f..4f0eef64418d 100644 --- a/layout/style/nsLayoutStylesheetCache.cpp +++ b/layout/style/nsLayoutStylesheetCache.cpp @@ -7,7 +7,6 @@ #include "nsLayoutStylesheetCache.h" #include "nsAppDirectoryServiceDefs.h" -#include "mozilla/StyleSheetInlines.h" #include "mozilla/MemoryReporting.h" #include "mozilla/Omnijar.h" #include "mozilla/Preferences.h" diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp index e901f633d35c..cb8090f80a7b 100644 --- a/layout/style/nsStyleStruct.cpp +++ b/layout/style/nsStyleStruct.cpp @@ -12,7 +12,6 @@ #include "nsStyleStruct.h" #include "nsStyleStructInlines.h" #include "nsStyleConsts.h" -#include "nsStyleConsts.h" #include "nsString.h" #include "nsPresContext.h" #include "nsIAppShellService.h" diff --git a/layout/svg/SVGContextPaint.cpp b/layout/svg/SVGContextPaint.cpp index 080a3b0693ff..0e20c4ef6040 100644 --- a/layout/svg/SVGContextPaint.cpp +++ b/layout/svg/SVGContextPaint.cpp @@ -14,7 +14,6 @@ #include "nsIDocument.h" #include "nsSVGPaintServerFrame.h" #include "SVGObserverUtils.h" -#include "nsSVGPaintServerFrame.h" using namespace mozilla::gfx; using namespace mozilla::image; diff --git a/layout/svg/nsFilterInstance.cpp b/layout/svg/nsFilterInstance.cpp index 782882a9125f..c208540e5ede 100644 --- a/layout/svg/nsFilterInstance.cpp +++ b/layout/svg/nsFilterInstance.cpp @@ -25,7 +25,6 @@ #include "nsSVGUtils.h" #include "SVGContentUtils.h" #include "FilterSupport.h" -#include "gfx2DGlue.h" #include "mozilla/Unused.h" using namespace mozilla; diff --git a/layout/tables/nsCellMap.h b/layout/tables/nsCellMap.h index 82abdbc531bd..40f8a277e79f 100644 --- a/layout/tables/nsCellMap.h +++ b/layout/tables/nsCellMap.h @@ -8,7 +8,6 @@ #include "nscore.h" #include "celldata.h" #include "nsTArray.h" -#include "nsTArray.h" #include "nsCOMPtr.h" #include "nsAlgorithm.h" #include "nsRect.h" From 63c73cd666169e1c894ccc7011a74fd9eb6fab5d Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Wed, 5 Dec 2018 19:56:32 +0000 Subject: [PATCH 13/34] Bug 1504756 - [marionette] Add executeSoon() to run tasks on the main thread. r=ato To be closer to other test harnesses which are using executeSoon() to run a task on the main thread, this patch adds the same method to Marionette's sync module. Differential Revision: https://phabricator.services.mozilla.com/D13659 --HG-- extra : moz-landing-system : lando --- testing/marionette/doc/internals/sync.rst | 2 ++ testing/marionette/sync.js | 16 ++++++++++++++++ testing/marionette/test/unit/test_sync.js | 17 +++++++++++++++++ testing/marionette/transport.js | 19 +++++++++++-------- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/testing/marionette/doc/internals/sync.rst b/testing/marionette/doc/internals/sync.rst index d5f35dedf94e..8e705e175f3e 100644 --- a/testing/marionette/doc/internals/sync.rst +++ b/testing/marionette/doc/internals/sync.rst @@ -3,6 +3,8 @@ sync module Provides an assortment of synchronisation primitives. +.. js:autofunction:: executeSoon + .. js:autoclass:: MessageManagerDestroyedPromise :members: diff --git a/testing/marionette/sync.js b/testing/marionette/sync.js index e3f7759a9c74..6573c0f4e3a3 100644 --- a/testing/marionette/sync.js +++ b/testing/marionette/sync.js @@ -18,6 +18,7 @@ const {Log} = ChromeUtils.import("chrome://marionette/content/log.js", {}); XPCOMUtils.defineLazyGetter(this, "log", Log.get); this.EXPORTED_SYMBOLS = [ + "executeSoon", "DebounceCallback", "IdlePromise", "MessageManagerDestroyedPromise", @@ -30,6 +31,21 @@ const {TYPE_ONE_SHOT, TYPE_REPEATING_SLACK} = Ci.nsITimer; const PROMISE_TIMEOUT = AppConstants.DEBUG ? 4500 : 1500; + +/** + * Dispatch a function to be executed on the main thread. + * + * @param {function} func + * Function to be executed. + */ +function executeSoon(func) { + if (typeof func != "function") { + throw new TypeError(); + } + + Services.tm.dispatchToMainThread(func); +} + /** * @callback Condition * diff --git a/testing/marionette/test/unit/test_sync.js b/testing/marionette/test/unit/test_sync.js index e7dbcb54a541..d9369edee05a 100644 --- a/testing/marionette/test/unit/test_sync.js +++ b/testing/marionette/test/unit/test_sync.js @@ -35,6 +35,23 @@ class MockTimer { } } +add_test(function test_executeSoon_callback() { + // executeSoon() is already defined for xpcshell in head.js. As such import + // our implementation into a custom namespace. + let sync = {}; + ChromeUtils.import("chrome://marionette/content/sync.js", sync); + + for (let func of ["foo", null, true, [], {}]) { + Assert.throws(() => sync.executeSoon(func), /TypeError/); + } + + let a; + sync.executeSoon(() => { a = 1; }); + executeSoon(() => equal(1, a)); + + run_next_test(); +}); + add_test(function test_PollPromise_funcTypes() { for (let type of ["foo", 42, null, undefined, true, [], {}]) { Assert.throws(() => new PollPromise(type), /TypeError/); diff --git a/testing/marionette/transport.js b/testing/marionette/transport.js index e357d360f0a2..61e927bcef62 100644 --- a/testing/marionette/transport.js +++ b/testing/marionette/transport.js @@ -10,14 +10,17 @@ const CC = Components.Constructor; ChromeUtils.import("resource://gre/modules/Services.jsm"); ChromeUtils.import("resource://gre/modules/EventEmitter.jsm"); -const {StreamUtils} = - ChromeUtils.import("chrome://marionette/content/stream-utils.js", {}); -const {Packet, JSONPacket, BulkPacket} = - ChromeUtils.import("chrome://marionette/content/packets.js", {}); - -const executeSoon = function(func) { - Services.tm.dispatchToMainThread(func); -}; +const { + StreamUtils, +} = ChromeUtils.import("chrome://marionette/content/stream-utils.js", {}); +const { + BulkPacket, + JSONPacket, + Packet, +} = ChromeUtils.import("chrome://marionette/content/packets.js", {}); +const { + executeSoon, +} = ChromeUtils.import("chrome://marionette/content/sync.js", {}); const flags = {wantVerbose: false, wantLogging: false}; From e099e3a84089d990c0c8e81d8094c4ff38e9dbe5 Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Wed, 5 Dec 2018 19:56:56 +0000 Subject: [PATCH 14/34] Bug 1504756 - [marionette] Use waitForEvent() when waiting for events. r=ato Depends on D13659 Differential Revision: https://phabricator.services.mozilla.com/D13660 --HG-- extra : moz-landing-system : lando --- testing/marionette/browser.js | 54 ++++---- testing/marionette/doc/internals/sync.rst | 2 + testing/marionette/driver.js | 33 +++-- testing/marionette/sync.js | 97 ++++++++++++++ testing/marionette/test/unit/test_sync.js | 123 ++++++++++++++++++ .../tests/execute_script/promise.py.ini | 1 - 6 files changed, 260 insertions(+), 50 deletions(-) diff --git a/testing/marionette/browser.js b/testing/marionette/browser.js index ca276dbe08b4..952cb58defd8 100644 --- a/testing/marionette/browser.js +++ b/testing/marionette/browser.js @@ -13,6 +13,7 @@ const { } = ChromeUtils.import("chrome://marionette/content/error.js", {}); const { MessageManagerDestroyedPromise, + waitForEvent, } = ChromeUtils.import("chrome://marionette/content/sync.js", {}); this.EXPORTED_SYMBOLS = ["browser", "Context", "WindowState"]; @@ -287,17 +288,13 @@ browser.Context = class { * A promise which is resolved when the current window has been closed. */ closeWindow() { - return new Promise(resolve => { - // Wait for the window message manager to be destroyed - let destroyed = new MessageManagerDestroyedPromise( - this.window.messageManager); + let destroyed = new MessageManagerDestroyedPromise( + this.window.messageManager); + let unloaded = waitForEvent(this.window, "unload"); - this.window.addEventListener("unload", async () => { - await destroyed; - resolve(); - }, {once: true}); - this.window.close(); - }); + this.window.close(); + + return Promise.all([destroyed, unloaded]); } /** @@ -319,30 +316,25 @@ browser.Context = class { return this.closeWindow(); } - return new Promise((resolve, reject) => { - // Wait for the browser message manager to be destroyed - let browserDetached = async () => { - await new MessageManagerDestroyedPromise(this.messageManager); - resolve(); - }; + let destroyed = new MessageManagerDestroyedPromise(this.messageManager); + let tabClosed; - if (this.tabBrowser.closeTab) { - // Fennec - this.tabBrowser.deck.addEventListener( - "TabClose", browserDetached, {once: true}); - this.tabBrowser.closeTab(this.tab); + if (this.tabBrowser.closeTab) { + // Fennec + tabClosed = waitForEvent(this.tabBrowser.deck, "TabClose"); + this.tabBrowser.closeTab(this.tab); - } else if (this.tabBrowser.removeTab) { - // Firefox - this.tab.addEventListener( - "TabClose", browserDetached, {once: true}); - this.tabBrowser.removeTab(this.tab); + } else if (this.tabBrowser.removeTab) { + // Firefox + tabClosed = waitForEvent(this.tab, "TabClose"); + this.tabBrowser.removeTab(this.tab); - } else { - reject(new UnsupportedOperationError( - `closeTab() not supported in ${this.driver.appName}`)); - } - }); + } else { + throw new UnsupportedOperationError( + `closeTab() not supported in ${this.driver.appName}`); + } + + return Promise.all([destroyed, tabClosed]); } /** diff --git a/testing/marionette/doc/internals/sync.rst b/testing/marionette/doc/internals/sync.rst index 8e705e175f3e..ae018afea2cf 100644 --- a/testing/marionette/doc/internals/sync.rst +++ b/testing/marionette/doc/internals/sync.rst @@ -16,3 +16,5 @@ Provides an assortment of synchronisation primitives. .. js:autoclass:: TimedPromise :members: + +.. js:autofunction:: waitForEvent diff --git a/testing/marionette/driver.js b/testing/marionette/driver.js index af9f758676fe..b9bbb0528f07 100644 --- a/testing/marionette/driver.js +++ b/testing/marionette/driver.js @@ -62,6 +62,7 @@ const { IdlePromise, PollPromise, TimedPromise, + waitForEvent, } = ChromeUtils.import("chrome://marionette/content/sync.js", {}); XPCOMUtils.defineLazyGetter(this, "logger", Log.get); @@ -3101,16 +3102,14 @@ GeckoDriver.prototype.dismissDialog = async function() { let win = assert.open(this.getCurrentWindow()); this._checkIfAlertIsPresent(); - await new Promise(resolve => { - win.addEventListener("DOMModalDialogClosed", async () => { - await new IdlePromise(win); - this.dialog = null; - resolve(); - }, {once: true}); + let dialogClosed = waitForEvent(win, "DOMModalDialogClosed"); - let {button0, button1} = this.dialog.ui; - (button1 ? button1 : button0).click(); - }); + let {button0, button1} = this.dialog.ui; + (button1 ? button1 : button0).click(); + + await dialogClosed; + + this.dialog = null; }; /** @@ -3121,16 +3120,14 @@ GeckoDriver.prototype.acceptDialog = async function() { let win = assert.open(this.getCurrentWindow()); this._checkIfAlertIsPresent(); - await new Promise(resolve => { - win.addEventListener("DOMModalDialogClosed", async () => { - await new IdlePromise(win); - this.dialog = null; - resolve(); - }, {once: true}); + let dialogClosed = waitForEvent(win, "DOMModalDialogClosed"); - let {button0} = this.dialog.ui; - button0.click(); - }); + let {button0} = this.dialog.ui; + button0.click(); + + await dialogClosed; + + this.dialog = null; }; /** diff --git a/testing/marionette/sync.js b/testing/marionette/sync.js index 6573c0f4e3a3..badfea82667e 100644 --- a/testing/marionette/sync.js +++ b/testing/marionette/sync.js @@ -25,6 +25,7 @@ this.EXPORTED_SYMBOLS = [ "PollPromise", "Sleep", "TimedPromise", + "waitForEvent", ]; const {TYPE_ONE_SHOT, TYPE_REPEATING_SLACK} = Ci.nsITimer; @@ -367,3 +368,99 @@ class DebounceCallback { } } this.DebounceCallback = DebounceCallback; + +/** + * Wait for an event to be fired on a specified element. + * + * This method has been duplicated from BrowserTestUtils.jsm. + * + * Because this function is intended for testing, any error in checkFn + * will cause the returned promise to be rejected instead of waiting for + * the next event, since this is probably a bug in the test. + * + * Usage:: + * + * let promiseEvent = waitForEvent(element, "eventName"); + * // Do some processing here that will cause the event to be fired + * // ... + * // Now wait until the Promise is fulfilled + * let receivedEvent = await promiseEvent; + * + * The promise resolution/rejection handler for the returned promise is + * guaranteed not to be called until the next event tick after the event + * listener gets called, so that all other event listeners for the element + * are executed before the handler is executed:: + * + * let promiseEvent = waitForEvent(element, "eventName"); + * // Same event tick here. + * await promiseEvent; + * // Next event tick here. + * + * If some code, such like adding yet another event listener, needs to be + * executed in the same event tick, use raw addEventListener instead and + * place the code inside the event listener:: + * + * element.addEventListener("load", () => { + * // Add yet another event listener in the same event tick as the load + * // event listener. + * p = waitForEvent(element, "ready"); + * }, { once: true }); + * + * @param {Element} subject + * The element that should receive the event. + * @param {string} eventName + * Name of the event to listen to. + * @param {Object=} options + * Extra options. + * @param {boolean=} options.capture + * True to use a capturing listener. + * @param {function(Event)=} options.checkFn + * Called with the ``Event`` object as argument, should return ``true`` + * if the event is the expected one, or ``false`` if it should be + * ignored and listening should continue. If not specified, the first + * event with the specified name resolves the returned promise. + * @param {boolean=} options.wantsUntrusted + * True to receive synthetic events dispatched by web content. + * + * @return {Promise.} + * Promise which resolves to the received ``Event`` object, or rejects + * in case of a failure. + */ +function waitForEvent(subject, eventName, + {capture = false, checkFn = null, wantsUntrusted = false} = {}) { + if (subject == null || !("addEventListener" in subject)) { + throw new TypeError(); + } + if (typeof eventName != "string") { + throw new TypeError(); + } + if (capture != null && typeof capture != "boolean") { + throw new TypeError(); + } + if (checkFn != null && typeof checkFn != "function") { + throw new TypeError(); + } + if (wantsUntrusted != null && typeof wantsUntrusted != "boolean") { + throw new TypeError(); + } + + return new Promise((resolve, reject) => { + subject.addEventListener(eventName, function listener(event) { + log.trace(`Received DOM event ${event.type} for ${event.target}`); + try { + if (checkFn && !checkFn(event)) { + return; + } + subject.removeEventListener(eventName, listener, capture); + executeSoon(() => resolve(event)); + } catch (ex) { + try { + subject.removeEventListener(eventName, listener, capture); + } catch (ex2) { + // Maybe the provided object does not support removeEventListener. + } + executeSoon(() => reject(ex)); + } + }, capture, wantsUntrusted); + }); +} diff --git a/testing/marionette/test/unit/test_sync.js b/testing/marionette/test/unit/test_sync.js index d9369edee05a..795dc537b964 100644 --- a/testing/marionette/test/unit/test_sync.js +++ b/testing/marionette/test/unit/test_sync.js @@ -8,10 +8,53 @@ const { PollPromise, Sleep, TimedPromise, + waitForEvent, } = ChromeUtils.import("chrome://marionette/content/sync.js", {}); const DEFAULT_TIMEOUT = 2000; +/** + * Mimic a DOM node for listening for events. + */ +class MockElement { + constructor() { + this.capture = false; + this.func = null; + this.eventName = null; + this.untrusted = false; + } + + addEventListener(name, func, capture, untrusted) { + this.eventName = name; + this.func = func; + if (capture != null) { + this.capture = capture; + } + if (untrusted != null) { + this.untrusted = untrusted; + } + } + + click() { + if (this.func) { + let details = { + capture: this.capture, + target: this, + type: this.eventName, + untrusted: this.untrusted, + }; + this.func(details); + } + } + + removeEventListener(name, func) { + this.capture = false; + this.func = null; + this.eventName = null; + this.untrusted = false; + } +} + /** * Mimics nsITimer, but instead of using a system clock you can * preprogram it to invoke the callback after a given number of ticks. @@ -230,3 +273,83 @@ add_task(async function test_DebounceCallback_repeatedCallback() { equal(ncalls, 1); ok(debouncer.timer.cancelled); }); + +add_task(async function test_waitForEvent_subjectAndEventNameTypes() { + let element = new MockElement(); + + for (let subject of ["foo", 42, null, undefined, true, [], {}]) { + Assert.throws(() => waitForEvent(subject, "click"), /TypeError/); + } + + for (let eventName of [42, null, undefined, true, [], {}]) { + Assert.throws(() => waitForEvent(element, eventName), /TypeError/); + } + + let clicked = waitForEvent(element, "click"); + element.click(); + let event = await clicked; + equal(element, event.target); +}); + +add_task(async function test_waitForEvent_captureTypes() { + let element = new MockElement(); + + for (let capture of ["foo", 42, [], {}]) { + Assert.throws(() => waitForEvent( + element, "click", {capture}), /TypeError/); + } + + for (let capture of [null, undefined, false, true]) { + let expected_capture = (capture == null) ? false : capture; + + element = new MockElement(); + let clicked = waitForEvent(element, "click", {capture}); + element.click(); + let event = await clicked; + equal(element, event.target); + equal(expected_capture, event.capture); + } +}); + +add_task(async function test_waitForEvent_checkFnTypes() { + let element = new MockElement(); + + for (let checkFn of ["foo", 42, true, [], {}]) { + Assert.throws(() => waitForEvent( + element, "click", {checkFn}), /TypeError/); + } + + let count; + for (let checkFn of [null, undefined, event => count++ > 0]) { + let expected_count = (checkFn == null) ? 0 : 2; + count = 0; + + element = new MockElement(); + let clicked = waitForEvent(element, "click", {checkFn}); + element.click(); + element.click(); + let event = await clicked; + equal(element, event.target); + equal(expected_count, count); + } +}); + +add_task(async function test_waitForEvent_wantsUntrustedTypes() { + let element = new MockElement(); + + for (let wantsUntrusted of ["foo", 42, [], {}]) { + Assert.throws(() => waitForEvent( + element, "click", {wantsUntrusted}), /TypeError/); + } + + for (let wantsUntrusted of [null, undefined, false, true]) { + let expected_untrusted = (wantsUntrusted == null) ? false : wantsUntrusted; + + element = new MockElement(); + let clicked = waitForEvent(element, "click", {wantsUntrusted}); + element.click(); + let event = await clicked; + equal(element, event.target); + equal(expected_untrusted, event.untrusted); + } +}); diff --git a/testing/web-platform/meta/webdriver/tests/execute_script/promise.py.ini b/testing/web-platform/meta/webdriver/tests/execute_script/promise.py.ini index 97bf2cf50f85..f1544e24b8ba 100644 --- a/testing/web-platform/meta/webdriver/tests/execute_script/promise.py.ini +++ b/testing/web-platform/meta/webdriver/tests/execute_script/promise.py.ini @@ -1,5 +1,4 @@ [promise.py] - expected: TIMEOUT [test_promise_timeout] expected: FAIL From c3571c164d1dd8392c2bdfbf9c7a219d98244453 Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Wed, 5 Dec 2018 19:57:23 +0000 Subject: [PATCH 15/34] Bug 1504756 - [marionette] Use waitForMessage() to wait for an expected message manager message. r=ato Depends on D13660 Differential Revision: https://phabricator.services.mozilla.com/D13661 --HG-- extra : moz-landing-system : lando --- testing/marionette/doc/internals/sync.rst | 2 + testing/marionette/sync.js | 47 +++++++++++++++ testing/marionette/test/unit/test_sync.js | 70 +++++++++++++++++++++++ 3 files changed, 119 insertions(+) diff --git a/testing/marionette/doc/internals/sync.rst b/testing/marionette/doc/internals/sync.rst index ae018afea2cf..d97c12742781 100644 --- a/testing/marionette/doc/internals/sync.rst +++ b/testing/marionette/doc/internals/sync.rst @@ -18,3 +18,5 @@ Provides an assortment of synchronisation primitives. :members: .. js:autofunction:: waitForEvent + +.. js:autofunction:: waitForMessage diff --git a/testing/marionette/sync.js b/testing/marionette/sync.js index badfea82667e..d88ed8ac02cc 100644 --- a/testing/marionette/sync.js +++ b/testing/marionette/sync.js @@ -13,6 +13,7 @@ const { stack, TimeoutError, } = ChromeUtils.import("chrome://marionette/content/error.js", {}); +const {truncate} = ChromeUtils.import("chrome://marionette/content/format.js", {}); const {Log} = ChromeUtils.import("chrome://marionette/content/log.js", {}); XPCOMUtils.defineLazyGetter(this, "log", Log.get); @@ -26,6 +27,7 @@ this.EXPORTED_SYMBOLS = [ "Sleep", "TimedPromise", "waitForEvent", + "waitForMessage", ]; const {TYPE_ONE_SHOT, TYPE_REPEATING_SLACK} = Ci.nsITimer; @@ -464,3 +466,48 @@ function waitForEvent(subject, eventName, }, capture, wantsUntrusted); }); } + +/** + * Wait for a message to be fired from a particular message manager. + * + * This method has been duplicated from BrowserTestUtils.jsm. + * + * @param {nsIMessageManager} messageManager + * The message manager that should be used. + * @param {string} messageName + * The message to wait for. + * @param {Object=} options + * Extra options. + * @param {function(Message)=} options.checkFn + * Called with the ``Message`` object as argument, should return ``true`` + * if the message is the expected one, or ``false`` if it should be + * ignored and listening should continue. If not specified, the first + * message with the specified name resolves the returned promise. + * + * @return {Promise.} + * Promise which resolves to the data property of the received + * ``Message``. + */ +function waitForMessage(messageManager, messageName, + {checkFn = undefined} = {}) { + if (messageManager == null || !("addMessageListener" in messageManager)) { + throw new TypeError(); + } + if (typeof messageName != "string") { + throw new TypeError(); + } + if (checkFn && typeof checkFn != "function") { + throw new TypeError(); + } + + return new Promise(resolve => { + messageManager.addMessageListener(messageName, function onMessage(msg) { + log.trace(`Received ${messageName} for ${msg.target}`); + if (checkFn && !checkFn(msg)) { + return; + } + messageManager.removeMessageListener(messageName, onMessage); + resolve(msg.data); + }); + }); +} diff --git a/testing/marionette/test/unit/test_sync.js b/testing/marionette/test/unit/test_sync.js index 795dc537b964..aaae9b9302ec 100644 --- a/testing/marionette/test/unit/test_sync.js +++ b/testing/marionette/test/unit/test_sync.js @@ -9,6 +9,7 @@ const { Sleep, TimedPromise, waitForEvent, + waitForMessage, } = ChromeUtils.import("chrome://marionette/content/sync.js", {}); const DEFAULT_TIMEOUT = 2000; @@ -55,6 +56,36 @@ class MockElement { } } +/** + * Mimic a message manager for sending messages. + */ +class MessageManager { + constructor() { + this.func = null; + this.message = null; + } + + addMessageListener(message, func) { + this.func = func; + this.message = message; + } + + removeMessageListener(message) { + this.func = null; + this.message = null; + } + + send(message, data) { + if (this.func) { + this.func({ + data, + message, + target: this, + }); + } + } +} + /** * Mimics nsITimer, but instead of using a system clock you can * preprogram it to invoke the callback after a given number of ticks. @@ -353,3 +384,42 @@ add_task(async function test_waitForEvent_wantsUntrustedTypes() { equal(expected_untrusted, event.untrusted); } }); + +add_task(async function test_waitForMessage_messageManagerAndMessageTypes() { + let messageManager = new MessageManager(); + + for (let manager of ["foo", 42, null, undefined, true, [], {}]) { + Assert.throws(() => waitForMessage(manager, "message"), /TypeError/); + } + + for (let message of [42, null, undefined, true, [], {}]) { + Assert.throws(() => waitForEvent(messageManager, message), /TypeError/); + } + + let data = {"foo": "bar"}; + let sent = waitForMessage(messageManager, "message"); + messageManager.send("message", data); + equal(data, await sent); +}); + +add_task(async function test_waitForMessage_checkFnTypes() { + let messageManager = new MessageManager(); + + for (let checkFn of ["foo", 42, true, [], {}]) { + Assert.throws(() => waitForMessage( + messageManager, "message", {checkFn}), /TypeError/); + } + + let data1 = {"fo": "bar"}; + let data2 = {"foo": "bar"}; + + for (let checkFn of [null, undefined, msg => "foo" in msg.data]) { + let expected_data = (checkFn == null) ? data1 : data2; + + messageManager = new MessageManager(); + let sent = waitForMessage(messageManager, "message", {checkFn}); + messageManager.send("message", data1); + messageManager.send("message", data2); + equal(expected_data, await sent); + } +}); From eb8486826a8caeb078c88b518e14afc4afa586e7 Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Wed, 5 Dec 2018 19:57:48 +0000 Subject: [PATCH 16/34] Bug 1504756 - [marionette] Use waitForObserverTopic when waiting for observer notifications. r=ato Depends on D13661 Differential Revision: https://phabricator.services.mozilla.com/D13662 --HG-- extra : moz-landing-system : lando --- testing/marionette/browser.js | 18 +++-- testing/marionette/doc/internals/sync.rst | 5 +- testing/marionette/driver.js | 10 +-- testing/marionette/proxy.js | 6 +- testing/marionette/sync.js | 90 ++++++++++++----------- testing/marionette/test/unit/test_sync.js | 36 +++++++++ 6 files changed, 106 insertions(+), 59 deletions(-) diff --git a/testing/marionette/browser.js b/testing/marionette/browser.js index 952cb58defd8..4a10661aee61 100644 --- a/testing/marionette/browser.js +++ b/testing/marionette/browser.js @@ -12,8 +12,8 @@ const { UnsupportedOperationError, } = ChromeUtils.import("chrome://marionette/content/error.js", {}); const { - MessageManagerDestroyedPromise, waitForEvent, + waitForObserverTopic, } = ChromeUtils.import("chrome://marionette/content/sync.js", {}); this.EXPORTED_SYMBOLS = ["browser", "Context", "WindowState"]; @@ -288,13 +288,15 @@ browser.Context = class { * A promise which is resolved when the current window has been closed. */ closeWindow() { - let destroyed = new MessageManagerDestroyedPromise( - this.window.messageManager); + // Create a copy of the messageManager before it is disconnected + let messageManager = this.window.messageManager; + let disconnected = waitForObserverTopic("message-manager-disconnect", + subject => subject === messageManager); let unloaded = waitForEvent(this.window, "unload"); this.window.close(); - return Promise.all([destroyed, unloaded]); + return Promise.all([disconnected, unloaded]); } /** @@ -316,7 +318,11 @@ browser.Context = class { return this.closeWindow(); } - let destroyed = new MessageManagerDestroyedPromise(this.messageManager); + // Create a copy of the messageManager before it is disconnected + let messageManager = this.messageManager; + let disconnected = waitForObserverTopic("message-manager-disconnect", + subject => subject === messageManager); + let tabClosed; if (this.tabBrowser.closeTab) { @@ -334,7 +340,7 @@ browser.Context = class { `closeTab() not supported in ${this.driver.appName}`); } - return Promise.all([destroyed, tabClosed]); + return Promise.all([disconnected, tabClosed]); } /** diff --git a/testing/marionette/doc/internals/sync.rst b/testing/marionette/doc/internals/sync.rst index d97c12742781..0ec60ff5600d 100644 --- a/testing/marionette/doc/internals/sync.rst +++ b/testing/marionette/doc/internals/sync.rst @@ -5,9 +5,6 @@ Provides an assortment of synchronisation primitives. .. js:autofunction:: executeSoon -.. js:autoclass:: MessageManagerDestroyedPromise - :members: - .. js:autoclass:: PollPromise :members: @@ -20,3 +17,5 @@ Provides an assortment of synchronisation primitives. .. js:autofunction:: waitForEvent .. js:autofunction:: waitForMessage + +.. js:autofunction:: waitForObserverTopic diff --git a/testing/marionette/driver.js b/testing/marionette/driver.js index b9bbb0528f07..395295f99b7c 100644 --- a/testing/marionette/driver.js +++ b/testing/marionette/driver.js @@ -63,6 +63,7 @@ const { PollPromise, TimedPromise, waitForEvent, + waitForObserverTopic, } = ChromeUtils.import("chrome://marionette/content/sync.js", {}); XPCOMUtils.defineLazyGetter(this, "logger", Log.get); @@ -3298,15 +3299,10 @@ GeckoDriver.prototype.quit = async function(cmd) { this.deleteSession(); // delay response until the application is about to quit - let quitApplication = new Promise(resolve => { - Services.obs.addObserver( - (subject, topic, data) => resolve(data), - "quit-application"); - }); - + let quitApplication = waitForObserverTopic("quit-application"); Services.startup.quit(mode); - return {cause: await quitApplication}; + return {cause: (await quitApplication).data}; }; GeckoDriver.prototype.installAddon = function(cmd) { diff --git a/testing/marionette/proxy.js b/testing/marionette/proxy.js index 55097c012172..458443563471 100644 --- a/testing/marionette/proxy.js +++ b/testing/marionette/proxy.js @@ -15,7 +15,7 @@ ChromeUtils.import("chrome://marionette/content/evaluate.js"); const {Log} = ChromeUtils.import("chrome://marionette/content/log.js", {}); ChromeUtils.import("chrome://marionette/content/modal.js"); const { - MessageManagerDestroyedPromise, + waitForObserverTopic, } = ChromeUtils.import("chrome://marionette/content/sync.js", {}); this.EXPORTED_SYMBOLS = ["proxy"]; @@ -156,7 +156,9 @@ proxy.AsyncMessageChannel = class { break; } - await new MessageManagerDestroyedPromise(messageManager); + await waitForObserverTopic("message-manager-disconnect", + subject => subject === messageManager); + this.removeHandlers(); resolve(); }; diff --git a/testing/marionette/sync.js b/testing/marionette/sync.js index d88ed8ac02cc..df15d8596609 100644 --- a/testing/marionette/sync.js +++ b/testing/marionette/sync.js @@ -22,12 +22,12 @@ this.EXPORTED_SYMBOLS = [ "executeSoon", "DebounceCallback", "IdlePromise", - "MessageManagerDestroyedPromise", "PollPromise", "Sleep", "TimedPromise", "waitForEvent", "waitForMessage", + "waitForObserverTopic", ]; const {TYPE_ONE_SHOT, TYPE_REPEATING_SLACK} = Ci.nsITimer; @@ -255,46 +255,6 @@ function Sleep(timeout) { return new TimedPromise(() => {}, {timeout, throws: null}); } -/** - * Detects when the specified message manager has been destroyed. - * - * One can observe the removal and detachment of a content browser - * (``) or a chrome window by its message manager - * disconnecting. - * - * When a browser is associated with a tab, this is safer than only - * relying on the event `TabClose` which signalises the _intent to_ - * remove a tab and consequently would lead to the destruction of - * the content browser and its browser message manager. - * - * When closing a chrome window it is safer than only relying on - * the event 'unload' which signalises the _intent to_ close the - * chrome window and consequently would lead to the destruction of - * the window and its window message manager. - * - * @param {MessageListenerManager} messageManager - * The message manager to observe for its disconnect state. - * Use the browser message manager when closing a content browser, - * and the window message manager when closing a chrome window. - * - * @return {Promise} - * A promise that resolves when the message manager has been destroyed. - */ -function MessageManagerDestroyedPromise(messageManager) { - return new Promise(resolve => { - function observe(subject, topic) { - log.trace(`Received observer notification ${topic}`); - - if (subject == messageManager) { - Services.obs.removeObserver(this, "message-manager-disconnect"); - resolve(); - } - } - - Services.obs.addObserver(observe, "message-manager-disconnect"); - }); -} - /** * Throttle until the main thread is idle and `window` has performed * an animation frame (in that order). @@ -511,3 +471,51 @@ function waitForMessage(messageManager, messageName, }); }); } + +/** + * Wait for the specified observer topic to be observed. + * + * This method has been duplicated from TestUtils.jsm. + * + * Because this function is intended for testing, any error in checkFn + * will cause the returned promise to be rejected instead of waiting for + * the next notification, since this is probably a bug in the test. + * + * @param {string} topic + * The topic to observe. + * @param {Object=} options + * Extra options. + * @param {function(String,Object)=} options.checkFn + * Called with ``subject``, and ``data`` as arguments, should return true + * if the notification is the expected one, or false if it should be + * ignored and listening should continue. If not specified, the first + * notification for the specified topic resolves the returned promise. + * + * @return {Promise.>} + * Promise which resolves to an array of ``subject``, and ``data`` from + * the observed notification. + */ +function waitForObserverTopic(topic, {checkFn = null} = {}) { + if (typeof topic != "string") { + throw new TypeError(); + } + if (checkFn != null && typeof checkFn != "function") { + throw new TypeError(); + } + + return new Promise((resolve, reject) => { + Services.obs.addObserver(function observer(subject, topic, data) { + log.trace(`Received observer notification ${topic}`); + try { + if (checkFn && !checkFn(subject, data)) { + return; + } + Services.obs.removeObserver(observer, topic); + resolve({subject, data}); + } catch (ex) { + Services.obs.removeObserver(observer, topic); + reject(ex); + } + }, topic); + }); +} diff --git a/testing/marionette/test/unit/test_sync.js b/testing/marionette/test/unit/test_sync.js index aaae9b9302ec..83efce4fe459 100644 --- a/testing/marionette/test/unit/test_sync.js +++ b/testing/marionette/test/unit/test_sync.js @@ -2,6 +2,8 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ +ChromeUtils.import("resource://gre/modules/Services.jsm"); + const { DebounceCallback, IdlePromise, @@ -10,6 +12,7 @@ const { TimedPromise, waitForEvent, waitForMessage, + waitForObserverTopic, } = ChromeUtils.import("chrome://marionette/content/sync.js", {}); const DEFAULT_TIMEOUT = 2000; @@ -423,3 +426,36 @@ add_task(async function test_waitForMessage_checkFnTypes() { equal(expected_data, await sent); } }); + +add_task(async function test_waitForObserverTopic_topicTypes() { + for (let topic of [42, null, undefined, true, [], {}]) { + Assert.throws(() => waitForObserverTopic(topic), /TypeError/); + } + + let data = {"foo": "bar"}; + let sent = waitForObserverTopic("message"); + Services.obs.notifyObservers(this, "message", data); + let result = await sent; + equal(this, result.subject); + equal(data, result.data); +}); + +add_task(async function test_waitForObserverTopic_checkFnTypes() { + for (let checkFn of ["foo", 42, true, [], {}]) { + Assert.throws(() => waitForObserverTopic( + "message", {checkFn}), /TypeError/); + } + + let data1 = {"fo": "bar"}; + let data2 = {"foo": "bar"}; + + for (let checkFn of [null, undefined, (subject, data) => data == data2]) { + let expected_data = (checkFn == null) ? data1 : data2; + + let sent = waitForObserverTopic("message"); + Services.obs.notifyObservers(this, "message", data1); + Services.obs.notifyObservers(this, "message", data2); + let result = await sent; + equal(expected_data, result.data); + } +}); From 52b84a6f90adf3c59812a32715700a244e2e0afd Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Wed, 5 Dec 2018 19:58:06 +0000 Subject: [PATCH 17/34] Bug 1504756 - [marionette] Added "WebDriver:NewWindow" command to open a new top-level browsing context. r=ato The patch adds the end-point for the recently defined `Create window` command (https://github.com/w3c/webdriver/issues/1138). It allows to open a new top-level browsing context as tab or as window. Depends on D13662 Differential Revision: https://phabricator.services.mozilla.com/D13663 --HG-- extra : moz-landing-system : lando --- testing/marionette/browser.js | 126 ++++++++++++++++++++++++++-------- testing/marionette/driver.js | 76 ++++++++++++++++++-- testing/marionette/server.js | 3 +- 3 files changed, 172 insertions(+), 33 deletions(-) diff --git a/testing/marionette/browser.js b/testing/marionette/browser.js index 4a10661aee61..ad02132f08b8 100644 --- a/testing/marionette/browser.js +++ b/testing/marionette/browser.js @@ -71,11 +71,11 @@ this.Context = Context; */ browser.getBrowserForTab = function(tab) { // Fennec - if ("browser" in tab) { + if (tab && "browser" in tab) { return tab.browser; // Firefox - } else if ("linkedBrowser" in tab) { + } else if (tab && "linkedBrowser" in tab) { return tab.linkedBrowser; } @@ -299,6 +299,50 @@ browser.Context = class { return Promise.all([disconnected, unloaded]); } + /** + * Open a new browser window. + * + * @return {Promise} + * A promise resolving to the newly created chrome window. + */ + async openBrowserWindow(focus = false) { + switch (this.driver.appName) { + case "firefox": + // Open new browser window, and wait until it is fully loaded. + // Also wait for the window to be focused and activated to prevent a + // race condition when promptly focusing to the original window again. + let win = this.window.OpenBrowserWindow(); + + // Bug 1509380 - Missing focus/activate event when Firefox is not + // the top-most application. As such wait for the next tick, and + // manually focus the newly opened window. + win.setTimeout(() => win.focus(), 0); + + let activated = waitForEvent(win, "activate"); + let focused = waitForEvent(win, "focus", {capture: true}); + let startup = waitForObserverTopic("browser-delayed-startup-finished", + subject => subject == win); + await Promise.all([activated, focused, startup]); + + if (!focus) { + // The new window shouldn't get focused. As such set the + // focus back to the currently selected window. + activated = waitForEvent(this.window, "activate"); + focused = waitForEvent(this.window, "focus", {capture: true}); + + this.window.focus(); + + await Promise.all([activated, focused]); + } + + return win; + + default: + throw new UnsupportedOperationError( + `openWindow() not supported in ${this.driver.appName}`); + } + } + /** * Close the current tab. * @@ -325,32 +369,58 @@ browser.Context = class { let tabClosed; - if (this.tabBrowser.closeTab) { - // Fennec - tabClosed = waitForEvent(this.tabBrowser.deck, "TabClose"); - this.tabBrowser.closeTab(this.tab); + switch (this.driver.appName) { + case "fennec": + // Fennec + tabClosed = waitForEvent(this.tabBrowser.deck, "TabClose"); + this.tabBrowser.closeTab(this.tab); + break; - } else if (this.tabBrowser.removeTab) { - // Firefox - tabClosed = waitForEvent(this.tab, "TabClose"); - this.tabBrowser.removeTab(this.tab); + case "firefox": + tabClosed = waitForEvent(this.tab, "TabClose"); + this.tabBrowser.removeTab(this.tab); + break; - } else { - throw new UnsupportedOperationError( - `closeTab() not supported in ${this.driver.appName}`); + default: + throw new UnsupportedOperationError( + `closeTab() not supported in ${this.driver.appName}`); } return Promise.all([disconnected, tabClosed]); } /** - * Opens a tab with given URI. - * - * @param {string} uri - * URI to open. + * Open a new tab in the currently selected chrome window. */ - addTab(uri) { - return this.tabBrowser.addTab(uri, true); + async openTab(focus = false) { + let tab = null; + let tabOpened = waitForEvent(this.window, "TabOpen"); + + switch (this.driver.appName) { + case "fennec": + tab = this.tabBrowser.addTab(null, {selected: focus}); + break; + + case "firefox": + this.window.BrowserOpenTab(); + tab = this.tabBrowser.selectedTab; + + // The new tab is always selected by default. If focus is not wanted, + // the previously tab needs to be selected again. + if (!focus) { + this.tabBrowser.selectedTab = this.tab; + } + + break; + + default: + throw new UnsupportedOperationError( + `openTab() not supported in ${this.driver.appName}`); + } + + await tabOpened; + + return tab; } /** @@ -384,16 +454,18 @@ browser.Context = class { this.tab = this.tabBrowser.tabs[index]; if (focus) { - if (this.tabBrowser.selectTab) { - // Fennec - this.tabBrowser.selectTab(this.tab); + switch (this.driver.appName) { + case "fennec": + this.tabBrowser.selectTab(this.tab); + break; - } else if ("selectedTab" in this.tabBrowser) { - // Firefox - this.tabBrowser.selectedTab = this.tab; + case "firefox": + this.tabBrowser.selectedTab = this.tab; + break; - } else { - throw new UnsupportedOperationError("switchToTab() not supported"); + default: + throw new UnsupportedOperationError( + `switchToTab() not supported in ${this.driver.appName}`); } } } diff --git a/testing/marionette/driver.js b/testing/marionette/driver.js index 395295f99b7c..ac59a7e9d242 100644 --- a/testing/marionette/driver.js +++ b/testing/marionette/driver.js @@ -108,13 +108,12 @@ const globalMessageManager = Services.mm; * * @class GeckoDriver * - * @param {string} appId - * Unique identifier of the application. * @param {MarionetteServer} server * The instance of Marionette server. */ -this.GeckoDriver = function(appId, server) { - this.appId = appId; +this.GeckoDriver = function(server) { + this.appId = Services.appinfo.ID; + this.appName = Services.appinfo.name.toLowerCase(); this._server = server; this.sessionID = null; @@ -1309,6 +1308,7 @@ GeckoDriver.prototype.getIdForBrowser = function(browser) { if (browser === null) { return null; } + let permKey = browser.permanentKey; if (this._browserIds.has(permKey)) { return this._browserIds.get(permKey); @@ -2724,6 +2724,73 @@ GeckoDriver.prototype.deleteCookie = async function(cmd) { } }; +/** + * Open a new top-level browsing context. + * + * @param {string=} type + * Optional type of the new top-level browsing context. Can be one of + * `tab` or `window`. + * @param {boolean=} focus + * Optional flag if the new top-level browsing context should be opened + * in foreground (focused) or background (not focused). + * + * @return {Object.} + * Handle and type of the new browsing context. + */ +GeckoDriver.prototype.newWindow = async function(cmd) { + assert.open(this.getCurrentWindow(Context.Content)); + await this._handleUserPrompts(); + + let focus = false; + if (typeof cmd.parameters.focus != "undefined") { + focus = assert.boolean(cmd.parameters.focus, + pprint`Expected "focus" to be a boolean, got ${cmd.parameters.focus}`); + } + + let type; + if (typeof cmd.parameters.type != "undefined") { + type = assert.string(cmd.parameters.type, + pprint`Expected "type" to be a string, got ${cmd.parameters.type}`); + } + + let types = ["tab", "window"]; + switch (this.appName) { + case "firefox": + if (typeof type == "undefined" || !types.includes(type)) { + type = "window"; + } + break; + case "fennec": + if (typeof type == "undefined" || !types.includes(type)) { + type = "tab"; + } + break; + } + + let contentBrowser; + + switch (type) { + case "tab": + let tab = await this.curBrowser.openTab(focus); + contentBrowser = browser.getBrowserForTab(tab); + break; + + default: + let win = await this.curBrowser.openBrowserWindow(focus); + contentBrowser = browser.getTabBrowser(win).selectedBrowser; + } + + // Even with the framescript registered, the browser might not be known to + // the parent process yet. Wait until it is available. + // TODO: Fix by using `Browser:Init` or equivalent on bug 1311041 + let windowId = await new PollPromise((resolve, reject) => { + let id = this.getIdForBrowser(contentBrowser); + this.windowHandles.includes(id) ? resolve(id) : reject(); + }); + + return {"handle": windowId.toString(), type}; +}; + /** * Close the currently selected tab/window. * @@ -3564,6 +3631,7 @@ GeckoDriver.prototype.commands = { "WebDriver:MaximizeWindow": GeckoDriver.prototype.maximizeWindow, "WebDriver:Navigate": GeckoDriver.prototype.get, "WebDriver:NewSession": GeckoDriver.prototype.newSession, + "WebDriver:NewWindow": GeckoDriver.prototype.newWindow, "WebDriver:PerformActions": GeckoDriver.prototype.performActions, "WebDriver:Refresh": GeckoDriver.prototype.refresh, "WebDriver:ReleaseActions": GeckoDriver.prototype.releaseActions, diff --git a/testing/marionette/server.js b/testing/marionette/server.js index 33a7df51f159..f2cbf756fbfd 100644 --- a/testing/marionette/server.js +++ b/testing/marionette/server.js @@ -11,7 +11,6 @@ const ServerSocket = CC( "nsIServerSocket", "initSpecialConnection"); -ChromeUtils.import("resource://gre/modules/Services.jsm"); ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); ChromeUtils.import("chrome://marionette/content/assert.js"); @@ -74,7 +73,7 @@ class TCPListener { */ driverFactory() { MarionettePrefs.contentListener = false; - return new GeckoDriver(Services.appinfo.ID, this); + return new GeckoDriver(this); } set acceptConnections(value) { From 499ecd94412eb7d59541bfcf26b4bd410143045d Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Wed, 5 Dec 2018 19:58:32 +0000 Subject: [PATCH 18/34] Bug 1504756 - [marionette] Added opening a new browsing context to Marionette client. r=ato The patch updates the Marionette client and all Marionette unit tests to make use of the new `Create Window` command as much as possible. Depends on D13663 Differential Revision: https://phabricator.services.mozilla.com/D13664 --HG-- extra : moz-landing-system : lando --- .../client/marionette_driver/marionette.py | 14 ++ .../runner/mixins/window_manager.py | 47 ++-- .../tests/unit/test_chrome.py | 29 +-- .../tests/unit/test_click.py | 12 +- .../tests/unit/test_key_actions.py | 19 -- .../tests/unit/test_navigation.py | 26 +-- .../tests/unit/test_screenshot.py | 8 +- .../tests/unit/test_switch_window_chrome.py | 76 ++---- .../tests/unit/test_switch_window_content.py | 82 +++---- .../tests/unit/test_window_close_chrome.py | 22 +- .../tests/unit/test_window_close_content.py | 47 ++-- .../tests/unit/test_window_handles_chrome.py | 218 +++++------------- .../tests/unit/test_window_handles_content.py | 72 ++---- .../tests/unit/test_window_management.py | 86 +++---- .../tests/unit/test_window_status_content.py | 55 ++--- 15 files changed, 259 insertions(+), 554 deletions(-) diff --git a/testing/marionette/client/marionette_driver/marionette.py b/testing/marionette/client/marionette_driver/marionette.py index 3f5b09057b71..425e678f77fd 100644 --- a/testing/marionette/client/marionette_driver/marionette.py +++ b/testing/marionette/client/marionette_driver/marionette.py @@ -1428,6 +1428,20 @@ class Marionette(object): return self._send_message("WebDriver:GetPageSource", key="value") + def open(self, type=None, focus=False): + """Open a new window, or tab based on the specified context type. + + If no context type is given the application will choose the best + option based on tab and window support. + + :param type: Type of window to be opened. Can be one of "tab" or "window" + :param focus: If true, the opened window will be focused + + :returns: Dict with new window handle, and type of opened window + """ + body = {"type": type, "focus": focus} + return self._send_message("WebDriver:NewWindow", body) + def close(self): """Close the current window, ending the session if it's the last window currently open. diff --git a/testing/marionette/harness/marionette_harness/runner/mixins/window_manager.py b/testing/marionette/harness/marionette_harness/runner/mixins/window_manager.py index 42172a285a1f..2eccee63f6f1 100644 --- a/testing/marionette/harness/marionette_harness/runner/mixins/window_manager.py +++ b/testing/marionette/harness/marionette_harness/runner/mixins/window_manager.py @@ -6,14 +6,12 @@ from __future__ import absolute_import import sys -from marionette_driver import By, Wait +from marionette_driver import Wait from six import reraise class WindowManagerMixin(object): - _menu_item_new_tab = (By.ID, "menu_newNavigatorTab") - def setUp(self): super(WindowManagerMixin, self).setUp() @@ -60,15 +58,18 @@ class WindowManagerMixin(object): self.marionette.switch_to_window(self.start_window) - def open_tab(self, trigger="menu"): + def open_tab(self, callback=None, focus=False): current_tabs = self.marionette.window_handles try: - if callable(trigger): - trigger() - elif trigger == 'menu': - with self.marionette.using_context("chrome"): - self.marionette.find_element(*self._menu_item_new_tab).click() + if callable(callback): + callback() + else: + result = self.marionette.open(type="tab", focus=focus) + if result["type"] != "tab": + raise Exception( + "Newly opened browsing context is of type {} and not tab.".format( + result["type"])) except Exception: exc, val, tb = sys.exc_info() reraise(exc, 'Failed to trigger opening a new tab: {}'.format(val), tb) @@ -82,8 +83,9 @@ class WindowManagerMixin(object): return new_tab - def open_window(self, trigger=None): + def open_window(self, callback=None, focus=False): current_windows = self.marionette.chrome_window_handles + current_tabs = self.marionette.window_handles def loaded(handle): with self.marionette.using_context("chrome"): @@ -95,11 +97,14 @@ class WindowManagerMixin(object): """, script_args=[handle]) try: - if callable(trigger): - trigger() + if callable(callback): + callback() else: - with self.marionette.using_context("chrome"): - self.marionette.execute_script("OpenBrowserWindow();") + result = self.marionette.open(type="window", focus=focus) + if result["type"] != "window": + raise Exception( + "Newly opened browsing context is of type {} and not window.".format( + result["type"])) except Exception: exc, val, tb = sys.exc_info() reraise(exc, 'Failed to trigger opening a new window: {}'.format(val), tb) @@ -116,9 +121,16 @@ class WindowManagerMixin(object): lambda _: loaded(new_window), message="Window with handle '{}'' did not finish loading".format(new_window)) - return new_window + # Bug 1507771 - Return the correct handle based on the currently selected context + # as long as "WebDriver:NewWindow" is not handled separtely in chrome context + context = self.marionette._send_message("Marionette:GetContext", key="value") + if context == "chrome": + return new_window + elif context == "content": + [new_tab] = list(set(self.marionette.window_handles) - set(current_tabs)) + return new_tab - def open_chrome_window(self, url): + def open_chrome_window(self, url, focus=False): """Open a new chrome window with the specified chrome URL. Can be replaced with "WebDriver:NewWindow" once the command @@ -166,4 +178,5 @@ class WindowManagerMixin(object): })(); """, script_args=(url,)) - return self.open_window(trigger=open_with_js) + with self.marionette.using_context("chrome"): + return self.open_window(callback=open_with_js, focus=focus) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_chrome.py b/testing/marionette/harness/marionette_harness/tests/unit/test_chrome.py index fbe9ec2885a2..ad40ebee7e55 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_chrome.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_chrome.py @@ -1,22 +1,5 @@ -#Copyright 2007-2009 WebDriver committers -#Copyright 2007-2009 Google Inc. -# -#Licensed under the Apache License, Version 2.0 (the "License"); -#you may not use this file except in compliance with the License. -#You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -#Unless required by applicable law or agreed to in writing, software -#distributed under the License is distributed on an "AS IS" BASIS, -#WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -#See the License for the specific language governing permissions and -#limitations under the License. - from __future__ import absolute_import -from marionette_driver import By - from marionette_harness import MarionetteTestCase, WindowManagerMixin @@ -25,18 +8,13 @@ class ChromeTests(WindowManagerMixin, MarionetteTestCase): def setUp(self): super(ChromeTests, self).setUp() - self.marionette.set_context('chrome') - def tearDown(self): self.close_all_windows() super(ChromeTests, self).tearDown() def test_hang_until_timeout(self): - def open_with_menu(): - menu = self.marionette.find_element(By.ID, 'aboutName') - menu.click() - - new_window = self.open_window(trigger=open_with_menu) + with self.marionette.using_context("chrome"): + new_window = self.open_window() self.marionette.switch_to_window(new_window) try: @@ -45,7 +23,8 @@ class ChromeTests(WindowManagerMixin, MarionetteTestCase): # while running this test. Otherwise it would mask eg. IOError as # thrown for a socket timeout. raise NotImplementedError('Exception should not cause a hang when ' - 'closing the chrome window') + 'closing the chrome window in content ' + 'context') finally: self.marionette.close_chrome_window() self.marionette.switch_to_window(self.start_window) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_click.py b/testing/marionette/harness/marionette_harness/tests/unit/test_click.py index d1bde3b2bbf3..9cf5b4a3bbc6 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_click.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_click.py @@ -449,20 +449,16 @@ class TestClickCloseContext(WindowManagerMixin, MarionetteTestCase): super(TestClickCloseContext, self).tearDown() def test_click_close_tab(self): - self.marionette.navigate(self.marionette.absolute_url("windowHandles.html")) - tab = self.open_tab( - lambda: self.marionette.find_element(By.ID, "new-tab").click()) - self.marionette.switch_to_window(tab) + new_tab = self.open_tab() + self.marionette.switch_to_window(new_tab) self.marionette.navigate(self.test_page) self.marionette.find_element(By.ID, "close-window").click() @skip_if_mobile("Fennec doesn't support other chrome windows") def test_click_close_window(self): - self.marionette.navigate(self.marionette.absolute_url("windowHandles.html")) - win = self.open_window( - lambda: self.marionette.find_element(By.ID, "new-window").click()) - self.marionette.switch_to_window(win) + new_tab = self.open_window() + self.marionette.switch_to_window(new_tab) self.marionette.navigate(self.test_page) self.marionette.find_element(By.ID, "close-window").click() diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_key_actions.py b/testing/marionette/harness/marionette_harness/tests/unit/test_key_actions.py index 4a7544b792da..75f89fb250ac 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_key_actions.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_key_actions.py @@ -77,22 +77,3 @@ class TestKeyActions(WindowManagerMixin, MarionetteTestCase): .key_down("x") .perform()) self.assertEqual(self.key_reporter_value, "") - - @skip_if_mobile("Interacting with chrome windows not available for Fennec") - def test_open_in_new_window_shortcut(self): - - def open_window_with_action(): - el = self.marionette.find_element(By.TAG_NAME, "a") - (self.key_action.key_down(Keys.SHIFT) - .press(el) - .release() - .key_up(Keys.SHIFT) - .perform()) - - self.marionette.navigate(inline("Click")) - new_window = self.open_window(trigger=open_window_with_action) - - self.marionette.switch_to_window(new_window) - self.marionette.close_chrome_window() - - self.marionette.switch_to_window(self.start_window) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py b/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py index fa47f81adf95..1c0dc3db0854 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py @@ -55,13 +55,8 @@ class BaseNavigationTestCase(WindowManagerMixin, MarionetteTestCase): else: self.mod_key = Keys.CONTROL - def open_with_link(): - link = self.marionette.find_element(By.ID, "new-blank-tab") - link.click() - # Always use a blank new tab for an empty history - self.marionette.navigate(self.marionette.absolute_url("windowHandles.html")) - self.new_tab = self.open_tab(open_with_link) + self.new_tab = self.open_tab() self.marionette.switch_to_window(self.new_tab) Wait(self.marionette, timeout=self.marionette.timeout.page_load).until( lambda _: self.history_length == 1, @@ -298,7 +293,6 @@ class TestNavigate(BaseNavigationTestCase): focus_el = self.marionette.find_element(By.CSS_SELECTOR, ":focus") self.assertEqual(self.marionette.get_active_element(), focus_el) - @skip_if_mobile("Needs application independent method to open a new tab") def test_no_hang_when_navigating_after_closing_original_tab(self): # Close the start tab self.marionette.switch_to_window(self.start_tab) @@ -340,22 +334,6 @@ class TestNavigate(BaseNavigationTestCase): message="'{}' hasn't been loaded".format(self.test_page_remote)) self.assertTrue(self.is_remote_tab) - @skip_if_mobile("On Android no shortcuts are available") - def test_navigate_shortcut_key(self): - - def open_with_shortcut(): - self.marionette.navigate(self.test_page_remote) - with self.marionette.using_context("chrome"): - main_win = self.marionette.find_element(By.ID, "main-window") - main_win.send_keys(self.mod_key, Keys.SHIFT, "a") - - new_tab = self.open_tab(trigger=open_with_shortcut) - self.marionette.switch_to_window(new_tab) - - Wait(self.marionette, timeout=self.marionette.timeout.page_load).until( - lambda mn: mn.get_url() == "about:addons", - message="'about:addons' hasn't been loaded") - class TestBackForwardNavigation(BaseNavigationTestCase): @@ -823,7 +801,7 @@ class TestPageLoadStrategy(BaseNavigationTestCase): @skip("Bug 1422741 - Causes following tests to fail in loading remote browser") @run_if_e10s("Requires e10s mode enabled") def test_strategy_after_remoteness_change(self): - """Bug 1378191 - Reset of capabilities after listener reload""" + """Bug 1378191 - Reset of capabilities after listener reload.""" self.marionette.delete_session() self.marionette.start_session({"pageLoadStrategy": "eager"}) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py b/testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py index afb2d929404e..5c5928eb4ca7 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_screenshot.py @@ -253,7 +253,8 @@ class TestScreenCaptureChrome(WindowManagerMixin, ScreenCaptureTestCase): chrome_document_element = self.document_element with self.marionette.using_context('content'): - self.assertRaisesRegexp(NoSuchElementException, "Web element reference not seen before", + self.assertRaisesRegexp(NoSuchElementException, + "Web element reference not seen before", self.marionette.screenshot, highlights=[chrome_document_element]) @@ -274,10 +275,9 @@ class TestScreenCaptureContent(WindowManagerMixin, ScreenCaptureTestCase): return [document.body.scrollWidth, document.body.scrollHeight] """)) - @skip_if_mobile("Needs application independent method to open a new tab") def test_capture_tab_already_closed(self): - tab = self.open_tab() - self.marionette.switch_to_window(tab) + new_tab = self.open_tab() + self.marionette.switch_to_window(new_tab) self.marionette.close() self.assertRaises(NoSuchWindowException, self.marionette.screenshot) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_switch_window_chrome.py b/testing/marionette/harness/marionette_harness/tests/unit/test_switch_window_chrome.py index 2ff58cfa35c6..111ae1f970d5 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_switch_window_chrome.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_switch_window_chrome.py @@ -6,9 +6,8 @@ from __future__ import absolute_import import os import sys -from unittest import skipIf -from marionette_driver import By +from unittest import skipIf # add this directory to the path sys.path.append(os.path.dirname(__file__)) @@ -28,119 +27,70 @@ class TestSwitchWindowChrome(TestSwitchToWindowContent): super(TestSwitchWindowChrome, self).tearDown() - def open_window_in_background(self): - with self.marionette.using_context("chrome"): - self.marionette.execute_async_script(""" - let callback = arguments[0]; - (async function() { - function promiseEvent(target, type, args) { - return new Promise(r => { - let params = Object.assign({once: true}, args); - target.addEventListener(type, r, params); - }); - } - function promiseWindowFocus(w) { - return Promise.all([ - promiseEvent(w, "focus", {capture: true}), - promiseEvent(w, "activate"), - ]); - } - // Open a window, wait for it to receive focus - let win = OpenBrowserWindow(); - await promiseWindowFocus(win); - - // Now refocus our original window and wait for that to happen. - let windowFocusPromise = promiseWindowFocus(window); - window.focus(); - return windowFocusPromise; - })().then(() => { - // can't just pass `callback`, as we can't JSON-ify the events it'd get passed. - callback() - }); - """) - - def open_window_in_foreground(self): - with self.marionette.using_context("content"): - self.marionette.navigate(self.test_page) - link = self.marionette.find_element(By.ID, "new-window") - link.click() - + @skipIf(sys.platform.startswith("linux"), + "Bug 1511970 - New window isn't moved to the background on Linux") def test_switch_tabs_for_new_background_window_without_focus_change(self): - # Open an addition tab in the original window so we can better check + # Open an additional tab in the original window so we can better check # the selected index in thew new window to be opened. - second_tab = self.open_tab(trigger=self.open_tab_in_foreground) + second_tab = self.open_tab(focus=True) self.marionette.switch_to_window(second_tab, focus=True) second_tab_index = self.get_selected_tab_index() self.assertNotEqual(second_tab_index, self.selected_tab_index) - # Opens a new background window, but we are interested in the tab - tab_in_new_window = self.open_tab(trigger=self.open_window_in_background) + # Open a new background window, but we are interested in the tab + with self.marionette.using_context("content"): + tab_in_new_window = self.open_window() self.assertEqual(self.marionette.current_window_handle, second_tab) self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) self.assertEqual(self.get_selected_tab_index(), second_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.empty_page) # Switch to the tab in the new window but don't focus it self.marionette.switch_to_window(tab_in_new_window, focus=False) self.assertEqual(self.marionette.current_window_handle, tab_in_new_window) self.assertNotEqual(self.marionette.current_chrome_window_handle, self.start_window) self.assertEqual(self.get_selected_tab_index(), second_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), "about:blank") def test_switch_tabs_for_new_foreground_window_with_focus_change(self): # Open an addition tab in the original window so we can better check # the selected index in thew new window to be opened. - second_tab = self.open_tab(trigger=self.open_tab_in_foreground) + second_tab = self.open_tab() self.marionette.switch_to_window(second_tab, focus=True) second_tab_index = self.get_selected_tab_index() self.assertNotEqual(second_tab_index, self.selected_tab_index) # Opens a new window, but we are interested in the tab - tab_in_new_window = self.open_tab(trigger=self.open_window_in_foreground) + with self.marionette.using_context("content"): + tab_in_new_window = self.open_window(focus=True) self.assertEqual(self.marionette.current_window_handle, second_tab) self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) self.assertNotEqual(self.get_selected_tab_index(), second_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) self.marionette.switch_to_window(tab_in_new_window) self.assertEqual(self.marionette.current_window_handle, tab_in_new_window) self.assertNotEqual(self.marionette.current_chrome_window_handle, self.start_window) self.assertNotEqual(self.get_selected_tab_index(), second_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.empty_page) self.marionette.switch_to_window(second_tab, focus=True) self.assertEqual(self.marionette.current_window_handle, second_tab) self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) # Bug 1335085 - The focus doesn't change even as requested so. # self.assertEqual(self.get_selected_tab_index(), second_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) def test_switch_tabs_for_new_foreground_window_without_focus_change(self): # Open an addition tab in the original window so we can better check # the selected index in thew new window to be opened. - second_tab = self.open_tab(trigger=self.open_tab_in_foreground) + second_tab = self.open_tab() self.marionette.switch_to_window(second_tab, focus=True) second_tab_index = self.get_selected_tab_index() self.assertNotEqual(second_tab_index, self.selected_tab_index) - # Opens a new window, but we are interested in the tab which automatically - # gets the focus. - self.open_tab(trigger=self.open_window_in_foreground) + self.open_window(focus=True) self.assertEqual(self.marionette.current_window_handle, second_tab) self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) self.assertNotEqual(self.get_selected_tab_index(), second_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) # Switch to the second tab in the first window, but don't focus it. self.marionette.switch_to_window(second_tab, focus=False) self.assertEqual(self.marionette.current_window_handle, second_tab) self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) self.assertNotEqual(self.get_selected_tab_index(), second_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_switch_window_content.py b/testing/marionette/harness/marionette_harness/tests/unit/test_switch_window_content.py index ccf5c1262952..e8ff9b25dc7d 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_switch_window_content.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_switch_window_content.py @@ -4,10 +4,10 @@ from __future__ import absolute_import -from marionette_driver import Actions, By, Wait +from marionette_driver import By from marionette_driver.keys import Keys -from marionette_harness import MarionetteTestCase, skip_if_mobile, WindowManagerMixin +from marionette_harness import MarionetteTestCase, WindowManagerMixin class TestSwitchToWindowContent(WindowManagerMixin, MarionetteTestCase): @@ -20,14 +20,8 @@ class TestSwitchToWindowContent(WindowManagerMixin, MarionetteTestCase): else: self.mod_key = Keys.CONTROL - self.empty_page = self.marionette.absolute_url("empty.html") - self.test_page = self.marionette.absolute_url("windowHandles.html") - self.selected_tab_index = self.get_selected_tab_index() - with self.marionette.using_context("content"): - self.marionette.navigate(self.test_page) - def tearDown(self): self.close_all_tabs() @@ -69,78 +63,51 @@ class TestSwitchToWindowContent(WindowManagerMixin, MarionetteTestCase): } """) - def open_tab_in_background(self): - with self.marionette.using_context("content"): - link = self.marionette.find_element(By.ID, "new-tab") - - action = Actions(self.marionette) - action.key_down(self.mod_key).click(link).perform() - - def open_tab_in_foreground(self): - with self.marionette.using_context("content"): - link = self.marionette.find_element(By.ID, "new-tab") - link.click() - def test_switch_tabs_with_focus_change(self): - new_tab = self.open_tab(self.open_tab_in_foreground) + new_tab = self.open_tab(focus=True) self.assertEqual(self.marionette.current_window_handle, self.start_tab) self.assertNotEqual(self.get_selected_tab_index(), self.selected_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) + # Switch to new tab first because it is already selected self.marionette.switch_to_window(new_tab) self.assertEqual(self.marionette.current_window_handle, new_tab) self.assertNotEqual(self.get_selected_tab_index(), self.selected_tab_index) - with self.marionette.using_context("content"): - Wait(self.marionette, timeout=self.marionette.timeout.page_load).until( - lambda _: self.marionette.get_url() == self.empty_page, - message="{} has been loaded in the newly opened tab.".format(self.empty_page)) - + # Switch to original tab by explicitely setting the focus self.marionette.switch_to_window(self.start_tab, focus=True) self.assertEqual(self.marionette.current_window_handle, self.start_tab) self.assertEqual(self.get_selected_tab_index(), self.selected_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) self.marionette.switch_to_window(new_tab) self.marionette.close() - self.marionette.switch_to_window(self.start_tab) + self.marionette.switch_to_window(self.start_tab) self.assertEqual(self.marionette.current_window_handle, self.start_tab) self.assertEqual(self.get_selected_tab_index(), self.selected_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) def test_switch_tabs_without_focus_change(self): - new_tab = self.open_tab(self.open_tab_in_foreground) + new_tab = self.open_tab(focus=True) self.assertEqual(self.marionette.current_window_handle, self.start_tab) self.assertNotEqual(self.get_selected_tab_index(), self.selected_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) # Switch to new tab first because it is already selected self.marionette.switch_to_window(new_tab) self.assertEqual(self.marionette.current_window_handle, new_tab) + # Switch to original tab by explicitely not setting the focus self.marionette.switch_to_window(self.start_tab, focus=False) self.assertEqual(self.marionette.current_window_handle, self.start_tab) self.assertNotEqual(self.get_selected_tab_index(), self.selected_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) - self.marionette.switch_to_window(new_tab) self.marionette.close() self.marionette.switch_to_window(self.start_tab) self.assertEqual(self.marionette.current_window_handle, self.start_tab) self.assertEqual(self.get_selected_tab_index(), self.selected_tab_index) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) def test_switch_from_content_to_chrome_window_should_not_change_selected_tab(self): - new_tab = self.open_tab(self.open_tab_in_foreground) + new_tab = self.open_tab(focus=True) self.marionette.switch_to_window(new_tab) self.assertEqual(self.marionette.current_window_handle, new_tab) @@ -150,24 +117,31 @@ class TestSwitchToWindowContent(WindowManagerMixin, MarionetteTestCase): self.assertEqual(self.marionette.current_window_handle, new_tab) self.assertEqual(self.get_selected_tab_index(), new_tab_index) - @skip_if_mobile("New windows not supported in Fennec") - def test_switch_to_new_private_browsing_window_has_to_register_browsers(self): + def test_switch_to_new_private_browsing_tab(self): # Test that tabs (browsers) are correctly registered for a newly opened - # private browsing window. This has to also happen without explicitely + # private browsing window/tab. This has to also happen without explicitely # switching to the tab itself before using any commands in content scope. # # Note: Not sure why this only affects private browsing windows only. + new_tab = self.open_tab(focus=True) + self.marionette.switch_to_window(new_tab) - def open_private_browsing_window(): + def open_private_browsing_window_firefox(): with self.marionette.using_context("content"): - self.marionette.navigate("about:privatebrowsing") - button = self.marionette.find_element(By.ID, "startPrivateBrowsing") - button.click() + self.marionette.find_element(By.ID, "startPrivateBrowsing").click() - new_window = self.open_window(open_private_browsing_window) - self.marionette.switch_to_window(new_window) - self.assertEqual(self.marionette.current_chrome_window_handle, new_window) - self.assertNotEqual(self.marionette.current_window_handle, self.start_tab) + def open_private_browsing_tab_fennec(): + with self.marionette.using_context("content"): + self.marionette.find_element(By.ID, "newPrivateTabLink").click() with self.marionette.using_context("content"): - self.marionette.execute_script(" return true; ") + self.marionette.navigate("about:privatebrowsing") + if self.marionette.session_capabilities["browserName"] == "fennec": + new_pb_tab = self.open_tab(open_private_browsing_tab_fennec) + else: + new_pb_tab = self.open_tab(open_private_browsing_window_firefox) + + self.marionette.switch_to_window(new_pb_tab) + self.assertEqual(self.marionette.current_window_handle, new_pb_tab) + + self.marionette.execute_script(" return true; ") diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_chrome.py b/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_chrome.py index ec13c5a4ed19..79d9d6ddefea 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_chrome.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_chrome.py @@ -21,14 +21,14 @@ class TestCloseWindow(WindowManagerMixin, MarionetteTestCase): super(TestCloseWindow, self).tearDown() def test_close_chrome_window_for_browser_window(self): - win = self.open_window() - self.marionette.switch_to_window(win) + new_window = self.open_window() + self.marionette.switch_to_window(new_window) - self.assertNotIn(win, self.marionette.window_handles) + self.assertNotIn(new_window, self.marionette.window_handles) chrome_window_handles = self.marionette.close_chrome_window() - self.assertNotIn(win, chrome_window_handles) + self.assertNotIn(new_window, chrome_window_handles) self.assertListEqual(self.start_windows, chrome_window_handles) - self.assertNotIn(win, self.marionette.window_handles) + self.assertNotIn(new_window, self.marionette.window_handles) def test_close_chrome_window_for_non_browser_window(self): win = self.open_chrome_window("chrome://marionette/content/test.xul") @@ -50,20 +50,20 @@ class TestCloseWindow(WindowManagerMixin, MarionetteTestCase): self.assertIsNotNone(self.marionette.session) def test_close_window_for_browser_tab(self): - tab = self.open_tab() - self.marionette.switch_to_window(tab) + new_tab = self.open_tab() + self.marionette.switch_to_window(new_tab) window_handles = self.marionette.close() - self.assertNotIn(tab, window_handles) + self.assertNotIn(new_tab, window_handles) self.assertListEqual(self.start_tabs, window_handles) def test_close_window_for_browser_window_with_single_tab(self): - win = self.open_window() - self.marionette.switch_to_window(win) + new_window = self.open_window() + self.marionette.switch_to_window(new_window) self.assertEqual(len(self.start_tabs) + 1, len(self.marionette.window_handles)) window_handles = self.marionette.close() - self.assertNotIn(win, window_handles) + self.assertNotIn(new_window, window_handles) self.assertListEqual(self.start_tabs, window_handles) self.assertListEqual(self.start_windows, self.marionette.chrome_window_handles) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_content.py b/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_content.py index 4107ae235233..46b4f6ba4a63 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_content.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_content.py @@ -24,26 +24,27 @@ class TestCloseWindow(WindowManagerMixin, MarionetteTestCase): @skip_if_mobile("Interacting with chrome windows not available for Fennec") def test_close_chrome_window_for_browser_window(self): - win = self.open_window() - self.marionette.switch_to_window(win) + with self.marionette.using_context("chrome"): + new_window = self.open_window() + self.marionette.switch_to_window(new_window) - self.assertNotIn(win, self.marionette.window_handles) + self.assertIn(new_window, self.marionette.chrome_window_handles) chrome_window_handles = self.marionette.close_chrome_window() - self.assertNotIn(win, chrome_window_handles) + self.assertNotIn(new_window, chrome_window_handles) self.assertListEqual(self.start_windows, chrome_window_handles) - self.assertNotIn(win, self.marionette.window_handles) + self.assertNotIn(new_window, self.marionette.window_handles) @skip_if_mobile("Interacting with chrome windows not available for Fennec") def test_close_chrome_window_for_non_browser_window(self): - win = self.open_chrome_window("chrome://marionette/content/test.xul") - self.marionette.switch_to_window(win) + new_window = self.open_chrome_window("chrome://marionette/content/test.xul") + self.marionette.switch_to_window(new_window) - self.assertIn(win, self.marionette.chrome_window_handles) - self.assertNotIn(win, self.marionette.window_handles) + self.assertIn(new_window, self.marionette.chrome_window_handles) + self.assertNotIn(new_window, self.marionette.window_handles) chrome_window_handles = self.marionette.close_chrome_window() - self.assertNotIn(win, chrome_window_handles) + self.assertNotIn(new_window, chrome_window_handles) self.assertListEqual(self.start_windows, chrome_window_handles) - self.assertNotIn(win, self.marionette.window_handles) + self.assertNotIn(new_window, self.marionette.window_handles) @skip_if_mobile("Interacting with chrome windows not available for Fennec") def test_close_chrome_window_for_last_open_window(self): @@ -54,19 +55,17 @@ class TestCloseWindow(WindowManagerMixin, MarionetteTestCase): self.assertListEqual([self.start_window], self.marionette.chrome_window_handles) self.assertIsNotNone(self.marionette.session) - @skip_if_mobile("Needs application independent method to open a new tab") def test_close_window_for_browser_tab(self): - tab = self.open_tab() - self.marionette.switch_to_window(tab) + new_tab = self.open_tab() + self.marionette.switch_to_window(new_tab) window_handles = self.marionette.close() - self.assertNotIn(tab, window_handles) + self.assertNotIn(new_tab, window_handles) self.assertListEqual(self.start_tabs, window_handles) - @skip_if_mobile("Needs application independent method to open a new tab") def test_close_window_with_dismissed_beforeunload_prompt(self): - tab = self.open_tab() - self.marionette.switch_to_window(tab) + new_tab = self.open_tab() + self.marionette.switch_to_window(new_tab) self.marionette.navigate(inline(""" @@ -82,12 +81,12 @@ class TestCloseWindow(WindowManagerMixin, MarionetteTestCase): @skip_if_mobile("Interacting with chrome windows not available for Fennec") def test_close_window_for_browser_window_with_single_tab(self): - win = self.open_window() - self.marionette.switch_to_window(win) + new_tab = self.open_window() + self.marionette.switch_to_window(new_tab) - self.assertEqual(len(self.start_tabs) + 1, len(self.marionette.window_handles)) + self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1) window_handles = self.marionette.close() - self.assertNotIn(win, window_handles) + self.assertNotIn(new_tab, window_handles) self.assertListEqual(self.start_tabs, window_handles) self.assertListEqual(self.start_windows, self.marionette.chrome_window_handles) @@ -104,8 +103,8 @@ class TestCloseWindow(WindowManagerMixin, MarionetteTestCase): self.close_all_tabs() test_page = self.marionette.absolute_url("windowHandles.html") - tab = self.open_tab() - self.marionette.switch_to_window(tab) + new_tab = self.open_tab() + self.marionette.switch_to_window(new_tab) self.marionette.navigate(test_page) self.marionette.switch_to_window(self.start_tab) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_window_handles_chrome.py b/testing/marionette/harness/marionette_harness/tests/unit/test_window_handles_chrome.py index c850a0286757..b55cc45f9383 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_window_handles_chrome.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_window_handles_chrome.py @@ -6,7 +6,7 @@ from __future__ import absolute_import import types -from marionette_driver import By, errors, Wait +from marionette_driver import errors from marionette_harness import MarionetteTestCase, WindowManagerMixin @@ -16,9 +16,7 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase): def setUp(self): super(TestWindowHandles, self).setUp() - self.empty_page = self.marionette.absolute_url("empty.html") - self.test_page = self.marionette.absolute_url("windowHandles.html") - self.marionette.navigate(self.test_page) + self.xul_dialog = "chrome://marionette/content/test_dialog.xul" self.marionette.set_context("chrome") @@ -42,17 +40,16 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase): self.assertIsInstance(handle, types.StringTypes) def test_chrome_window_handles_with_scopes(self): - # Open a browser and a non-browser (about window) chrome window - self.open_window( - trigger=lambda: self.marionette.execute_script("OpenBrowserWindow();")) + new_browser = self.open_window() self.assert_window_handles() self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows) + 1) + self.assertIn(new_browser, self.marionette.chrome_window_handles) self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) - self.open_window( - trigger=lambda: self.marionette.find_element(By.ID, "aboutName").click()) + new_dialog = self.open_chrome_window(self.xul_dialog) self.assert_window_handles() self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows) + 2) + self.assertIn(new_dialog, self.marionette.chrome_window_handles) self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) chrome_window_handles_in_chrome_scope = self.marionette.chrome_window_handles @@ -64,117 +61,112 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase): self.assertEqual(self.marionette.window_handles, window_handles_in_chrome_scope) - def test_chrome_window_handles_after_opening_new_dialog(self): - xul_dialog = "chrome://marionette/content/test_dialog.xul" - new_win = self.open_chrome_window(xul_dialog) + def test_chrome_window_handles_after_opening_new_chrome_window(self): + new_window = self.open_chrome_window(self.xul_dialog) self.assert_window_handles() self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows) + 1) + self.assertIn(new_window, self.marionette.chrome_window_handles) self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) - # Check that the new tab has the correct page loaded - self.marionette.switch_to_window(new_win) + # Check that the new chrome window has the correct URL loaded + self.marionette.switch_to_window(new_window) self.assert_window_handles() - self.assertEqual(self.marionette.current_chrome_window_handle, new_win) - self.assertEqual(self.marionette.get_url(), xul_dialog) + self.assertEqual(self.marionette.current_chrome_window_handle, new_window) + self.assertEqual(self.marionette.get_url(), self.xul_dialog) - # Close the opened dialog and carry on in our original tab. + # Close the chrome window, and carry on in our original window. self.marionette.close_chrome_window() self.assert_window_handles() self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows)) + self.assertNotIn(new_window, self.marionette.chrome_window_handles) self.marionette.switch_to_window(self.start_window) self.assert_window_handles() self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) def test_chrome_window_handles_after_opening_new_window(self): - def open_with_link(): - with self.marionette.using_context("content"): - link = self.marionette.find_element(By.ID, "new-window") - link.click() - - # We open a new window but are actually interested in the new tab - new_win = self.open_window(trigger=open_with_link) + new_window = self.open_window() self.assert_window_handles() self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows) + 1) + self.assertIn(new_window, self.marionette.chrome_window_handles) self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) - # Check that the new tab has the correct page loaded - self.marionette.switch_to_window(new_win) + self.marionette.switch_to_window(new_window) self.assert_window_handles() - self.assertEqual(self.marionette.current_chrome_window_handle, new_win) - with self.marionette.using_context("content"): - Wait(self.marionette, timeout=self.marionette.timeout.page_load).until( - lambda mn: mn.get_url() == self.empty_page, - message="{} did not load after opening a new tab".format(self.empty_page)) + self.assertEqual(self.marionette.current_chrome_window_handle, new_window) - # Ensure navigate works in our current window - other_page = self.marionette.absolute_url("test.html") - with self.marionette.using_context("content"): - self.marionette.navigate(other_page) - self.assertEqual(self.marionette.get_url(), other_page) - - # Close the opened window and carry on in our original tab. + # Close the opened window and carry on in our original window. self.marionette.close() self.assert_window_handles() self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows)) + self.assertNotIn(new_window, self.marionette.chrome_window_handles) self.marionette.switch_to_window(self.start_window) self.assert_window_handles() self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) def test_window_handles_after_opening_new_tab(self): - def open_with_link(): - with self.marionette.using_context("content"): - link = self.marionette.find_element(By.ID, "new-tab") - link.click() - - new_tab = self.open_tab(trigger=open_with_link) + with self.marionette.using_context("content"): + new_tab = self.open_tab() self.assert_window_handles() self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1) + self.assertIn(new_tab, self.marionette.window_handles) self.assertEqual(self.marionette.current_window_handle, self.start_tab) self.marionette.switch_to_window(new_tab) self.assert_window_handles() self.assertEqual(self.marionette.current_window_handle, new_tab) - with self.marionette.using_context("content"): - Wait(self.marionette, timeout=self.marionette.timeout.page_load).until( - lambda mn: mn.get_url() == self.empty_page, - message="{} did not load after opening a new tab".format(self.empty_page)) - - # Ensure navigate works in our current tab - other_page = self.marionette.absolute_url("test.html") - with self.marionette.using_context("content"): - self.marionette.navigate(other_page) - self.assertEqual(self.marionette.get_url(), other_page) self.marionette.switch_to_window(self.start_tab) self.assert_window_handles() self.assertEqual(self.marionette.current_window_handle, self.start_tab) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) self.marionette.switch_to_window(new_tab) self.marionette.close() self.assert_window_handles() self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs)) + self.assertNotIn(new_tab, self.marionette.window_handles) self.marionette.switch_to_window(self.start_tab) + self.assert_window_handles() self.assertEqual(self.marionette.current_window_handle, self.start_tab) - def test_window_handles_after_opening_new_dialog(self): - xul_dialog = "chrome://marionette/content/test_dialog.xul" - new_win = self.open_chrome_window(xul_dialog) + def test_window_handles_after_opening_new_foreground_tab(self): + with self.marionette.using_context("content"): + new_tab = self.open_tab(focus=True) + self.assert_window_handles() + self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1) + self.assertIn(new_tab, self.marionette.window_handles) + self.assertEqual(self.marionette.current_window_handle, self.start_tab) + + # We still have the default tab set as our window handle. This + # get_url command should be sent immediately, and not be forever-queued. + with self.marionette.using_context("content"): + self.marionette.get_url() + + self.marionette.switch_to_window(new_tab) + self.assert_window_handles() + self.assertEqual(self.marionette.current_window_handle, new_tab) + + self.marionette.close() self.assert_window_handles() self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs)) + self.assertNotIn(new_tab, self.marionette.window_handles) + + self.marionette.switch_to_window(self.start_tab) + self.assert_window_handles() self.assertEqual(self.marionette.current_window_handle, self.start_tab) - self.marionette.switch_to_window(new_win) + def test_window_handles_after_opening_new_chrome_window(self): + new_window = self.open_chrome_window(self.xul_dialog) self.assert_window_handles() - self.assertEqual(self.marionette.get_url(), xul_dialog) + self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs)) + self.assertNotIn(new_window, self.marionette.window_handles) + self.assertEqual(self.marionette.current_window_handle, self.start_tab) + + self.marionette.switch_to_window(new_window) + self.assert_window_handles() + self.assertEqual(self.marionette.get_url(), self.xul_dialog) # Check that the opened dialog is not accessible via window handles with self.assertRaises(errors.NoSuchWindowException): @@ -190,111 +182,23 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase): self.marionette.switch_to_window(self.start_tab) self.assert_window_handles() self.assertEqual(self.marionette.current_window_handle, self.start_tab) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) - - def test_window_handles_after_opening_new_window(self): - def open_with_link(): - with self.marionette.using_context("content"): - link = self.marionette.find_element(By.ID, "new-window") - link.click() - - # We open a new window but are actually interested in the new tab - new_tab = self.open_tab(trigger=open_with_link) - self.assert_window_handles() - self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1) - self.assertEqual(self.marionette.current_window_handle, self.start_tab) - - # Check that the new tab has the correct page loaded - self.marionette.switch_to_window(new_tab) - self.assert_window_handles() - self.assertEqual(self.marionette.current_window_handle, new_tab) - with self.marionette.using_context("content"): - Wait(self.marionette, timeout=self.marionette.timeout.page_load).until( - lambda mn: mn.get_url() == self.empty_page, - message="{} did not load after opening a new tab".format(self.empty_page)) - - # Ensure navigate works in our current window - other_page = self.marionette.absolute_url("test.html") - with self.marionette.using_context("content"): - self.marionette.navigate(other_page) - self.assertEqual(self.marionette.get_url(), other_page) - - # Close the opened window and carry on in our original tab. - self.marionette.close() - self.assert_window_handles() - self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs)) - - self.marionette.switch_to_window(self.start_tab) - self.assert_window_handles() - self.assertEqual(self.marionette.current_window_handle, self.start_tab) - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) def test_window_handles_after_closing_original_tab(self): - def open_with_link(): - with self.marionette.using_context("content"): - link = self.marionette.find_element(By.ID, "new-tab") - link.click() - - new_tab = self.open_tab(trigger=open_with_link) + with self.marionette.using_context("content"): + new_tab = self.open_tab() self.assert_window_handles() self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1) + self.assertIn(new_tab, self.marionette.window_handles) self.assertEqual(self.marionette.current_window_handle, self.start_tab) self.marionette.close() self.assert_window_handles() self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs)) + self.assertIn(new_tab, self.marionette.window_handles) self.marionette.switch_to_window(new_tab) self.assert_window_handles() self.assertEqual(self.marionette.current_window_handle, new_tab) - with self.marionette.using_context("content"): - Wait(self.marionette, timeout=self.marionette.timeout.page_load).until( - lambda mn: mn.get_url() == self.empty_page, - message="{} did not load after opening a new tab".format(self.empty_page)) - - def test_window_handles_no_switch(self): - """Regression test for bug 1294456. - This test is testing the case where Marionette attempts to send a - command to a window handle when the browser has opened and selected - a new tab. Before bug 1294456 landed, the Marionette driver was getting - confused about which window handle the client cared about, and assumed - it was the window handle for the newly opened and selected tab. - - This caused Marionette to think that the browser needed to do a remoteness - flip in the e10s case, since the tab opened by menu_newNavigatorTab is - about:newtab (which is currently non-remote). This meant that commands - sent to what should have been the original window handle would be - queued and never sent, since the remoteness flip in the new tab was - never going to happen. - """ - def open_with_menu(): - menu_new_tab = self.marionette.find_element(By.ID, 'menu_newNavigatorTab') - menu_new_tab.click() - - new_tab = self.open_tab(trigger=open_with_menu) - self.assert_window_handles() - - # We still have the default tab set as our window handle. This - # get_url command should be sent immediately, and not be forever-queued. - with self.marionette.using_context("content"): - self.assertEqual(self.marionette.get_url(), self.test_page) - - self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1) - self.assertEqual(self.marionette.current_window_handle, self.start_tab) - - self.marionette.switch_to_window(new_tab) - self.assert_window_handles() - self.assertEqual(self.marionette.current_window_handle, new_tab) - - self.marionette.close() - self.assert_window_handles() - self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs)) - - self.marionette.switch_to_window(self.start_tab) - self.assert_window_handles() - self.assertEqual(self.marionette.current_window_handle, self.start_tab) def test_window_handles_after_closing_last_window(self): self.close_all_windows() diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_window_handles_content.py b/testing/marionette/harness/marionette_harness/tests/unit/test_window_handles_content.py index 2a19bdb82fa8..bc95f9a30be4 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_window_handles_content.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_window_handles_content.py @@ -7,7 +7,7 @@ from __future__ import absolute_import import types import urllib -from marionette_driver import By, errors, Wait +from marionette_driver import errors from marionette_harness import MarionetteTestCase, skip_if_mobile, WindowManagerMixin @@ -21,9 +21,7 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase): def setUp(self): super(TestWindowHandles, self).setUp() - self.empty_page = self.marionette.absolute_url("empty.html") - self.test_page = self.marionette.absolute_url("windowHandles.html") - self.marionette.navigate(self.test_page) + self.xul_dialog = "chrome://marionette/content/test_dialog.xul" def tearDown(self): self.close_all_tabs() @@ -39,12 +37,8 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase): for handle in self.marionette.window_handles: self.assertIsInstance(handle, types.StringTypes) - def test_window_handles_after_opening_new_tab(self): - def open_with_link(): - link = self.marionette.find_element(By.ID, "new-tab") - link.click() - - new_tab = self.open_tab(trigger=open_with_link) + def tst_window_handles_after_opening_new_tab(self): + new_tab = self.open_tab() self.assert_window_handles() self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1) self.assertEqual(self.marionette.current_window_handle, self.start_tab) @@ -52,13 +46,9 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase): self.marionette.switch_to_window(new_tab) self.assert_window_handles() self.assertEqual(self.marionette.current_window_handle, new_tab) - Wait(self.marionette, timeout=self.marionette.timeout.page_load).until( - lambda mn: mn.get_url() == self.empty_page, - message="{} did not load after opening a new tab".format(self.empty_page)) self.marionette.switch_to_window(self.start_tab) self.assertEqual(self.marionette.current_window_handle, self.start_tab) - self.assertEqual(self.marionette.get_url(), self.test_page) self.marionette.switch_to_window(new_tab) self.marionette.close() @@ -69,29 +59,15 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase): self.assert_window_handles() self.assertEqual(self.marionette.current_window_handle, self.start_tab) - def test_window_handles_after_opening_new_browser_window(self): - def open_with_link(): - link = self.marionette.find_element(By.ID, "new-window") - link.click() - - # We open a new window but are actually interested in the new tab - new_tab = self.open_tab(trigger=open_with_link) + def tst_window_handles_after_opening_new_browser_window(self): + new_tab = self.open_window() self.assert_window_handles() self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1) self.assertEqual(self.marionette.current_window_handle, self.start_tab) - # Check that the new tab has the correct page loaded self.marionette.switch_to_window(new_tab) self.assert_window_handles() self.assertEqual(self.marionette.current_window_handle, new_tab) - Wait(self.marionette, self.marionette.timeout.page_load).until( - lambda _: self.marionette.get_url() == self.empty_page, - message="The expected page '{}' has not been loaded".format(self.empty_page)) - - # Ensure navigate works in our current window - other_page = self.marionette.absolute_url("test.html") - self.marionette.navigate(other_page) - self.assertEqual(self.marionette.get_url(), other_page) # Close the opened window and carry on in our original tab. self.marionette.close() @@ -101,31 +77,16 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase): self.marionette.switch_to_window(self.start_tab) self.assert_window_handles() self.assertEqual(self.marionette.current_window_handle, self.start_tab) - self.assertEqual(self.marionette.get_url(), self.test_page) @skip_if_mobile("Fennec doesn't support other chrome windows") - def test_window_handles_after_opening_new_non_browser_window(self): - def open_with_link(): - self.marionette.navigate(inline(""" - Download - - - """)) - link = self.marionette.find_element(By.ID, "blob-download") - link.click() - - new_win = self.open_window(trigger=open_with_link) + def tst_window_handles_after_opening_new_non_browser_window(self): + new_window = self.open_chrome_window(self.xul_dialog) self.assert_window_handles() self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs)) self.assertEqual(self.marionette.current_window_handle, self.start_tab) + self.assertNotIn(new_window, self.marionette.window_handles) - self.marionette.switch_to_window(new_win) + self.marionette.switch_to_window(new_window) self.assert_window_handles() # Check that the opened window is not accessible via window handles @@ -144,26 +105,21 @@ class TestWindowHandles(WindowManagerMixin, MarionetteTestCase): self.assertEqual(self.marionette.current_window_handle, self.start_tab) def test_window_handles_after_closing_original_tab(self): - def open_with_link(): - link = self.marionette.find_element(By.ID, "new-tab") - link.click() - - new_tab = self.open_tab(trigger=open_with_link) + new_tab = self.open_tab() self.assert_window_handles() self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs) + 1) self.assertEqual(self.marionette.current_window_handle, self.start_tab) + self.assertIn(new_tab, self.marionette.window_handles) self.marionette.close() self.assert_window_handles() self.assertEqual(len(self.marionette.window_handles), len(self.start_tabs)) + self.assertNotIn(self.start_tab, self.marionette.window_handles) self.marionette.switch_to_window(new_tab) self.assert_window_handles() self.assertEqual(self.marionette.current_window_handle, new_tab) - Wait(self.marionette, self.marionette.timeout.page_load).until( - lambda _: self.marionette.get_url() == self.empty_page, - message="The expected page '{}' has not been loaded".format(self.empty_page)) - def test_window_handles_after_closing_last_tab(self): + def tst_window_handles_after_closing_last_tab(self): self.close_all_tabs() self.assertEqual(self.marionette.close(), []) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_window_management.py b/testing/marionette/harness/marionette_harness/tests/unit/test_window_management.py index 5923c55935aa..0a0062b02868 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_window_management.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_window_management.py @@ -21,15 +21,9 @@ class TestNoSuchWindowContent(WindowManagerMixin, MarionetteTestCase): @skip_if_mobile("Fennec doesn't support other chrome windows") def test_closed_chrome_window(self): - - def open_with_link(): - with self.marionette.using_context("content"): - test_page = self.marionette.absolute_url("windowHandles.html") - self.marionette.navigate(test_page) - self.marionette.find_element(By.ID, "new-window").click() - - win = self.open_window(open_with_link) - self.marionette.switch_to_window(win) + with self.marionette.using_context("chrome"): + new_window = self.open_window() + self.marionette.switch_to_window(new_window) self.marionette.close_chrome_window() # When closing a browser window both handles are not available @@ -43,12 +37,12 @@ class TestNoSuchWindowContent(WindowManagerMixin, MarionetteTestCase): self.marionette.switch_to_window(self.start_window) with self.assertRaises(NoSuchWindowException): - self.marionette.switch_to_window(win) + self.marionette.switch_to_window(new_window) @skip_if_mobile("Fennec doesn't support other chrome windows") def test_closed_chrome_window_while_in_frame(self): - win = self.open_chrome_window("chrome://marionette/content/test.xul") - self.marionette.switch_to_window(win) + new_window = self.open_chrome_window("chrome://marionette/content/test.xul") + self.marionette.switch_to_window(new_window) with self.marionette.using_context("chrome"): self.marionette.switch_to_frame("iframe") self.marionette.close_chrome_window() @@ -61,13 +55,12 @@ class TestNoSuchWindowContent(WindowManagerMixin, MarionetteTestCase): self.marionette.switch_to_window(self.start_window) with self.assertRaises(NoSuchWindowException): - self.marionette.switch_to_window(win) + self.marionette.switch_to_window(new_window) def test_closed_tab(self): - with self.marionette.using_context("content"): - tab = self.open_tab() - self.marionette.switch_to_window(tab) - self.marionette.close() + new_tab = self.open_tab() + self.marionette.switch_to_window(new_tab) + self.marionette.close() # Check that only the content window is not available in both contexts for context in ("chrome", "content"): @@ -79,25 +72,26 @@ class TestNoSuchWindowContent(WindowManagerMixin, MarionetteTestCase): self.marionette.switch_to_window(self.start_tab) with self.assertRaises(NoSuchWindowException): - self.marionette.switch_to_window(tab) + self.marionette.switch_to_window(new_tab) def test_closed_tab_while_in_frame(self): + new_tab = self.open_tab() + self.marionette.switch_to_window(new_tab) + with self.marionette.using_context("content"): - tab = self.open_tab() - self.marionette.switch_to_window(tab) self.marionette.navigate(self.marionette.absolute_url("test_iframe.html")) frame = self.marionette.find_element(By.ID, "test_iframe") self.marionette.switch_to_frame(frame) - self.marionette.close() + self.marionette.close() - with self.assertRaises(NoSuchWindowException): - self.marionette.current_window_handle - self.marionette.current_chrome_window_handle + with self.assertRaises(NoSuchWindowException): + self.marionette.current_window_handle + self.marionette.current_chrome_window_handle self.marionette.switch_to_window(self.start_tab) with self.assertRaises(NoSuchWindowException): - self.marionette.switch_to_window(tab) + self.marionette.switch_to_window(new_tab) class TestNoSuchWindowChrome(TestNoSuchWindowContent): @@ -121,42 +115,22 @@ class TestSwitchWindow(WindowManagerMixin, MarionetteTestCase): self.close_all_windows() super(TestSwitchWindow, self).tearDown() - def test_windows(self): - def open_browser_with_js(): - self.marionette.execute_script(" window.open(); ") - - new_window = self.open_window(trigger=open_browser_with_js) + def test_switch_window_after_open_and_close(self): + with self.marionette.using_context("chrome"): + new_window = self.open_window() + self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows) + 1) + self.assertIn(new_window, self.marionette.chrome_window_handles) self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) - # switch to the other window + # switch to the new chrome window and close it self.marionette.switch_to_window(new_window) self.assertEqual(self.marionette.current_chrome_window_handle, new_window) self.assertNotEqual(self.marionette.current_chrome_window_handle, self.start_window) - # switch back and close original window + self.marionette.close_chrome_window() + self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows)) + self.assertNotIn(new_window, self.marionette.chrome_window_handles) + + # switch back to the original chrome window self.marionette.switch_to_window(self.start_window) self.assertEqual(self.marionette.current_chrome_window_handle, self.start_window) - self.marionette.close_chrome_window() - - self.assertNotIn(self.start_window, self.marionette.chrome_window_handles) - self.assertEqual(len(self.marionette.chrome_window_handles), len(self.start_windows)) - - def test_should_load_and_close_a_window(self): - def open_window_with_link(): - test_html = self.marionette.absolute_url("test_windows.html") - with self.marionette.using_context("content"): - self.marionette.navigate(test_html) - self.marionette.find_element(By.LINK_TEXT, "Open new window").click() - - new_window = self.open_window(trigger=open_window_with_link) - self.marionette.switch_to_window(new_window) - self.assertEqual(self.marionette.current_chrome_window_handle, new_window) - self.assertEqual(len(self.marionette.chrome_window_handles), 2) - - with self.marionette.using_context('content'): - self.assertEqual(self.marionette.title, "We Arrive Here") - - # Let's close and check - self.marionette.close_chrome_window() - self.marionette.switch_to_window(self.start_window) - self.assertEqual(len(self.marionette.chrome_window_handles), 1) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_window_status_content.py b/testing/marionette/harness/marionette_harness/tests/unit/test_window_status_content.py index 16f5449707ff..c9aee1cbbc8e 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_window_status_content.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_window_status_content.py @@ -15,30 +15,15 @@ class TestNoSuchWindowContent(WindowManagerMixin, MarionetteTestCase): def setUp(self): super(TestNoSuchWindowContent, self).setUp() - self.test_page = self.marionette.absolute_url("windowHandles.html") - with self.marionette.using_context("content"): - self.marionette.navigate(self.test_page) - def tearDown(self): self.close_all_windows() super(TestNoSuchWindowContent, self).tearDown() - def open_tab_in_foreground(self): - with self.marionette.using_context("content"): - link = self.marionette.find_element(By.ID, "new-tab") - link.click() - @skip_if_mobile("Fennec doesn't support other chrome windows") def test_closed_chrome_window(self): - - def open_with_link(): - with self.marionette.using_context("content"): - test_page = self.marionette.absolute_url("windowHandles.html") - self.marionette.navigate(test_page) - self.marionette.find_element(By.ID, "new-window").click() - - win = self.open_window(open_with_link) - self.marionette.switch_to_window(win) + with self.marionette.using_context("chrome"): + new_window = self.open_window() + self.marionette.switch_to_window(new_window) self.marionette.close_chrome_window() # When closing a browser window both handles are not available @@ -52,12 +37,13 @@ class TestNoSuchWindowContent(WindowManagerMixin, MarionetteTestCase): self.marionette.switch_to_window(self.start_window) with self.assertRaises(NoSuchWindowException): - self.marionette.switch_to_window(win) + self.marionette.switch_to_window(new_window) @skip_if_mobile("Fennec doesn't support other chrome windows") def test_closed_chrome_window_while_in_frame(self): - win = self.open_chrome_window("chrome://marionette/content/test.xul") - self.marionette.switch_to_window(win) + new_window = self.open_chrome_window("chrome://marionette/content/test.xul") + self.marionette.switch_to_window(new_window) + with self.marionette.using_context("chrome"): self.marionette.switch_to_frame("iframe") self.marionette.close_chrome_window() @@ -70,13 +56,12 @@ class TestNoSuchWindowContent(WindowManagerMixin, MarionetteTestCase): self.marionette.switch_to_window(self.start_window) with self.assertRaises(NoSuchWindowException): - self.marionette.switch_to_window(win) + self.marionette.switch_to_window(new_window) def test_closed_tab(self): - with self.marionette.using_context("content"): - tab = self.open_tab(self.open_tab_in_foreground) - self.marionette.switch_to_window(tab) - self.marionette.close() + new_tab = self.open_tab(focus=True) + self.marionette.switch_to_window(new_tab) + self.marionette.close() # Check that only the content window is not available in both contexts for context in ("chrome", "content"): @@ -88,22 +73,24 @@ class TestNoSuchWindowContent(WindowManagerMixin, MarionetteTestCase): self.marionette.switch_to_window(self.start_tab) with self.assertRaises(NoSuchWindowException): - self.marionette.switch_to_window(tab) + self.marionette.switch_to_window(new_tab) def test_closed_tab_while_in_frame(self): + new_tab = self.open_tab() + self.marionette.switch_to_window(new_tab) + with self.marionette.using_context("content"): - tab = self.open_tab(self.open_tab_in_foreground) - self.marionette.switch_to_window(tab) self.marionette.navigate(self.marionette.absolute_url("test_iframe.html")) frame = self.marionette.find_element(By.ID, "test_iframe") self.marionette.switch_to_frame(frame) - self.marionette.close() - with self.assertRaises(NoSuchWindowException): - self.marionette.current_window_handle - self.marionette.current_chrome_window_handle + self.marionette.close() + + with self.assertRaises(NoSuchWindowException): + self.marionette.current_window_handle + self.marionette.current_chrome_window_handle self.marionette.switch_to_window(self.start_tab) with self.assertRaises(NoSuchWindowException): - self.marionette.switch_to_window(tab) + self.marionette.switch_to_window(new_tab) From ec8e87d9fc8771e7acbfa05427b50cd95953e229 Mon Sep 17 00:00:00 2001 From: Narcis Beleuzu Date: Wed, 5 Dec 2018 22:06:21 +0200 Subject: [PATCH 19/34] Backed out changeset 4ad8241eb92a (bug 1511398) gv-junit2 crashes on [@ mozilla::a11y::Accessible::IsDefunct() const] . CLOSED TREE --- accessible/android/SessionAccessibility.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/accessible/android/SessionAccessibility.cpp b/accessible/android/SessionAccessibility.cpp index a45b6b68dfa5..75f580a8150f 100644 --- a/accessible/android/SessionAccessibility.cpp +++ b/accessible/android/SessionAccessibility.cpp @@ -3,7 +3,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#include "Accessible-inl.h" #include "SessionAccessibility.h" #include "AndroidUiThread.h" #include "DocAccessibleParent.h" @@ -372,15 +371,9 @@ void SessionAccessibility::UpdateCachedBounds( auto infos = jni::ObjectArray::New(aAccessibles.Length()); for (size_t i = 0; i < aAccessibles.Length(); i++) { AccessibleWrap* acc = aAccessibles.ElementAt(i); - if (!acc || acc->IsDefunct()) { - MOZ_ASSERT_UNREACHABLE("Updated accessible is gone."); - continue; - } - if (aData.Length() == aAccessibles.Length()) { const BatchData& data = aData.ElementAt(i); - auto bundle = - acc->ToSmallBundle(data.State(), data.Bounds(), data.ActionCount()); + auto bundle = acc->ToSmallBundle(data.State(), data.Bounds(), data.ActionCount()); infos->SetElement(i, bundle); } else { infos->SetElement(i, acc->ToSmallBundle()); From d69962cc066abb2a2dfe81a3f7d76cefac630ef4 Mon Sep 17 00:00:00 2001 From: Narcis Beleuzu Date: Wed, 5 Dec 2018 22:19:24 +0200 Subject: [PATCH 20/34] Backed out 2 changesets (bug 1504659) for mochitest failures on test_innerWidthHeight_script.html . CLOSED TREE Backed out changeset f2574f5b186e (bug 1504659) Backed out changeset 45f63618f66e (bug 1504659) --- .../responsive.html/test/browser/browser.ini | 1 - .../test/browser/browser_scroll.js | 75 ------------------- .../responsive.html/test/browser/head.js | 11 --- .../test/unit/test_resize_viewport.js | 10 +-- layout/base/MobileViewportManager.cpp | 8 +- 5 files changed, 5 insertions(+), 100 deletions(-) delete mode 100644 devtools/client/responsive.html/test/browser/browser_scroll.js diff --git a/devtools/client/responsive.html/test/browser/browser.ini b/devtools/client/responsive.html/test/browser/browser.ini index 4339e292d775..2af02083e196 100644 --- a/devtools/client/responsive.html/test/browser/browser.ini +++ b/devtools/client/responsive.html/test/browser/browser.ini @@ -52,7 +52,6 @@ tags = devtools geolocation skip-if = true # Bug 1413765 [browser_preloaded_newtab.js] [browser_screenshot_button.js] -[browser_scroll.js] [browser_state_restore.js] [browser_tab_close.js] [browser_tab_remoteness_change.js] diff --git a/devtools/client/responsive.html/test/browser/browser_scroll.js b/devtools/client/responsive.html/test/browser/browser_scroll.js deleted file mode 100644 index a33c5b0da6fb..000000000000 --- a/devtools/client/responsive.html/test/browser/browser_scroll.js +++ /dev/null @@ -1,75 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. -http://creativecommons.org/publicdomain/zero/1.0/ */ - -"use strict"; - -/** - * This test is checking that keyboard scrolling of content in RDM - * behaves correctly, both with and without touch simulation enabled. - */ - -const TEST_URL = "data:text/html;charset=utf-8," + - "
"; - -addRDMTask(TEST_URL, async function({ ui, manager }) { - info("Turning off keyboard APZ for this test."); - await SpecialPowers.pushPrefEnv({ - set: [["apz.keyboard.enabled", false]], - }); - - await setViewportSize(ui, manager, 100, 100); - const browser = ui.getViewportBrowser(); - - info("Setting focus on the browser."); - browser.focus(); - - info("Testing scroll behavior with touch simulation OFF."); - await testScrollingOfContent(ui); - - // Run the tests again with touch simulation on. - const reloadNeeded = await ui.updateTouchSimulation(true); - if (reloadNeeded) { - info("Reload is needed -- waiting for it."); - const reload = waitForViewportLoad(ui); - browser.reload(); - await reload; - - await ContentTask.spawn(browser, null, () => { - content.scrollTo(0, 0); - }); - } - - info("Testing scroll behavior with touch simulation ON."); - await testScrollingOfContent(ui); -}); - -async function testScrollingOfContent(ui) { - let scroll; - - info("Checking initial scroll conditions."); - const viewportScroll = await getViewportScroll(ui); - is(viewportScroll.x, 0, "Content should load with scrollX 0."); - is(viewportScroll.y, 0, "Content should load with scrollY 0."); - - /** - * Here we're going to send off some arrow key events to trigger scrolling. - * What we would like to be able to do is to await the scroll event and then - * check the scroll position to confirm the amount of scrolling that has - * happened. Unfortunately, APZ makes the scrolling happen asynchronously on - * the compositor thread, and it's very difficult to await the end state of - * the APZ animation -- see the tests in /gfx/layers/apz/test/mochitest for - * an example. For our purposes, it's sufficient to test that the scroll - * event is fired at all, and not worry about the amount of scrolling that - * has occurred at the time of the event. If the key events don't trigger - * scrolling, then no event will be fired and the test will time out. - */ - scroll = waitForViewportScroll(ui); - EventUtils.synthesizeKey("KEY_ArrowDown"); - await scroll; - info("Scroll event was fired after arrow key down."); - - scroll = waitForViewportScroll(ui); - EventUtils.synthesizeKey("KEY_ArrowRight"); - await scroll; - info("Scroll event was fired after arrow key right."); -} diff --git a/devtools/client/responsive.html/test/browser/head.js b/devtools/client/responsive.html/test/browser/head.js index b9f7156e355d..a4fdff613e33 100644 --- a/devtools/client/responsive.html/test/browser/head.js +++ b/devtools/client/responsive.html/test/browser/head.js @@ -332,13 +332,6 @@ function getContentSize(ui) { })); } -function getViewportScroll(ui) { - return spawnViewportTask(ui, {}, () => ({ - x: content.scrollX, - y: content.scrollY, - })); -} - async function waitForPageShow(browser) { const tab = gBrowser.getTabForBrowser(browser); const ui = ResponsiveUIManager.getResponsiveUIForTab(tab); @@ -356,10 +349,6 @@ function waitForViewportLoad(ui) { return BrowserTestUtils.waitForContentEvent(ui.getViewportBrowser(), "load", true); } -function waitForViewportScroll(ui) { - return BrowserTestUtils.waitForContentEvent(ui.getViewportBrowser(), "scroll", true); -} - function load(browser, url) { const loaded = BrowserTestUtils.browserLoaded(browser, false, url); BrowserTestUtils.loadURI(browser, url); diff --git a/devtools/client/responsive.html/test/unit/test_resize_viewport.js b/devtools/client/responsive.html/test/unit/test_resize_viewport.js index e6503f7b4825..d983f3a0c09f 100644 --- a/devtools/client/responsive.html/test/unit/test_resize_viewport.js +++ b/devtools/client/responsive.html/test/unit/test_resize_viewport.js @@ -7,7 +7,6 @@ const { addViewport, resizeViewport } = require("devtools/client/responsive.html/actions/viewports"); -const { toggleTouchSimulation } = require("devtools/client/responsive.html/actions/ui"); add_task(async function() { const store = Store(); @@ -16,14 +15,7 @@ add_task(async function() { dispatch(addViewport()); dispatch(resizeViewport(0, 500, 500)); - let viewport = getState().viewports[0]; + const viewport = getState().viewports[0]; equal(viewport.width, 500, "Resized width of 500"); equal(viewport.height, 500, "Resized height of 500"); - - dispatch(toggleTouchSimulation(true)); - dispatch(resizeViewport(0, 400, 400)); - - viewport = getState().viewports[0]; - equal(viewport.width, 400, "Resized width of 400 (with touch simulation on)"); - equal(viewport.height, 400, "Resized height of 400 (with touch simulation on)"); }); diff --git a/layout/base/MobileViewportManager.cpp b/layout/base/MobileViewportManager.cpp index e9b107af4ad8..fd78d13eab0d 100644 --- a/layout/base/MobileViewportManager.cpp +++ b/layout/base/MobileViewportManager.cpp @@ -397,6 +397,10 @@ void MobileViewportManager::RefreshVisualViewportSize() { // This function is a subset of RefreshViewportSize, and only updates the // visual viewport size. + if (!gfxPrefs::APZAllowZooming()) { + return; + } + ScreenIntSize displaySize = ViewAs( mDisplaySize, PixelCastJustification::LayoutDeviceIsScreenForBounds); @@ -472,10 +476,6 @@ void MobileViewportManager::RefreshViewportSize(bool aForceAdjustResolution) { if (gfxPrefs::APZAllowZooming()) { UpdateResolution(viewportInfo, displaySize, viewport, displayWidthChangeRatio, UpdateType::ViewportSize); - } else { - // Even without zoom, we need to update that the visual viewport size - // has changed. - RefreshVisualViewportSize(); } if (gfxPlatform::AsyncPanZoomEnabled()) { UpdateDisplayPortMargins(); From e027ad57e6438df06298926c2c622e6333472371 Mon Sep 17 00:00:00 2001 From: Eitan Isaacson Date: Wed, 5 Dec 2018 20:13:07 +0000 Subject: [PATCH 21/34] Bug 1507713 - Provide heading level in roleDescription. r=yzen Differential Revision: https://phabricator.services.mozilla.com/D13504 --HG-- extra : moz-landing-system : lando --- accessible/android/AccessibleWrap.cpp | 19 +++++- accessible/android/AccessibleWrap.h | 5 +- .../geckoview/test/AccessibilityTest.kt | 58 +++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/accessible/android/AccessibleWrap.cpp b/accessible/android/AccessibleWrap.cpp index f729154b2cc3..fad97ae46be7 100644 --- a/accessible/android/AccessibleWrap.cpp +++ b/accessible/android/AccessibleWrap.cpp @@ -287,7 +287,9 @@ uint32_t AccessibleWrap::GetFlags(role aRole, uint64_t aState, return flags; } -void AccessibleWrap::GetRoleDescription(role aRole, nsAString& aGeckoRole, +void AccessibleWrap::GetRoleDescription(role aRole, + nsIPersistentProperties* aAttributes, + nsAString& aGeckoRole, nsAString& aRoleDescription) { nsresult rv = NS_OK; @@ -305,6 +307,19 @@ void AccessibleWrap::GetRoleDescription(role aRole, nsAString& aGeckoRole, return; } + if (aRole == roles::HEADING) { + nsString level; + rv = aAttributes->GetStringProperty(NS_LITERAL_CSTRING("level"), level); + if (NS_SUCCEEDED(rv)) { + const char16_t* formatString[] = {level.get()}; + rv = bundle->FormatStringFromName("headingLevel", formatString, 1, + aRoleDescription); + if (NS_SUCCEEDED(rv)) { + return; + } + } + } + GetAccService()->GetStringRole(aRole, aGeckoRole); rv = bundle->GetStringFromName(NS_ConvertUTF16toUTF8(aGeckoRole).get(), aRoleDescription); @@ -450,7 +465,7 @@ mozilla::java::GeckoBundle::LocalRef AccessibleWrap::ToBundle( nsAutoString geckoRole; nsAutoString roleDescription; if (VirtualViewID() != kNoID) { - GetRoleDescription(role, geckoRole, roleDescription); + GetRoleDescription(role, aAttributes, geckoRole, roleDescription); } GECKOBUNDLE_PUT(nodeInfo, "roleDescription", diff --git a/accessible/android/AccessibleWrap.h b/accessible/android/AccessibleWrap.h index 32a9c88cb147..941ff8f6128f 100644 --- a/accessible/android/AccessibleWrap.h +++ b/accessible/android/AccessibleWrap.h @@ -81,8 +81,11 @@ class AccessibleWrap : public Accessible { virtual role WrapperRole() { return Role(); } - static void GetRoleDescription(role aRole, nsAString& aGeckoRole, + static void GetRoleDescription(role aRole, + nsIPersistentProperties* aAttributes, + nsAString& aGeckoRole, nsAString& aRoleDescription); + static uint32_t GetFlags(role aRole, uint64_t aState, uint8_t aActionCount); }; diff --git a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt index f1f6cc8c10e2..97b2cd5e2893 100644 --- a/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt +++ b/mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt @@ -437,6 +437,64 @@ class AccessibilityTest : BaseSessionTest() { waitUntilTextTraversed(0, 18) // "Lorem ipsum dolor " } + @Test fun testHeadings() { + var nodeId = AccessibilityNodeProvider.HOST_VIEW_ID; + sessionRule.session.loadString(""" + preamble +

Fried cheese

with club sauce.

+

Popcorn shrimp

+

Chicken fingers

with spicy club sauce.

""".trimIndent(), "text/html") + waitForInitialFocus() + + val bundle = Bundle() + bundle.putString(AccessibilityNodeInfo.ACTION_ARGUMENT_HTML_ELEMENT_STRING, "HEADING") + + provider.performAction(nodeId, AccessibilityNodeInfo.ACTION_NEXT_HTML_ELEMENT, bundle) + sessionRule.waitUntilCalled(object : EventDelegate { + @AssertCalled(count = 1) + override fun onAccessibilityFocused(event: AccessibilityEvent) { + nodeId = getSourceId(event) + val node = createNodeInfo(nodeId) + assertThat("Accessibility focus on first heading", node.text as String, startsWith("Fried cheese")) + if (Build.VERSION.SDK_INT >= 19) { + assertThat("First heading is level 1", + node.extras.getCharSequence("AccessibilityNodeInfo.roleDescription").toString(), + equalTo("heading level 1")) + } + } + }) + + provider.performAction(nodeId, AccessibilityNodeInfo.ACTION_NEXT_HTML_ELEMENT, bundle) + sessionRule.waitUntilCalled(object : EventDelegate { + @AssertCalled(count = 1) + override fun onAccessibilityFocused(event: AccessibilityEvent) { + nodeId = getSourceId(event) + val node = createNodeInfo(nodeId) + assertThat("Accessibility focus on second heading", node.text as String, startsWith("Popcorn shrimp")) + if (Build.VERSION.SDK_INT >= 19) { + assertThat("Second heading is level 2", + node.extras.getCharSequence("AccessibilityNodeInfo.roleDescription").toString(), + equalTo("heading level 2")) + } + } + }) + + provider.performAction(nodeId, AccessibilityNodeInfo.ACTION_NEXT_HTML_ELEMENT, bundle) + sessionRule.waitUntilCalled(object : EventDelegate { + @AssertCalled(count = 1) + override fun onAccessibilityFocused(event: AccessibilityEvent) { + nodeId = getSourceId(event) + val node = createNodeInfo(nodeId) + assertThat("Accessibility focus on second heading", node.text as String, startsWith("Chicken fingers")) + if (Build.VERSION.SDK_INT >= 19) { + assertThat("Third heading is level 3", + node.extras.getCharSequence("AccessibilityNodeInfo.roleDescription").toString(), + equalTo("heading level 3")) + } + } + }) + } + @Test fun testCheckbox() { var nodeId = AccessibilityNodeProvider.HOST_VIEW_ID; sessionRule.session.loadString("", "text/html") From dfbc8576880b2eee255bf6f1703ea0eaa0453d8e Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Fri, 30 Nov 2018 22:30:39 +0000 Subject: [PATCH 22/34] Bug 1511483 - pageAction show_matches/hide_matches should allow unrestricted schemes on privileged mozilla addons. r=mixedpuppy Differential Revision: https://phabricator.services.mozilla.com/D13582 --HG-- extra : moz-landing-system : lando --- .../extensions/parent/ext-pageAction.js | 5 +- .../browser_ext_pageAction_show_matches.js | 60 +++++++++++++++++++ toolkit/components/extensions/Extension.jsm | 25 ++++++-- 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/browser/components/extensions/parent/ext-pageAction.js b/browser/components/extensions/parent/ext-pageAction.js index cb7d2689fb91..bf02be486434 100644 --- a/browser/components/extensions/parent/ext-pageAction.js +++ b/browser/components/extensions/parent/ext-pageAction.js @@ -55,8 +55,9 @@ this.pageAction = class extends ExtensionAPI { show = false; } else { // Might show or hide depending on the URL. Enable pattern matching. - showMatches = new MatchPatternSet(show_matches); - hideMatches = new MatchPatternSet(hide_matches); + const {restrictSchemes} = extension; + showMatches = new MatchPatternSet(show_matches, {restrictSchemes}); + hideMatches = new MatchPatternSet(hide_matches, {restrictSchemes}); } this.defaults = { diff --git a/browser/components/extensions/test/browser/browser_ext_pageAction_show_matches.js b/browser/components/extensions/test/browser/browser_ext_pageAction_show_matches.js index 6de5228ead99..5c57a1825cee 100644 --- a/browser/components/extensions/test/browser/browser_ext_pageAction_show_matches.js +++ b/browser/components/extensions/test/browser/browser_ext_pageAction_show_matches.js @@ -175,3 +175,63 @@ add_task(async function test_pageAction_all_urls() { let rejects = await extension.startup().then(() => false, () => true); is(rejects, true, "startup failed"); }); + +add_task(async function test_pageAction_restrictScheme_false() { + info("Check restricted origins are allowed in show_matches for privileged extensions"); + let extension = ExtensionTestUtils.loadExtension({ + isPrivileged: true, + manifest: { + permissions: ["mozillaAddons", "tabs"], + page_action: { + "show_matches": ["about:reader*"], + "hide_matches": ["*://*/*"], + }, + }, + background: function() { + browser.tabs.onUpdated.addListener(async (tabId, changeInfo) => { + if (changeInfo.url && changeInfo.url.startsWith("about:reader")) { + browser.test.sendMessage("readerModeEntered"); + } + }); + + browser.test.onMessage.addListener(async (msg) => { + if (msg !== "enterReaderMode") { + browser.test.fail(`Received unexpected test message: ${msg}`); + return; + } + + browser.tabs.toggleReaderMode(); + }); + }, + }); + + async function expectPageAction(extension, tab, isShown) { + await promiseAnimationFrame(); + let widgetId = makeWidgetId(extension.id); + let pageActionId = BrowserPageActions.urlbarButtonNodeIDForActionID(widgetId); + let iconEl = document.getElementById(pageActionId); + + if (isShown) { + ok(iconEl && !iconEl.hasAttribute("disabled"), "pageAction is shown"); + } else { + ok(iconEl == null || iconEl.getAttribute("disabled") == "true", "pageAction is hidden"); + } + } + + const baseUrl = getRootDirectory(gTestPath).replace("chrome://mochitests/content", "http://example.com"); + const url = `${baseUrl}/readerModeArticle.html`; + let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, url, true, true); + + await extension.startup(); + + await expectPageAction(extension, tab, false); + + extension.sendMessage("enterReaderMode"); + await extension.awaitMessage("readerModeEntered"); + + await expectPageAction(extension, tab, true); + + BrowserTestUtils.removeTab(tab); + + await extension.unload(); +}); diff --git a/toolkit/components/extensions/Extension.jsm b/toolkit/components/extensions/Extension.jsm index 126e8fb0c0e4..18505b12e626 100644 --- a/toolkit/components/extensions/Extension.jsm +++ b/toolkit/components/extensions/Extension.jsm @@ -467,6 +467,13 @@ class ExtensionData { }); } + get restrictSchemes() { + // ExtensionData can't check the signature (as it is not yet passed to its constructor + // as it is for the Extension class, where this getter is overridden to check both the + // signature and the permissions). + return !this.hasPermission("mozillaAddons"); + } + /** * Returns an object representing any capabilities that the extension * has access to based on fixed properties in the manifest. The result @@ -481,7 +488,7 @@ class ExtensionData { let permissions = new Set(); let origins = new Set(); - let restrictSchemes = !this.hasPermission("mozillaAddons"); + let {restrictSchemes} = this; for (let perm of this.manifest.permissions || []) { let type = classifyPermission(perm, restrictSchemes); if (type.origin) { @@ -858,7 +865,9 @@ class ExtensionData { await this.apiManager.lazyInit(); this.webAccessibleResources = manifestData.webAccessibleResources.map(res => new MatchGlob(res)); - this.whiteListedHosts = new MatchPatternSet(manifestData.originPermissions, {restrictSchemes: !this.hasPermission("mozillaAddons")}); + this.whiteListedHosts = new MatchPatternSet(manifestData.originPermissions, { + restrictSchemes: this.restrictSchemes, + }); return this.manifest; } @@ -1376,8 +1385,10 @@ class Extension extends ExtensionData { if (permissions.origins.length > 0) { let patterns = this.whiteListedHosts.patterns.map(host => host.pattern); - this.whiteListedHosts = new MatchPatternSet(new Set([...patterns, ...permissions.origins]), - {restrictSchemes: !this.hasPermission("mozillaAddons"), ignorePath: true}); + this.whiteListedHosts = new MatchPatternSet(new Set([...patterns, ...permissions.origins]), { + restrictSchemes: this.restrictSchemes, + ignorePath: true, + }); } this.policy.permissions = Array.from(this.permissions); @@ -1406,6 +1417,10 @@ class Extension extends ExtensionData { /* eslint-enable mozilla/balanced-listeners */ } + get restrictSchemes() { + return !(this.isPrivileged && this.hasPermission("mozillaAddons")); + } + // Some helpful properties added elsewhere: /** * An object used to map between extension-visible tab ids and @@ -2015,7 +2030,7 @@ class Extension extends ExtensionData { get optionalOrigins() { if (this._optionalOrigins == null) { - let restrictSchemes = !this.hasPermission("mozillaAddons"); + let {restrictSchemes} = this; let origins = this.manifest.optional_permissions.filter(perm => classifyPermission(perm, restrictSchemes).origin); this._optionalOrigins = new MatchPatternSet(origins, {restrictSchemes, ignorePath: true}); } From bf88a10c9f940e5ebb597a576563bafd2a6f6fee Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Tue, 4 Dec 2018 20:53:35 +0000 Subject: [PATCH 23/34] Bug 1483620 - Screenshots pageAction should be shown on about:reader urls. r=_6a68 Depends on D13582 Differential Revision: https://phabricator.services.mozilla.com/D13583 --HG-- extra : moz-landing-system : lando --- browser/extensions/screenshots/manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/extensions/screenshots/manifest.json b/browser/extensions/screenshots/manifest.json index 7cc2dc10199c..2055e6065625 100644 --- a/browser/extensions/screenshots/manifest.json +++ b/browser/extensions/screenshots/manifest.json @@ -36,7 +36,7 @@ "32": "icons/icon-v2.svg" }, "default_title": "__MSG_contextMenuLabel__", - "show_matches": [""], + "show_matches": ["", "about:reader*"], "pinned": false }, "icons": { From 6f42a078131e3c402bb77ffc26a8b178680ce078 Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Wed, 5 Dec 2018 19:19:56 +0000 Subject: [PATCH 24/34] Bug 1436037 - [taskgraph] Support Windows generic-worker with run-task, r=gps This enables Windows generic-worker based tasks to use the run-task script. Differential Revision: https://phabricator.services.mozilla.com/D10758 --HG-- extra : moz-landing-system : lando --- taskcluster/scripts/run-task | 6 +++- .../taskgraph/transforms/job/common.py | 22 ++++++++---- taskcluster/taskgraph/transforms/job/mach.py | 15 +++++--- .../taskgraph/transforms/job/python_test.py | 15 ++++---- .../taskgraph/transforms/job/run_task.py | 34 +++++++++++++++---- taskcluster/taskgraph/transforms/task.py | 4 +-- 6 files changed, 69 insertions(+), 27 deletions(-) diff --git a/taskcluster/scripts/run-task b/taskcluster/scripts/run-task index ddb090f9d8b7..8f4ea64797af 100755 --- a/taskcluster/scripts/run-task +++ b/taskcluster/scripts/run-task @@ -448,13 +448,14 @@ def vcs_checkout(source_repo, dest, store_path, if sparse_profile: args.extend(['--sparseprofile', sparse_profile]) + dest = os.path.abspath(dest) args.extend([ revision_flag, revision_value, source_repo, dest, ]) res = run_and_prefix_output(b'vcs', args, - extra_env={b'PYTHONUNBUFFERED': b'1'}) + extra_env={'PYTHONUNBUFFERED': '1'}) if res: sys.exit(res) @@ -741,6 +742,9 @@ def main(args): return 1 try: + if 'GECKO_PATH' in os.environ: + os.environ['GECKO_PATH'] = os.path.abspath(os.environ['GECKO_PATH']) + if 'MOZ_FETCHES' in os.environ: fetch_artifacts() diff --git a/taskcluster/taskgraph/transforms/job/common.py b/taskcluster/taskgraph/transforms/job/common.py index 6f3e599028ac..c01f4cc7bb7a 100644 --- a/taskcluster/taskgraph/transforms/job/common.py +++ b/taskcluster/taskgraph/transforms/job/common.py @@ -64,10 +64,20 @@ def support_vcs_checkout(config, job, taskdesc, sparse=False): This can only be used with ``run-task`` tasks, as the cache name is reserved for ``run-task`` tasks. """ - level = config.params['level'] + is_win = job['worker']['os'] == 'windows' - # native-engine does not support caches (yet), so we just do a full clone - # every time :( + if is_win: + checkoutdir = './build' + geckodir = '{}/src'.format(checkoutdir) + hgstore = 'y:/hg-shared' + else: + checkoutdir = '{workdir}/checkouts'.format(**job['run']) + geckodir = '{}/gecko'.format(checkoutdir) + hgstore = '{}/hg-store'.format(checkoutdir) + + level = config.params['level'] + # native-engine and generic-worker do not support caches (yet), so we just + # do a full clone every time :( if job['worker']['implementation'] in ('docker-worker', 'docker-engine'): name = 'level-%s-checkouts' % level @@ -84,15 +94,15 @@ def support_vcs_checkout(config, job, taskdesc, sparse=False): taskdesc['worker'].setdefault('caches', []).append({ 'type': 'persistent', 'name': name, - 'mount-point': '{workdir}/checkouts'.format(**job['run']), + 'mount-point': checkoutdir, }) taskdesc['worker'].setdefault('env', {}).update({ 'GECKO_BASE_REPOSITORY': config.params['base_repository'], 'GECKO_HEAD_REPOSITORY': config.params['head_repository'], 'GECKO_HEAD_REV': config.params['head_rev'], - 'GECKO_PATH': '{workdir}/checkouts/gecko'.format(**job['run']), - 'HG_STORE_PATH': '{workdir}/checkouts/hg-store'.format(**job['run']), + 'GECKO_PATH': geckodir, + 'HG_STORE_PATH': hgstore, }) if 'comm_base_repository' in config.params: diff --git a/taskcluster/taskgraph/transforms/job/mach.py b/taskcluster/taskgraph/transforms/job/mach.py index a216d263a141..6b643588b5a7 100644 --- a/taskcluster/taskgraph/transforms/job/mach.py +++ b/taskcluster/taskgraph/transforms/job/mach.py @@ -30,14 +30,19 @@ mach_schema = Schema({ }) -@run_job_using("docker-worker", "mach", schema=mach_schema, defaults={'comm-checkout': False}) -@run_job_using("native-engine", "mach", schema=mach_schema, defaults={'comm-checkout': False}) -@run_job_using("generic-worker", "mach", schema=mach_schema, defaults={'comm-checkout': False}) -def docker_worker_mach(config, job, taskdesc): +defaults = { + 'comm-checkout': False, +} + + +@run_job_using("docker-worker", "mach", schema=mach_schema, defaults=defaults) +@run_job_using("native-engine", "mach", schema=mach_schema, defaults=defaults) +@run_job_using("generic-worker", "mach", schema=mach_schema, defaults=defaults) +def configure_mach(config, job, taskdesc): run = job['run'] # defer to the run_task implementation - run['command'] = 'cd {workdir}/checkouts/gecko && ./mach {mach}'.format(**run) + run['command'] = 'cd $GECKO_PATH && ./mach {mach}'.format(**run) run['using'] = 'run-task' del run['mach'] configure_taskdesc_for_run(config, job, taskdesc, job['worker']['implementation']) diff --git a/taskcluster/taskgraph/transforms/job/python_test.py b/taskcluster/taskgraph/transforms/job/python_test.py index eaac16532300..10dafb225ba4 100644 --- a/taskcluster/taskgraph/transforms/job/python_test.py +++ b/taskcluster/taskgraph/transforms/job/python_test.py @@ -25,12 +25,15 @@ python_test_schema = Schema({ }) -@run_job_using( - 'docker-worker', - 'python-test', - schema=python_test_schema, - defaults={'python-version': 2, 'subsuite': 'default'}) -def docker_worker_python_test(config, job, taskdesc): +defaults = { + 'python-version': 2, + 'subsuite': 'default', +} + + +@run_job_using('docker-worker', 'python-test', schema=python_test_schema, defaults=defaults) +@run_job_using('generic-worker', 'python-test', schema=python_test_schema, defaults=defaults) +def configure_python_test(config, job, taskdesc): run = job['run'] # defer to the mach implementation diff --git a/taskcluster/taskgraph/transforms/job/run_task.py b/taskcluster/taskgraph/transforms/job/run_task.py index be6b1b0c67a3..800355414fba 100644 --- a/taskcluster/taskgraph/transforms/job/run_task.py +++ b/taskcluster/taskgraph/transforms/job/run_task.py @@ -41,12 +41,12 @@ run_task_schema = Schema({ }) -def common_setup(config, job, taskdesc, command, checkoutdir): +def common_setup(config, job, taskdesc, command, geckodir): run = job['run'] if run['checkout']: support_vcs_checkout(config, job, taskdesc, sparse=bool(run['sparse-profile'])) - command.append('--vcs-checkout={}/gecko'.format(checkoutdir)) + command.append('--vcs-checkout={}'.format(geckodir)) if run['sparse-profile']: command.append('--sparse-profile=build/sparse-profiles/%s' % @@ -73,7 +73,8 @@ def docker_worker_run_task(config, job, taskdesc): run = job['run'] worker = taskdesc['worker'] = job['worker'] command = ['/builds/worker/bin/run-task'] - common_setup(config, job, taskdesc, command, checkoutdir='{workdir}/checkouts'.format(**run)) + common_setup(config, job, taskdesc, command, + geckodir='{workdir}/checkouts/gecko'.format(**run)) if run.get('cache-dotcache'): worker['caches'].append({ @@ -100,7 +101,8 @@ def native_engine_run_task(config, job, taskdesc): run = job['run'] worker = taskdesc['worker'] = job['worker'] command = ['./run-task'] - common_setup(config, job, taskdesc, command, checkoutdir='{workdir}/checkouts'.format(**run)) + common_setup(config, job, taskdesc, command, + geckodir='{workdir}/checkouts/gecko'.format(**run)) worker['context'] = run_task_url(config) @@ -119,8 +121,16 @@ def native_engine_run_task(config, job, taskdesc): def generic_worker_run_task(config, job, taskdesc): run = job['run'] worker = taskdesc['worker'] = job['worker'] - command = ['./run-task'] - common_setup(config, job, taskdesc, command, checkoutdir='{workdir}/checkouts'.format(**run)) + is_win = worker['os'] == 'windows' + + if is_win: + command = ['C:/mozilla-build/python3/python3.exe', 'run-task'] + geckodir = './build/src' + else: + command = ['./run-task'] + geckodir = '{workdir}/checkouts/gecko'.format(**run) + + common_setup(config, job, taskdesc, command, geckodir=geckodir) worker.setdefault('mounts', []) if run.get('cache-dotcache'): @@ -137,7 +147,17 @@ def generic_worker_run_task(config, job, taskdesc): run_command = run['command'] if isinstance(run_command, basestring): + if is_win: + run_command = '"{}"'.format(run_command) run_command = ['bash', '-cx', run_command] + command.append('--') command.extend(run_command) - worker['command'] = [['chmod', '+x', 'run-task'], command] + + if is_win: + worker['command'] = [' '.join(command)] + else: + worker['command'] = [ + ['chmod', '+x', 'run-task'], + command, + ] diff --git a/taskcluster/taskgraph/transforms/task.py b/taskcluster/taskgraph/transforms/task.py index 5246218aff44..31353e96bf49 100644 --- a/taskcluster/taskgraph/transforms/task.py +++ b/taskcluster/taskgraph/transforms/task.py @@ -1880,8 +1880,8 @@ def check_caches_are_volumes(task): return raise Exception('task %s (image %s) has caches that are not declared as ' - 'Docker volumes: %s' - 'Have you added them as VOLUMEs in the Dockerfile?' + 'Docker volumes: %s ' + '(have you added them as VOLUMEs in the Dockerfile?)' % (task['label'], task['worker']['docker-image'], ', '.join(sorted(missing)))) From ce87b08ac4c54de2a7e16db157036cdb43c1f66f Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Wed, 5 Dec 2018 19:20:16 +0000 Subject: [PATCH 25/34] Bug 1436037 - [python] Create Windows python-test tasks, r=gps The following python-test paths are being moved out of 'make check' and into their own task: - python/mozlint - testing/mozbase - tools/lint The following python-test paths previously did not run on Windows: - python/mozterm - testing/marionette - testing/raptor - tools/tryselect Differential Revision: https://phabricator.services.mozilla.com/D10759 --HG-- extra : moz-landing-system : lando --- python/mozlint/test/python.ini | 2 +- taskcluster/ci/source-test/python.yml | 21 +++++++++++++++++++ .../harness_unit/test_marionette_runner.py | 4 +++- .../mozbase/manifestparser/tests/manifest.ini | 2 +- testing/mozbase/mozcrash/tests/manifest.ini | 2 +- testing/mozbase/mozdebug/tests/manifest.ini | 2 +- testing/mozbase/mozdevice/tests/manifest.ini | 2 +- testing/mozbase/mozfile/tests/manifest.ini | 2 +- testing/mozbase/mozhttpd/tests/manifest.ini | 2 +- testing/mozbase/mozinfo/tests/manifest.ini | 2 +- testing/mozbase/mozinstall/tests/manifest.ini | 2 +- testing/mozbase/mozlog/tests/manifest.ini | 2 +- testing/mozbase/moznetwork/tests/manifest.ini | 2 +- testing/mozbase/mozprocess/tests/manifest.ini | 2 +- testing/mozbase/mozprofile/tests/manifest.ini | 2 +- testing/mozbase/mozrunner/tests/manifest.ini | 2 +- .../mozsystemmonitor/tests/manifest.ini | 2 +- testing/mozbase/moztest/tests/manifest.ini | 2 +- testing/mozbase/mozversion/tests/manifest.ini | 2 +- tools/lint/test/python.ini | 2 +- 20 files changed, 42 insertions(+), 19 deletions(-) diff --git a/python/mozlint/test/python.ini b/python/mozlint/test/python.ini index 1b066797da94..df427c4064da 100644 --- a/python/mozlint/test/python.ini +++ b/python/mozlint/test/python.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozlint, os == "linux" +subsuite = mozlint skip-if = python == 3 [test_cli.py] diff --git a/taskcluster/ci/source-test/python.yml b/taskcluster/ci/source-test/python.yml index 5b94f40ce47a..4130022c3428 100644 --- a/taskcluster/ci/source-test/python.yml +++ b/taskcluster/ci/source-test/python.yml @@ -4,11 +4,14 @@ job-defaults: worker-type: by-platform: linux64.*: aws-provisioner-v1/gecko-t-linux-xlarge + windows10-64.*: aws-provisioner-v1/gecko-t-win10-64 worker: by-platform: linux64.*: docker-image: {in-tree: "lint"} max-run-time: 3600 + default: + max-run-time: 3600 treeherder: kind: test tier: 2 @@ -34,6 +37,9 @@ taskgraph-tests: marionette-harness: description: testing/marionette/harness unit tests + platform: + - linux64/opt + - windows10-64/opt python-version: [2] treeherder: symbol: mnh @@ -89,6 +95,9 @@ mochitest-harness: mozbase: description: testing/mozbase unit tests + platform: + - linux64/opt + - windows10-64/opt python-version: [2, 3] treeherder: symbol: mb @@ -115,6 +124,9 @@ mozharness: mozlint: description: python/mozlint unit tests + platform: + - linux64/opt + - windows10-64/opt python-version: [2] treeherder: symbol: ml @@ -140,6 +152,9 @@ mozrelease: mozterm: description: python/mozterm unit tests + platform: + - linux64/opt + - windows10-64/opt python-version: [2, 3] treeherder: symbol: term @@ -164,6 +179,9 @@ mozversioncontrol: raptor: description: testing/raptor unit tests + platform: + - linux64/opt + - windows10-64/opt python-version: [2] treeherder: symbol: rap @@ -215,6 +233,9 @@ reftest-harness: tryselect: description: tools/tryselect unit tests + platform: + - linux64/opt + - windows10-64/opt python-version: [2] treeherder: symbol: try diff --git a/testing/marionette/harness/marionette_harness/tests/harness_unit/test_marionette_runner.py b/testing/marionette/harness/marionette_harness/tests/harness_unit/test_marionette_runner.py index 55ed5416f28d..64728434c193 100644 --- a/testing/marionette/harness/marionette_harness/tests/harness_unit/test_marionette_runner.py +++ b/testing/marionette/harness/marionette_harness/tests/harness_unit/test_marionette_runner.py @@ -4,6 +4,8 @@ from __future__ import absolute_import +import os + import manifestparser import mozunit import pytest @@ -296,7 +298,7 @@ def test_add_test_directory(runner): runner.add_test(test_dir) assert isdir.called and walk.called for test in runner.tests: - assert test_dir in test['filepath'] + assert os.path.normpath(test_dir) in test['filepath'] assert len(runner.tests) == 2 diff --git a/testing/mozbase/manifestparser/tests/manifest.ini b/testing/mozbase/manifestparser/tests/manifest.ini index 09a7506ff6c8..675a6eaad358 100644 --- a/testing/mozbase/manifestparser/tests/manifest.ini +++ b/testing/mozbase/manifestparser/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase skip-if = python == 3 [test_expressionparser.py] [test_manifestparser.py] diff --git a/testing/mozbase/mozcrash/tests/manifest.ini b/testing/mozbase/mozcrash/tests/manifest.ini index a6473efb7d08..6918797a0115 100644 --- a/testing/mozbase/mozcrash/tests/manifest.ini +++ b/testing/mozbase/mozcrash/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase skip-if = python == 3 [test_basic.py] [test_java_exception.py] diff --git a/testing/mozbase/mozdebug/tests/manifest.ini b/testing/mozbase/mozdebug/tests/manifest.ini index 5c0376590f99..72aff7539a2a 100644 --- a/testing/mozbase/mozdebug/tests/manifest.ini +++ b/testing/mozbase/mozdebug/tests/manifest.ini @@ -1,3 +1,3 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase [test.py] diff --git a/testing/mozbase/mozdevice/tests/manifest.ini b/testing/mozbase/mozdevice/tests/manifest.ini index c078c87e7349..34d44605f073 100644 --- a/testing/mozbase/mozdevice/tests/manifest.ini +++ b/testing/mozbase/mozdevice/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase skip-if = python == 3 [test_socket_connection.py] [test_is_app_installed.py] diff --git a/testing/mozbase/mozfile/tests/manifest.ini b/testing/mozbase/mozfile/tests/manifest.ini index 43739b7f60e1..815233b70e31 100644 --- a/testing/mozbase/mozfile/tests/manifest.ini +++ b/testing/mozbase/mozfile/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase [test_extract.py] [test_load.py] [test_move_remove.py] diff --git a/testing/mozbase/mozhttpd/tests/manifest.ini b/testing/mozbase/mozhttpd/tests/manifest.ini index df5af9096d2f..dde73a6963b8 100644 --- a/testing/mozbase/mozhttpd/tests/manifest.ini +++ b/testing/mozbase/mozhttpd/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase [api.py] skip-if = python == 3 [baseurl.py] diff --git a/testing/mozbase/mozinfo/tests/manifest.ini b/testing/mozbase/mozinfo/tests/manifest.ini index 5c0376590f99..72aff7539a2a 100644 --- a/testing/mozbase/mozinfo/tests/manifest.ini +++ b/testing/mozbase/mozinfo/tests/manifest.ini @@ -1,3 +1,3 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase [test.py] diff --git a/testing/mozbase/mozinstall/tests/manifest.ini b/testing/mozbase/mozinstall/tests/manifest.ini index a74a58151033..8c8875839f89 100644 --- a/testing/mozbase/mozinstall/tests/manifest.ini +++ b/testing/mozbase/mozinstall/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase skip-if = python == 3 [test_binary.py] [test_install.py] diff --git a/testing/mozbase/mozlog/tests/manifest.ini b/testing/mozbase/mozlog/tests/manifest.ini index 89339a027b03..55b41eb518fa 100644 --- a/testing/mozbase/mozlog/tests/manifest.ini +++ b/testing/mozbase/mozlog/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase [test_logger.py] skip-if = python == 3 [test_logtypes.py] diff --git a/testing/mozbase/moznetwork/tests/manifest.ini b/testing/mozbase/moznetwork/tests/manifest.ini index 8c2c895d0a22..946dd6a46bd9 100644 --- a/testing/mozbase/moznetwork/tests/manifest.ini +++ b/testing/mozbase/moznetwork/tests/manifest.ini @@ -1,4 +1,4 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase skip-if = python == 3 [test.py] diff --git a/testing/mozbase/mozprocess/tests/manifest.ini b/testing/mozbase/mozprocess/tests/manifest.ini index cc93006cddf1..4eedb812ca42 100644 --- a/testing/mozbase/mozprocess/tests/manifest.ini +++ b/testing/mozbase/mozprocess/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase skip-if = python == 3 [test_detached.py] skip-if = os == "win" # Bug 1493796 diff --git a/testing/mozbase/mozprofile/tests/manifest.ini b/testing/mozbase/mozprofile/tests/manifest.ini index 74cfec024730..d69f4649cea2 100644 --- a/testing/mozbase/mozprofile/tests/manifest.ini +++ b/testing/mozbase/mozprofile/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase [test_addonid.py] [test_server_locations.py] [test_preferences.py] diff --git a/testing/mozbase/mozrunner/tests/manifest.ini b/testing/mozbase/mozrunner/tests/manifest.ini index e063e1a85445..0f6f74bb7613 100644 --- a/testing/mozbase/mozrunner/tests/manifest.ini +++ b/testing/mozbase/mozrunner/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase # We skip these tests in automated Windows builds because they trigger crashes # in sh.exe; see bug 1489277. skip-if = python == 3 || (automation && os == "win") diff --git a/testing/mozbase/mozsystemmonitor/tests/manifest.ini b/testing/mozbase/mozsystemmonitor/tests/manifest.ini index ee83d16743c9..4e265d757039 100644 --- a/testing/mozbase/mozsystemmonitor/tests/manifest.ini +++ b/testing/mozbase/mozsystemmonitor/tests/manifest.ini @@ -1,3 +1,3 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase [test_resource_monitor.py] diff --git a/testing/mozbase/moztest/tests/manifest.ini b/testing/mozbase/moztest/tests/manifest.ini index 57af128f2b9b..71707ab6ccfb 100644 --- a/testing/mozbase/moztest/tests/manifest.ini +++ b/testing/mozbase/moztest/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase skip-if = python == 3 [test.py] [test_resolve.py] diff --git a/testing/mozbase/mozversion/tests/manifest.ini b/testing/mozbase/mozversion/tests/manifest.ini index c5ddebee8909..ccdf63088cdf 100644 --- a/testing/mozbase/mozversion/tests/manifest.ini +++ b/testing/mozbase/mozversion/tests/manifest.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite = mozbase, os == "linux" +subsuite = mozbase [test_binary.py] [test_apk.py] diff --git a/tools/lint/test/python.ini b/tools/lint/test/python.ini index 2dcbb573cc0e..4d8120c1a590 100644 --- a/tools/lint/test/python.ini +++ b/tools/lint/test/python.ini @@ -1,5 +1,5 @@ [DEFAULT] -subsuite=mozlint, os == "linux" +subsuite = mozlint skip-if = python == 3 [test_eslint.py] From 0979860740e79317039ce2a05e5f8173b36a894e Mon Sep 17 00:00:00 2001 From: Jan-Ivar Bruaroey Date: Wed, 5 Dec 2018 21:46:51 +0000 Subject: [PATCH 26/34] Bug 1512197 - Temporary fix for incorrect sharing of MozPromiseHolder in MediaManager. r=jya Differential Revision: https://phabricator.services.mozilla.com/D13847 --HG-- extra : moz-landing-system : lando --- dom/media/MediaManager.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/dom/media/MediaManager.cpp b/dom/media/MediaManager.cpp index 1c97fb8fd0f8..893250e354c8 100644 --- a/dom/media/MediaManager.cpp +++ b/dom/media/MediaManager.cpp @@ -1123,8 +1123,14 @@ class GetUserMediaStreamRunnable : public Runnable { "TracksCreatedListener::mTrack", aTrack)) {} ~TracksCreatedListener() { - mHolder.RejectIfExists( - MakeRefPtr(MediaMgrError::Name::AbortError), __func__); + RejectIfExists(MakeRefPtr(MediaMgrError::Name::AbortError), + __func__); + } + + // TODO: The need for this should be removed by an upcoming refactor. + void RejectIfExists(RefPtr&& aError, + const char* aMethodName) { + mHolder.RejectIfExists(std::move(aError), aMethodName); } void NotifyOutput(MediaStreamGraph* aGraph, @@ -1384,7 +1390,7 @@ class GetUserMediaStreamRunnable : public Runnable { manager->SendPendingGUMRequest(); }, [manager = mManager, windowID = mWindowID, - holder = std::move(mHolder)](RefPtr&& aError) mutable { + tracksCreatedListener](RefPtr&& aError) { MOZ_ASSERT(NS_IsMainThread()); LOG( ("GetUserMediaStreamRunnable::Run: starting failure callback " @@ -1397,7 +1403,7 @@ class GetUserMediaStreamRunnable : public Runnable { } // This is safe since we're on main-thread, and the windowlist can // only be invalidated from the main-thread (see OnNavigation) - holder.Reject(std::move(aError), __func__); + tracksCreatedListener->RejectIfExists(std::move(aError), __func__); }); if (!IsPincipalInfoPrivate(mPrincipalInfo)) { From 55fdbb742f77950733aaa7cd77c8f97a72f0ff01 Mon Sep 17 00:00:00 2001 From: alwu Date: Wed, 5 Dec 2018 20:28:52 +0000 Subject: [PATCH 27/34] Bug 1512283 - Turn off doorhanger in Nightly. r=cpearce Now We decide to ship block-autoplay without doorhanger, so we would turn the pref off in Nightly. Differential Revision: https://phabricator.services.mozilla.com/D13853 --HG-- extra : moz-landing-system : lando --- browser/app/profile/firefox.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 859352ee474c..76df13e49e54 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1473,9 +1473,9 @@ pref("media.gmp-widevinecdm.enabled", true); // Switch block autoplay logic to v2, and enable UI. pref("media.autoplay.enabled.user-gestures-needed", true); // Allow asking for permission to autoplay to appear in UI. -pref("media.autoplay.ask-permission", true); +pref("media.autoplay.ask-permission", false); // Set Firefox to block autoplay, asking for permission by default. -pref("media.autoplay.default", 2); // 0=Allowed, 1=Blocked, 2=Prompt +pref("media.autoplay.default", 1); // 0=Allowed, 1=Blocked, 2=Prompt #else pref("media.autoplay.default", 0); // 0=Allowed, 1=Blocked, 2=Prompt pref("media.autoplay.enabled.user-gestures-needed", false); From e7bc28da030663da3fb0584876f0c61c2e347f14 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Wed, 5 Dec 2018 20:08:35 +0000 Subject: [PATCH 28/34] Bug 1511276 - Add UserAgentStyleSheetList.h and use it to simplify nsLayoutStylesheetCache. r=emilio Differential Revision: https://phabricator.services.mozilla.com/D13585 --HG-- extra : moz-landing-system : lando --- layout/style/UserAgentStyleSheetList.h | 35 ++++++ layout/style/moz.build | 1 + layout/style/nsLayoutStylesheetCache.cpp | 138 ++++------------------- layout/style/nsLayoutStylesheetCache.h | 19 +--- 4 files changed, 60 insertions(+), 133 deletions(-) create mode 100644 layout/style/UserAgentStyleSheetList.h diff --git a/layout/style/UserAgentStyleSheetList.h b/layout/style/UserAgentStyleSheetList.h new file mode 100644 index 000000000000..0e8fe4216703 --- /dev/null +++ b/layout/style/UserAgentStyleSheetList.h @@ -0,0 +1,35 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/* list of user agent style sheets that nsLayoutStylesheetCache manages */ + +/* + * STYLE_SHEET(identifier_, url_, lazy_) + * + * identifier_ + * An identifier for the style sheet, suitable for use as an enum class value. + * + * url_ + * The URL of the style sheet. + * + * lazy_ + * A boolean indicating whether the sheet is loaded lazily. + */ + +STYLE_SHEET(ContentEditable, "resource://gre/res/contenteditable.css", true) +STYLE_SHEET(CounterStyles, "resource://gre-resources/counterstyles.css", false) +STYLE_SHEET(DesignMode, "resource://gre/res/designmode.css", true) +STYLE_SHEET(Forms, "resource://gre-resources/forms.css", true) +STYLE_SHEET(HTML, "resource://gre-resources/html.css", false) +STYLE_SHEET(MathML, "resource://gre-resources/mathml.css", true) +STYLE_SHEET(MinimalXUL, "chrome://global/content/minimal-xul.css", false) +STYLE_SHEET(NoFrames, "resource://gre-resources/noframes.css", true) +STYLE_SHEET(NoScript, "resource://gre-resources/noscript.css", true) +STYLE_SHEET(Quirk, "resource://gre-resources/quirk.css", false) +STYLE_SHEET(Scrollbars, "chrome://global/skin/scrollbars.css", true) +STYLE_SHEET(SVG, "resource://gre/res/svg.css", false) +STYLE_SHEET(UA, "resource://gre-resources/ua.css", true) +STYLE_SHEET(XUL, "chrome://global/content/xul.css", true) diff --git a/layout/style/moz.build b/layout/style/moz.build index 4416391ed325..3a675052ef48 100644 --- a/layout/style/moz.build +++ b/layout/style/moz.build @@ -112,6 +112,7 @@ EXPORTS.mozilla += [ 'StyleSheetInfo.h', 'StyleSheetInlines.h', 'URLExtraData.h', + 'UserAgentStyleSheetList.h', ] EXPORTS.mozilla.dom += [ diff --git a/layout/style/nsLayoutStylesheetCache.cpp b/layout/style/nsLayoutStylesheetCache.cpp index 4f0eef64418d..db4630db5aca 100644 --- a/layout/style/nsLayoutStylesheetCache.cpp +++ b/layout/style/nsLayoutStylesheetCache.cpp @@ -59,25 +59,15 @@ nsresult nsLayoutStylesheetCache::Observe(nsISupports* aSubject, return NS_OK; } -StyleSheet* nsLayoutStylesheetCache::ScrollbarsSheet() { - if (!mScrollbarsSheet) { - // Scrollbars don't need access to unsafe rules - LoadSheetURL("chrome://global/skin/scrollbars.css", &mScrollbarsSheet, - eSafeAgentSheetFeatures, eCrash); +#define STYLE_SHEET(identifier_, url_, lazy_) \ + StyleSheet* nsLayoutStylesheetCache::identifier_##Sheet() { \ + if (lazy_ && !m##identifier_##Sheet) { \ + LoadSheetURL(url_, &m##identifier_##Sheet, eAgentSheetFeatures, eCrash); \ + } \ + return m##identifier_##Sheet; \ } - - return mScrollbarsSheet; -} - -StyleSheet* nsLayoutStylesheetCache::FormsSheet() { - if (!mFormsSheet) { - // forms.css needs access to unsafe rules - LoadSheetURL("resource://gre-resources/forms.css", &mFormsSheet, - eAgentSheetFeatures, eCrash); - } - - return mFormsSheet; -} +#include "mozilla/UserAgentStyleSheetList.h" +#undef STYLE_SHEET StyleSheet* nsLayoutStylesheetCache::UserContentSheet() { return mUserContentSheet; @@ -87,65 +77,6 @@ StyleSheet* nsLayoutStylesheetCache::UserChromeSheet() { return mUserChromeSheet; } -StyleSheet* nsLayoutStylesheetCache::UASheet() { - if (!mUASheet) { - LoadSheetURL("resource://gre-resources/ua.css", &mUASheet, - eAgentSheetFeatures, eCrash); - } - - return mUASheet; -} - -StyleSheet* nsLayoutStylesheetCache::HTMLSheet() { return mHTMLSheet; } - -StyleSheet* nsLayoutStylesheetCache::MinimalXULSheet() { - return mMinimalXULSheet; -} - -StyleSheet* nsLayoutStylesheetCache::XULSheet() { - if (!mXULSheet) { - LoadSheetURL("chrome://global/content/xul.css", &mXULSheet, - eAgentSheetFeatures, eCrash); - } - - return mXULSheet; -} - -StyleSheet* nsLayoutStylesheetCache::QuirkSheet() { return mQuirkSheet; } - -StyleSheet* nsLayoutStylesheetCache::SVGSheet() { return mSVGSheet; } - -StyleSheet* nsLayoutStylesheetCache::MathMLSheet() { - if (!mMathMLSheet) { - LoadSheetURL("resource://gre-resources/mathml.css", &mMathMLSheet, - eAgentSheetFeatures, eCrash); - } - - return mMathMLSheet; -} - -StyleSheet* nsLayoutStylesheetCache::CounterStylesSheet() { - return mCounterStylesSheet; -} - -StyleSheet* nsLayoutStylesheetCache::NoScriptSheet() { - if (!mNoScriptSheet) { - LoadSheetURL("resource://gre-resources/noscript.css", &mNoScriptSheet, - eAgentSheetFeatures, eCrash); - } - - return mNoScriptSheet; -} - -StyleSheet* nsLayoutStylesheetCache::NoFramesSheet() { - if (!mNoFramesSheet) { - LoadSheetURL("resource://gre-resources/noframes.css", &mNoFramesSheet, - eAgentSheetFeatures, eCrash); - } - - return mNoFramesSheet; -} - StyleSheet* nsLayoutStylesheetCache::ChromePreferenceSheet( nsPresContext* aPresContext) { if (!mChromePreferenceSheet) { @@ -164,24 +95,6 @@ StyleSheet* nsLayoutStylesheetCache::ContentPreferenceSheet( return mContentPreferenceSheet; } -StyleSheet* nsLayoutStylesheetCache::ContentEditableSheet() { - if (!mContentEditableSheet) { - LoadSheetURL("resource://gre/res/contenteditable.css", - &mContentEditableSheet, eAgentSheetFeatures, eCrash); - } - - return mContentEditableSheet; -} - -StyleSheet* nsLayoutStylesheetCache::DesignModeSheet() { - if (!mDesignModeSheet) { - LoadSheetURL("resource://gre/res/designmode.css", &mDesignModeSheet, - eAgentSheetFeatures, eCrash); - } - - return mDesignModeSheet; -} - void nsLayoutStylesheetCache::Shutdown() { gCSSLoader = nullptr; NS_WARNING_ASSERTION(!gStyleCache || !gUserContentSheetURL, @@ -214,24 +127,14 @@ size_t nsLayoutStylesheetCache::SizeOfIncludingThis( #define MEASURE(s) n += s ? s->SizeOfIncludingThis(aMallocSizeOf) : 0; +#define STYLE_SHEET(identifier_, url_, lazy_) MEASURE(m##identifier_##Sheet); +#include "mozilla/UserAgentStyleSheetList.h" +#undef STYLE_SHEET + MEASURE(mChromePreferenceSheet); - MEASURE(mContentEditableSheet); MEASURE(mContentPreferenceSheet); - MEASURE(mCounterStylesSheet); - MEASURE(mDesignModeSheet); - MEASURE(mFormsSheet); - MEASURE(mHTMLSheet); - MEASURE(mMathMLSheet); - MEASURE(mMinimalXULSheet); - MEASURE(mNoFramesSheet); - MEASURE(mNoScriptSheet); - MEASURE(mQuirkSheet); - MEASURE(mSVGSheet); - MEASURE(mScrollbarsSheet); - MEASURE(mUASheet); MEASURE(mUserChromeSheet); MEASURE(mUserContentSheet); - MEASURE(mXULSheet); // Measurement of the following members may be added later if DMD finds it is // worthwhile: @@ -255,16 +158,13 @@ nsLayoutStylesheetCache::nsLayoutStylesheetCache() { // And make sure that we load our UA sheets. No need to do this // per-profile, since they're profile-invariant. - LoadSheetURL("resource://gre-resources/counterstyles.css", - &mCounterStylesSheet, eAgentSheetFeatures, eCrash); - LoadSheetURL("resource://gre-resources/html.css", &mHTMLSheet, - eAgentSheetFeatures, eCrash); - LoadSheetURL("chrome://global/content/minimal-xul.css", &mMinimalXULSheet, - eAgentSheetFeatures, eCrash); - LoadSheetURL("resource://gre-resources/quirk.css", &mQuirkSheet, - eAgentSheetFeatures, eCrash); - LoadSheetURL("resource://gre/res/svg.css", &mSVGSheet, eAgentSheetFeatures, - eCrash); +#define STYLE_SHEET(identifier_, url_, lazy_) \ + if (!lazy_) { \ + LoadSheetURL(url_, &m##identifier_##Sheet, eAgentSheetFeatures, eCrash); \ + } +#include "mozilla/UserAgentStyleSheetList.h" +#undef STYLE_SHEET + if (XRE_IsParentProcess()) { // We know we need xul.css for the UI, so load that now too: XULSheet(); diff --git a/layout/style/nsLayoutStylesheetCache.h b/layout/style/nsLayoutStylesheetCache.h index d28598d93b5c..d5880bcbae1c 100644 --- a/layout/style/nsLayoutStylesheetCache.h +++ b/layout/style/nsLayoutStylesheetCache.h @@ -38,24 +38,15 @@ class nsLayoutStylesheetCache final : public nsIObserver, static nsLayoutStylesheetCache* Singleton(); - mozilla::StyleSheet* ScrollbarsSheet(); - mozilla::StyleSheet* FormsSheet(); +#define STYLE_SHEET(identifier_, url_, lazy_) \ + mozilla::StyleSheet* identifier_##Sheet(); +#include "mozilla/UserAgentStyleSheetList.h" +#undef STYLE_SHEET + mozilla::StyleSheet* UserContentSheet(); mozilla::StyleSheet* UserChromeSheet(); - mozilla::StyleSheet* UASheet(); - mozilla::StyleSheet* HTMLSheet(); - mozilla::StyleSheet* MinimalXULSheet(); - mozilla::StyleSheet* XULSheet(); - mozilla::StyleSheet* QuirkSheet(); - mozilla::StyleSheet* SVGSheet(); - mozilla::StyleSheet* MathMLSheet(); - mozilla::StyleSheet* CounterStylesSheet(); - mozilla::StyleSheet* NoScriptSheet(); - mozilla::StyleSheet* NoFramesSheet(); mozilla::StyleSheet* ChromePreferenceSheet(nsPresContext* aPresContext); mozilla::StyleSheet* ContentPreferenceSheet(nsPresContext* aPresContext); - mozilla::StyleSheet* ContentEditableSheet(); - mozilla::StyleSheet* DesignModeSheet(); static void InvalidatePreferenceSheets(); From ca62de346e049d8c969c8cf9aeab91cae6c85ed5 Mon Sep 17 00:00:00 2001 From: Mark Striemer Date: Tue, 4 Dec 2018 20:06:30 +0000 Subject: [PATCH 29/34] Bug 1506987 - Include preview for theme in AOM if it is added while open r=aswan Differential Revision: https://phabricator.services.mozilla.com/D12593 --HG-- extra : moz-landing-system : lando --- .../mozapps/extensions/content/extensions.js | 30 +++----- .../mozapps/extensions/content/extensions.xml | 9 ++- .../mozapps/extensions/content/extensions.xul | 2 +- .../extensions/test/browser/browser.ini | 1 + .../test/browser/browser_theme_previews.js | 72 +++++++++++++++++++ 5 files changed, 90 insertions(+), 24 deletions(-) create mode 100644 toolkit/mozapps/extensions/test/browser/browser_theme_previews.js diff --git a/toolkit/mozapps/extensions/content/extensions.js b/toolkit/mozapps/extensions/content/extensions.js index 57e08559d555..43aec6e2bc06 100644 --- a/toolkit/mozapps/extensions/content/extensions.js +++ b/toolkit/mozapps/extensions/content/extensions.js @@ -290,26 +290,6 @@ function setSearchLabel(type) { } } -function setThemeScreenshot(addon, node) { - let findElement = () => node.querySelector(".theme-screenshot") - || document.getAnonymousElementByAttribute(node, "anonid", "theme-screenshot"); - let screenshot = findElement(); - if (!screenshot) { - // Force a layout since screenshot might not exist yet on Windows. - node.clientTop; - screenshot = findElement(); - } - // There's a test that doesn't have this for some reason, but it's doing weird things. - if (!screenshot) - return; - if (addon.type == "theme" && addon.screenshots && addon.screenshots.length > 0) { - screenshot.setAttribute("src", addon.screenshots[0].url); - screenshot.hidden = false; - } else { - screenshot.hidden = true; - } -} - /** * Obtain the main DOMWindow for the current context. */ @@ -2447,7 +2427,6 @@ var gListView = { sortElements(elements, ["uiState", "name"], true); for (let element of elements) { this._listBox.appendChild(element); - setThemeScreenshot(element.mAddon, element); } } @@ -2615,7 +2594,14 @@ var gDetailView = { _updateView(aAddon, aIsRemote, aScrollToPreferences) { setSearchLabel(aAddon.type); - setThemeScreenshot(aAddon, this.node); + + // Set the preview image for themes, if available. + if (aAddon.type == "theme") { + let previewURL = aAddon.screenshots && aAddon.screenshots[0] && aAddon.screenshots[0].url; + if (previewURL) { + this.node.querySelector(".card-heading-image").src = previewURL; + } + } AddonManager.addManagerListener(this); this.clearLoading(); diff --git a/toolkit/mozapps/extensions/content/extensions.xml b/toolkit/mozapps/extensions/content/extensions.xml index 55d163743313..556f7f933426 100644 --- a/toolkit/mozapps/extensions/content/extensions.xml +++ b/toolkit/mozapps/extensions/content/extensions.xml @@ -591,7 +591,7 @@ -