diff --git a/browser/components/payments/res/containers/basic-card-form.js b/browser/components/payments/res/containers/basic-card-form.js index 58b3254d13cd..6df07494d248 100644 --- a/browser/components/payments/res/containers/basic-card-form.js +++ b/browser/components/payments/res/containers/basic-card-form.js @@ -118,8 +118,9 @@ export default class BasicCardForm extends PaymentStateSubscriberMixin(HTMLEleme this.addressAddLink.textContent = this.dataset.addressAddLinkLabel; this.addressEditLink.textContent = this.dataset.addressEditLinkLabel; - // The back button is temporarily hidden(See Bug 1462461). - this.backButton.hidden = !!page.onboardingWizard; + // The next line needs an onboarding check since we don't set previousId + // when navigating to add/edit directly from the summary page. + this.backButton.hidden = !page.previousId && page.onboardingWizard; this.cancelButton.hidden = !page.onboardingWizard; let record = {}; @@ -210,11 +211,39 @@ export default class BasicCardForm extends PaymentStateSubscriberMixin(HTMLEleme break; } case this.backButton: { - this.requestStore.setState({ + let { + page, + request, + "address-page": addressPage, + "basic-card-page": basicCardPage, + selectedShippingAddress, + } = this.requestStore.getState(); + + let nextState = { page: { - id: "payment-summary", + id: page.previousId || "payment-summary", + onboardingWizard: page.onboardingWizard, }, - }); + }; + + let addressPageState; + if (page.onboardingWizard) { + if (request.paymentOptions.requestShipping) { + addressPageState = Object.assign({}, addressPage, {guid: selectedShippingAddress}); + } else { + addressPageState = + Object.assign({}, addressPage, {guid: basicCardPage.billingAddressGUID}); + } + + let basicCardPageState = Object.assign({}, basicCardPage, {preserveFieldValues: true}); + + Object.assign(nextState, { + "address-page": addressPageState, + "basic-card-page": basicCardPageState, + }); + } + + this.requestStore.setState(nextState); break; } case this.saveButton: { diff --git a/browser/components/payments/test/browser/browser_payments_onboarding_wizard.js b/browser/components/payments/test/browser/browser_payments_onboarding_wizard.js index 49d094469e43..be9fbf608b88 100644 --- a/browser/components/payments/test/browser/browser_payments_onboarding_wizard.js +++ b/browser/components/payments/test/browser/browser_payments_onboarding_wizard.js @@ -406,3 +406,98 @@ add_task(async function test_on_boarding_wizard_with_requestShipping_turned_off_ cleanupFormAutofillStorage(); }); }); + +add_task(async function test_back_button_on_basic_card_page_during_onboarding() { + await BrowserTestUtils.withNewTab({ + gBrowser, + url: BLANK_PAGE_URL, + }, async browser => { + cleanupFormAutofillStorage(); + + info("Opening the payment dialog"); + let {win, frame} = + await setupPaymentDialog(browser, { + methodData: [PTU.MethodData.basicCard], + details: PTU.Details.total60USD, + merchantTaskFn: PTU.ContentTasks.createAndShowRequest, + }); + + await spawnPaymentDialogTask(frame, async function() { + let { + PaymentTestUtils: PTU, + } = ChromeUtils.import("resource://testing-common/PaymentTestUtils.jsm", {}); + + await PTU.DialogContentUtils.waitForState(content, (state) => { + return state.page.id == "address-page"; + }, "Address page is shown first if there are saved addresses during on boarding"); + + info("Checking if the address page has been rendered"); + let addressSaveButton = content.document.querySelector("address-form .save-button"); + ok(content.isVisible(addressSaveButton), "Address save button is rendered"); + + for (let [key, val] of Object.entries(PTU.Addresses.TimBL2)) { + let field = content.document.getElementById(key); + if (!field) { + ok(false, `${key} field not found`); + } + field.value = val; + ok(!field.disabled, `Field #${key} shouldn't be disabled`); + } + content.document.querySelector("address-form .save-button").click(); + + await PTU.DialogContentUtils.waitForState(content, (state) => { + return state.page.id == "basic-card-page"; + }, "Basic card page is shown next"); + + info("Checking if basic card page is rendered"); + let basicCardBackButton = content.document.querySelector("basic-card-form .back-button"); + ok(content.isVisible(basicCardBackButton), "Back button is visible on the basic card page"); + + info("Partially fill basic card form"); + let field = content.document.getElementById("cc-number"); + field.value = PTU.BasicCards.JohnDoe["cc-number"]; + + info("Clicking on the back button to edit address saved in the previous step"); + basicCardBackButton.click(); + + await PTU.DialogContentUtils.waitForState(content, (state) => { + return state.page.id == "address-page" && + state["address-page"].guid == state["basic-card-page"].billingAddressGUID; + }, "Address page is shown again"); + + info("Checking if the address page has been rendered"); + addressSaveButton = content.document.querySelector("address-form .save-button"); + ok(content.isVisible(addressSaveButton), "Address save button is rendered"); + + info("Checking if the address saved in the last step is correctly loaded in the form"); + field = content.document.getElementById("given-name"); + ok(field.value, PTU.Addresses.TimBL2["given-name"], + "Given name field value is correctly loaded"); + + info("Editing the address and saving again"); + field.value = "John"; + addressSaveButton.click(); + + info("Checking if the address was correctly edited"); + await PTU.DialogContentUtils.waitForState(content, (state) => { + return state.page.id == "basic-card-page" && + // eslint-disable-next-line max-len + state.savedAddresses[state["basic-card-page"].billingAddressGUID]["given-name"] == "John"; + }, "Address was correctly edited and saved"); + + // eslint-disable-next-line max-len + info("Checking if the basic card form is now rendered and if the field values from before are preserved"); + let basicCardCancelButton = content.document.querySelector("basic-card-form .cancel-button"); + ok(content.isVisible(basicCardCancelButton), + "Cancel button is visible on the basic card page"); + field = content.document.getElementById("cc-number"); + ok(field.value, PTU.BasicCards.JohnDoe["cc-number"], "Values in the form are preserved"); + }); + + info("Closing the payment dialog"); + spawnPaymentDialogTask(frame, PTU.DialogContentTasks.manuallyClickCancel); + await BrowserTestUtils.waitForCondition(() => win.closed, "dialog should be closed"); + + cleanupFormAutofillStorage(); + }); +}); diff --git a/devtools/client/inspector/animation/components/AnimationItem.js b/devtools/client/inspector/animation/components/AnimationItem.js index 5fbfa4d0479e..f1ba35f70ca3 100644 --- a/devtools/client/inspector/animation/components/AnimationItem.js +++ b/devtools/client/inspector/animation/components/AnimationItem.js @@ -34,16 +34,13 @@ class AnimationItem extends Component { super(props); this.state = { - isSelected: false, + isSelected: this.isSelected(props), }; } componentWillReceiveProps(nextProps) { - const { animation } = this.props; - this.setState({ - isSelected: nextProps.selectedAnimation && - animation.actorID === nextProps.selectedAnimation.actorID + isSelected: this.isSelected(nextProps), }); } @@ -53,6 +50,11 @@ class AnimationItem extends Component { this.props.timeScale !== nextProps.timeScale; } + isSelected(props) { + return props.selectedAnimation && + props.animation.actorID === props.selectedAnimation.actorID; + } + render() { const { animation, diff --git a/devtools/client/inspector/animation/reducers/animations.js b/devtools/client/inspector/animation/reducers/animations.js index 4f677b8e960c..340a00f34981 100644 --- a/devtools/client/inspector/animation/reducers/animations.js +++ b/devtools/client/inspector/animation/reducers/animations.js @@ -48,8 +48,12 @@ const reducers = { }, [UPDATE_DETAIL_VISIBILITY](state, { detailVisibility }) { + const selectedAnimation = + detailVisibility ? state.selectedAnimation : null; + return Object.assign({}, state, { - detailVisibility + detailVisibility, + selectedAnimation, }); }, diff --git a/devtools/client/inspector/animation/test/browser.ini b/devtools/client/inspector/animation/test/browser.ini index 4536a8705404..d64e5b74672b 100644 --- a/devtools/client/inspector/animation/test/browser.ini +++ b/devtools/client/inspector/animation/test/browser.ini @@ -26,6 +26,8 @@ support-files = [browser_animation_animation-detail_title.js] [browser_animation_animation-detail_visibility.js] [browser_animation_animation-list.js] +[browser_animation_animation-list_one-animation-select.js] +[browser_animation_animation-list_select.js] [browser_animation_animation-target.js] [browser_animation_animation-target_highlight.js] [browser_animation_animation-target_select.js] diff --git a/devtools/client/inspector/animation/test/browser_animation_animated-property-list.js b/devtools/client/inspector/animation/test/browser_animation_animated-property-list.js index 6aaabd663c08..fdb0eb41943b 100644 --- a/devtools/client/inspector/animation/test/browser_animation_animated-property-list.js +++ b/devtools/client/inspector/animation/test/browser_animation_animated-property-list.js @@ -38,17 +38,5 @@ add_task(async function() { is(itemEls.length, expectedNumber, `The number of animated-property-list should be ${ expectedNumber } ` + `at ${ targetClass }`); - - if (itemEls.length < 2) { - continue; - } - - info("Checking the background color for " + - `the animated property item at ${ targetClass }`); - const evenColor = panel.ownerGlobal.getComputedStyle(itemEls[0]).backgroundColor; - const oddColor = panel.ownerGlobal.getComputedStyle(itemEls[1]).backgroundColor; - isnot(evenColor, oddColor, - "Background color of an even animated property item " + - "should be different from odd"); } }); diff --git a/devtools/client/inspector/animation/test/browser_animation_animation-list.js b/devtools/client/inspector/animation/test/browser_animation_animation-list.js index 7ed5ce0da240..aa71186f565e 100644 --- a/devtools/client/inspector/animation/test/browser_animation_animation-list.js +++ b/devtools/client/inspector/animation/test/browser_animation_animation-list.js @@ -17,15 +17,6 @@ add_task(async function() { animationInspector.state.animations.length, "The number of animations displayed matches the number of animations"); - info("Checking the background color for the animation list items"); - const animationItemEls = panel.querySelectorAll(".animation-list .animation-item"); - const evenColor = - panel.ownerGlobal.getComputedStyle(animationItemEls[0]).backgroundColor; - const oddColor = - panel.ownerGlobal.getComputedStyle(animationItemEls[1]).backgroundColor; - isnot(evenColor, oddColor, - "Background color of an even animation should be different from odd"); - info("Checking list and items existence after select a element which has an animation"); await selectNodeAndWaitForAnimations(".animated", inspector); is(panel.querySelectorAll(".animation-list .animation-item").length, 1, diff --git a/devtools/client/inspector/animation/test/browser_animation_animation-list_one-animation-select.js b/devtools/client/inspector/animation/test/browser_animation_animation-list_one-animation-select.js new file mode 100644 index 000000000000..5d44e7a412e0 --- /dev/null +++ b/devtools/client/inspector/animation/test/browser_animation_animation-list_one-animation-select.js @@ -0,0 +1,22 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Test whether the animation item has been selected from first time +// if count of the animations is one. + +add_task(async function() { + await addTab(URL_ROOT + "doc_simple_animation.html"); + await removeAnimatedElementsExcept([".animated"]); + const { panel } = await openAnimationInspector(); + + info("Checking whether an item element has been selected"); + is(panel.querySelector(".animation-item").classList.contains("selected"), true, + "The animation item should have 'selected' class"); + + info("Checking whether the element will be unselected after closing the detail pane"); + clickOnDetailCloseButton(panel); + is(panel.querySelector(".animation-item").classList.contains("selected"), false, + "The animation item should not have 'selected' class"); +}); diff --git a/devtools/client/inspector/animation/test/browser_animation_animation-list_select.js b/devtools/client/inspector/animation/test/browser_animation_animation-list_select.js new file mode 100644 index 000000000000..1e74589f2c51 --- /dev/null +++ b/devtools/client/inspector/animation/test/browser_animation_animation-list_select.js @@ -0,0 +1,33 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Test whether the animation items in the list were selectable. + +add_task(async function() { + await addTab(URL_ROOT + "doc_simple_animation.html"); + await removeAnimatedElementsExcept([".animated", ".long"]); + const { animationInspector, panel } = await openAnimationInspector(); + + info("Checking whether 1st element will be selected"); + await clickOnAnimation(animationInspector, panel, 0); + assertSelection(panel, [true, false]); + + info("Checking whether 2nd element will be selected"); + await clickOnAnimation(animationInspector, panel, 1); + assertSelection(panel, [false, true]); + + info("Checking whether all elements will be unselected after closing the detail pane"); + clickOnDetailCloseButton(panel); + assertSelection(panel, [false, false]); +}); + +function assertSelection(panel, expectedResult) { + panel.querySelectorAll(".animation-item").forEach((item, index) => { + const shouldSelected = expectedResult[index]; + is(item.classList.contains("selected"), shouldSelected, + `Animation item[${ index }] should ` + + `${ shouldSelected ? "" : "not" } have 'selected' class`); + }); +} diff --git a/devtools/client/themes/animation.css b/devtools/client/themes/animation.css index 488d2b43d505..47add35af50a 100644 --- a/devtools/client/themes/animation.css +++ b/devtools/client/themes/animation.css @@ -5,31 +5,33 @@ /* Animation-inspector specific theme variables */ :root { - --animation-even-background-color: rgba(0, 0, 0, 0.05); + --animation-item-hover-color: var(--grey-30-a40); + --animation-item-selected-color: var(--grey-30-a90); --command-pick-image: url(chrome://devtools/skin/images/command-pick.svg); + --cssanimation-color: var(--purple-50); + --csstransition-color: var(--blue-55); --devtools-toolbar-height: 24px; --fast-track-image: url("images/animation-fast-track.svg"); - --fill-color-cssanimation: var(--theme-contrast-background); - --fill-color-csstransition: var(--theme-highlight-blue); - --fill-color-scriptanimation: var(--theme-graphs-green); --graph-height: 30px; --graph-right-offset: 10px; --keyframe-marker-shadow-color: #c4c4c4; --pause-image: url(chrome://devtools/skin/images/pause.svg); - --progress-bar-color: #909090; + --progress-bar-color: var(--grey-40); --resume-image: url(chrome://devtools/skin/images/play.svg); --rewind-image: url(chrome://devtools/skin/images/rewind.svg); - --scrubber-color: #dd00a9; + --scriptanimation-color: var(--green-60); + --scrubber-color: var(--magenta-65); --sidebar-width: 200px; - --stroke-color-cssanimation: var(--theme-highlight-lightorange); - --stroke-color-csstransition: var(--theme-highlight-bluegrey); - --stroke-color-scriptanimation: var(--theme-highlight-green); - --tick-line-style: 0.5px solid rgba(128, 136, 144, 0.5); + --tick-line-style: 0.5px solid var(--theme-splitter-color); } :root.theme-dark { - --animation-even-background-color: rgba(255, 255, 255, 0.05); + --animation-item-hover-color: var(--grey-60-a50); + --animation-item-selected-color: var(--grey-60); + --csstransition-color: var(--blue-50); --keyframe-marker-shadow-color: #818181; + --progress-bar-color: var(--grey-50); + --scrubber-color: var(--magenta-50); } /* Root element of animation inspector */ @@ -132,27 +134,27 @@ select.playback-rate-selector.devtools-button:not(:empty):not(:disabled):not(.ch } /* Animation Item */ -.animation-item:nth-child(2n+1) { - background-color: var(--animation-even-background-color); -} - .animation-item.cssanimation { - --computed-timing-graph-color: var(--fill-color-cssanimation); - --effect-timing-graph-color: var(--stroke-color-cssanimation); + --graph-color: var(--cssanimation-color); + --graph-opacity: 0.7; } .animation-item.csstransition { - --computed-timing-graph-color: var(--fill-color-csstransition); - --effect-timing-graph-color: var(--stroke-color-csstransition); + --graph-color: var(--csstransition-color); + --graph-opacity: 0.8; } .animation-item.scriptanimation { - --computed-timing-graph-color: var(--fill-color-scriptanimation); - --effect-timing-graph-color: var(--stroke-color-scriptanimation); + --graph-color: var(--scriptanimation-color); + --graph-opacity: 0.5; +} + +.animation-item:hover { + background-color: var(--animation-item-hover-color); } .animation-item.selected { - background-color: var(--theme-selection-background-hover); + background-color: var(--animation-item-selected-color); } /* Animation Target */ @@ -191,7 +193,8 @@ select.playback-rate-selector.devtools-button:not(:empty):not(:disabled):not(.ch cursor: pointer; grid-column: 2 / 3; height: var(--graph-height); - padding-top: 5px; + padding-bottom: 3px; + padding-top: 3px; position: relative; } @@ -215,7 +218,8 @@ select.playback-rate-selector.devtools-button:not(:empty):not(:disabled):not(.ch } .animation-computed-timing-path path { - fill: var(--computed-timing-graph-color); + fill: var(--graph-color); + fill-opacity: var(--graph-opacity); vector-effect: non-scaling-stroke; transform: scale(1, -1); } @@ -226,7 +230,7 @@ select.playback-rate-selector.devtools-button:not(:empty):not(:disabled):not(.ch .animation-effect-timing-path path { fill: none; - stroke: var(--effect-timing-graph-color); + stroke: var(--graph-color); stroke-dasharray: 2px 2px; transform: scale(1, -1); vector-effect: non-scaling-stroke; @@ -250,7 +254,7 @@ select.playback-rate-selector.devtools-button:not(:empty):not(:disabled):not(.ch background-color: var(--theme-graphs-grey); height: 3px; position: absolute; - top: calc(100% - 1.5px); + bottom: 2px; } .animation-delay-sign::before, @@ -266,7 +270,7 @@ select.playback-rate-selector.devtools-button:not(:empty):not(:disabled):not(.ch .animation-delay-sign.fill, .animation-end-delay-sign.fill { - background-color: var(--effect-timing-graph-color); + background-color: var(--graph-color); } .animation-delay-sign.negative::before { @@ -305,7 +309,7 @@ select.playback-rate-selector.devtools-button:not(:empty):not(:disabled):not(.ch /* Animation Detail */ .animation-detail-container { - background-color: var(--theme-body-background); + background-color: var(--theme-sidebar-background); display: flex; flex-direction: column; height: 100%; @@ -365,10 +369,6 @@ select.playback-rate-selector.devtools-button:not(:empty):not(:disabled):not(.ch } /* Animated Property Item */ -.animated-property-item:nth-child(2n+1) { - background-color: var(--animation-even-background-color); -} - .animated-property-item.unchanged { opacity: 0.6; } @@ -388,15 +388,15 @@ select.playback-rate-selector.devtools-button:not(:empty):not(:disabled):not(.ch } .animated-property-list-container.cssanimation .animated-property-name.compositor { - --fast-track-color: var(--stroke-color-cssanimation); + --fast-track-color: var(--cssanimation-color); } .animated-property-list-container.csstransition .animated-property-name.compositor { - --fast-track-color: var(--stroke-color-csstransition); + --fast-track-color: var(--csstransition-color); } .animated-property-list-container.scriptanimation .animated-property-name.compositor { - --fast-track-color: var(--stroke-color-scriptanimation); + --fast-track-color: var(--scriptanimation-color); } .animated-property-name.compositor span::before { @@ -430,20 +430,21 @@ select.playback-rate-selector.devtools-button:not(:empty):not(:disabled):not(.ch } .keyframes-graph-path path { - fill: #00b0bd88; - stroke: #00b0bd; + fill: var(--teal-60); + fill-opacity: 0.5; + stroke: var(--teal-70); vector-effect: non-scaling-stroke; transform: scale(1, -1); } .keyframes-graph.opacity .keyframes-graph-path path { - fill: #df00a988; - stroke: #df00a9; + fill: var(--magenta-50); + stroke: var(--magenta-70); } .keyframes-graph.transform .keyframes-graph-path path { - fill: #ea800088; - stroke: #ea8000; + fill: var(--yellow-50); + stroke: var(--yellow-60); } .keyframes-graph-path .color-path path { @@ -496,15 +497,15 @@ select.playback-rate-selector.devtools-button:not(:empty):not(:disabled):not(.ch } .animated-property-list-container.cssanimation .keyframe-marker-item { - background-color: var(--fill-color-cssanimation); + background-color: var(--cssanimation-color); } .animated-property-list-container.csstransition .keyframe-marker-item { - background-color: var(--fill-color-csstransition); + background-color: var(--csstransition-color); } .animated-property-list-container.scriptanimation .keyframe-marker-item { - background-color: var(--fill-color-scriptanimation); + background-color: var(--scriptanimation-color); } /* Common Components */ diff --git a/devtools/client/themes/variables.css b/devtools/client/themes/variables.css index 09efd3bd9c48..0a8d621eceb1 100644 --- a/devtools/client/themes/variables.css +++ b/devtools/client/themes/variables.css @@ -213,8 +213,11 @@ /* Firefox Colors CSS Variables v1.0.3 * Colors are taken from: https://github.com/FirefoxUX/design-tokens */ + --magenta-50: #ff1ad9; --magenta-65: #dd00a9; + --magenta-70: #b5007f; + --purple-50: #9400ff; --purple-60: #8000d7; --blue-40: #45a1ff; @@ -223,20 +226,28 @@ --blue-60: #0060df; --blue-70: #003eaa; + --teal-60: #00c8d7; + --teal-70: #008ea4; + --red-70: #a4000f; --green-50: #30e60b; --green-60: #12bc00; --green-70: #058b00; + --yellow-50: #ffe900; + --yellow-60: #d7b600; --yellow-80: #715100; --grey-10: #f9f9fa; --grey-20: #ededf0; --grey-30: #d7d7db; + --grey-30-a40: rgba(215, 215, 219, 0.4); + --grey-30-a90: rgba(215, 215, 219, 0.9); --grey-40: #b1b1b3; --grey-50: #737373; --grey-60: #4a4a4f; + --grey-60-a50: rgba(74, 74, 79, 0.5); --grey-70: #38383d; --grey-80: #2a2a2e; --grey-90: #0c0c0d; diff --git a/dom/payments/PaymentRequest.cpp b/dom/payments/PaymentRequest.cpp index 36f82d9c0c0a..d86949688b74 100644 --- a/dom/payments/PaymentRequest.cpp +++ b/dom/payments/PaymentRequest.cpp @@ -649,11 +649,12 @@ PaymentRequest::CanMakePayment(ErrorResult& aRv) } if (mResultPromise) { + // XXX This doesn't match the spec but does match Chromium. aRv.Throw(NS_ERROR_DOM_NOT_ALLOWED_ERR); return nullptr; } - nsCOMPtr global = GetOwnerGlobal(); + nsIGlobalObject* global = GetOwnerGlobal(); ErrorResult result; RefPtr promise = Promise::Create(global, result); if (result.Failed()) { @@ -662,10 +663,7 @@ PaymentRequest::CanMakePayment(ErrorResult& aRv) } RefPtr manager = PaymentRequestManager::GetSingleton(); - if (NS_WARN_IF(!manager)) { - aRv.Throw(NS_ERROR_FAILURE); - return nullptr; - } + MOZ_ASSERT(manager); nsresult rv = manager->CanMakePayment(this); if (NS_WARN_IF(NS_FAILED(rv))) { promise->MaybeReject(NS_ERROR_FAILURE); @@ -687,27 +685,28 @@ already_AddRefed PaymentRequest::Show(const Optional>& aDetailsPromise, ErrorResult& aRv) { - if (mState != eCreated) { - aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR); - return nullptr; - } - if (!EventStateManager::IsHandlingUserInput()) { aRv.Throw(NS_ERROR_DOM_SECURITY_ERR); return nullptr; } - nsCOMPtr global = GetOwnerGlobal(); - ErrorResult result; - RefPtr promise = Promise::Create(global, result); - if (result.Failed()) { - mState = eClosed; - aRv.Throw(NS_ERROR_FAILURE); + nsIGlobalObject* global = GetOwnerGlobal(); + nsCOMPtr win = do_QueryInterface(global); + MOZ_ASSERT(win); + nsIDocument* doc = win->GetExtantDoc(); + if (!doc || !doc->IsCurrentActiveDocument()) { + aRv.Throw(NS_ERROR_DOM_ABORT_ERR); return nullptr; } - RefPtr manager = PaymentRequestManager::GetSingleton(); - if (NS_WARN_IF(!manager)) { + if (mState != eCreated) { + aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR); + return nullptr; + } + + ErrorResult result; + RefPtr promise = Promise::Create(global, result); + if (result.Failed()) { mState = eClosed; aRv.Throw(NS_ERROR_FAILURE); return nullptr; @@ -719,6 +718,8 @@ PaymentRequest::Show(const Optional>& aDetailsPromise, mDeferredShow = true; } + RefPtr manager = PaymentRequestManager::GetSingleton(); + MOZ_ASSERT(manager); nsresult rv = manager->ShowPayment(this); if (NS_WARN_IF(NS_FAILED(rv))) { if (rv == NS_ERROR_ABORT) { @@ -794,11 +795,11 @@ PaymentRequest::Abort(ErrorResult& aRv) } if (mAbortPromise) { - aRv.Throw(NS_ERROR_DOM_NOT_ALLOWED_ERR); + aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR); return nullptr; } - nsCOMPtr global = GetOwnerGlobal(); + nsIGlobalObject* global = GetOwnerGlobal(); ErrorResult result; RefPtr promise = Promise::Create(global, result); if (result.Failed()) { @@ -807,12 +808,8 @@ PaymentRequest::Abort(ErrorResult& aRv) } RefPtr manager = PaymentRequestManager::GetSingleton(); - if (NS_WARN_IF(!manager)) { - aRv.Throw(NS_ERROR_FAILURE); - return nullptr; - } - - // It's possible for to call this between show and its promise resolving. + MOZ_ASSERT(manager); + // It's possible to be called between show and its promise resolving. nsresult rv = manager->AbortPayment(this, mDeferredShow); mDeferredShow = false; if (NS_WARN_IF(NS_FAILED(rv))) { @@ -1060,7 +1057,9 @@ PaymentRequest::RejectedCallback(JSContext* aCx, JS::Handle aValue) PaymentRequest::~PaymentRequest() { if (mIPC) { - mIPC->MaybeDelete(); + // If we're being destroyed, the PaymentRequestManager isn't holding any + // references to us and we can't be waiting for any replies. + mIPC->MaybeDelete(false); } } diff --git a/dom/payments/PaymentRequestManager.cpp b/dom/payments/PaymentRequestManager.cpp index 55cb11e916a5..0bd26fbbc1a7 100644 --- a/dom/payments/PaymentRequestManager.cpp +++ b/dom/payments/PaymentRequestManager.cpp @@ -309,6 +309,17 @@ PaymentRequestManager::NotifyRequestDone(PaymentRequest* aRequest) } } +void +PaymentRequestManager::RequestIPCOver(PaymentRequest* aRequest) +{ + // This must only be called from ActorDestroy or if we're sure we won't + // receive any more IPC for aRequest. + mActivePayments.Remove(aRequest); + if (aRequest == mShowingRequest) { + mShowingRequest = nullptr; + } +} + already_AddRefed PaymentRequestManager::GetSingleton() { diff --git a/dom/payments/PaymentRequestManager.h b/dom/payments/PaymentRequestManager.h index 67250fdc2a43..4225f8a4d8ad 100644 --- a/dom/payments/PaymentRequestManager.h +++ b/dom/payments/PaymentRequestManager.h @@ -64,6 +64,10 @@ public: nsresult ChangeShippingOption(PaymentRequest* aRequest, const nsAString& aOption); + // Called to ensure that we don't "leak" aRequest if we shut down while it had + // an active request to the parent. + void RequestIPCOver(PaymentRequest* aRequest); + private: PaymentRequestManager() = default; ~PaymentRequestManager() diff --git a/dom/payments/ipc/PaymentRequestChild.cpp b/dom/payments/ipc/PaymentRequestChild.cpp index 34283e517318..c6bcd3421615 100644 --- a/dom/payments/ipc/PaymentRequestChild.cpp +++ b/dom/payments/ipc/PaymentRequestChild.cpp @@ -86,34 +86,31 @@ void PaymentRequestChild::ActorDestroy(ActorDestroyReason aWhy) { if (mRequest) { - DetachFromRequest(); + DetachFromRequest(true); } } void -PaymentRequestChild::MaybeDelete() +PaymentRequestChild::MaybeDelete(bool aCanBeInManager) { if (mRequest) { - DetachFromRequest(); + DetachFromRequest(aCanBeInManager); Send__delete__(this); } } -bool -PaymentRequestChild::SendRequestPayment(const IPCPaymentActionRequest& aAction) -{ - return PPaymentRequestChild::SendRequestPayment(aAction); -} - void -PaymentRequestChild::DetachFromRequest() +PaymentRequestChild::DetachFromRequest(bool aCanBeInManager) { MOZ_ASSERT(mRequest); - nsAutoString id; - mRequest->GetInternalId(id); - RefPtr manager = PaymentRequestManager::GetSingleton(); - MOZ_ASSERT(manager); + if (aCanBeInManager) { + RefPtr manager = PaymentRequestManager::GetSingleton(); + MOZ_ASSERT(manager); + + RefPtr request(mRequest); + manager->RequestIPCOver(request); + } mRequest->SetIPC(nullptr); mRequest = nullptr; diff --git a/dom/payments/ipc/PaymentRequestChild.h b/dom/payments/ipc/PaymentRequestChild.h index 9cea980ad8ec..b99e74bca070 100644 --- a/dom/payments/ipc/PaymentRequestChild.h +++ b/dom/payments/ipc/PaymentRequestChild.h @@ -19,7 +19,7 @@ class PaymentRequestChild final : public PPaymentRequestChild public: explicit PaymentRequestChild(PaymentRequest* aRequest); - void MaybeDelete(); + void MaybeDelete(bool aCanBeInManager); nsresult RequestPayment(const IPCPaymentActionRequest& aAction); @@ -40,8 +40,8 @@ protected: private: ~PaymentRequestChild() = default; - bool SendRequestPayment(const IPCPaymentActionRequest& aAction); - void DetachFromRequest(); + void DetachFromRequest(bool aCanBeInManager); + PaymentRequest* MOZ_NON_OWNING_REF mRequest; }; diff --git a/dom/vr/VRDisplay.cpp b/dom/vr/VRDisplay.cpp index d5e5a141b13e..579335055c50 100644 --- a/dom/vr/VRDisplay.cpp +++ b/dom/vr/VRDisplay.cpp @@ -263,7 +263,6 @@ VRPose::VRPose(nsISupports* aParent, const gfx::VRHMDSensorState& aState) : Pose(aParent) , mVRState(aState) { - mFrameId = aState.inputFrameID; mozilla::HoldJSObjects(this); } @@ -273,7 +272,6 @@ VRPose::VRPose(nsISupports* aParent) mVRState.inputFrameID = 0; mVRState.timestamp = 0.0; mVRState.flags = gfx::VRDisplayCapabilityFlags::Cap_None; - mFrameId = 0; mozilla::HoldJSObjects(this); } diff --git a/dom/vr/VRDisplay.h b/dom/vr/VRDisplay.h index 457c9a9153ae..156641a9c580 100644 --- a/dom/vr/VRDisplay.h +++ b/dom/vr/VRDisplay.h @@ -103,8 +103,6 @@ public: VRPose(nsISupports* aParent, const gfx::VRHMDSensorState& aState); explicit VRPose(nsISupports* aParent); - uint64_t FrameID() const { return mFrameId; } - virtual void GetPosition(JSContext* aCx, JS::MutableHandle aRetval, ErrorResult& aRv) override; @@ -129,7 +127,6 @@ public: protected: ~VRPose(); - uint64_t mFrameId; gfx::VRHMDSensorState mVRState; }; diff --git a/taskcluster/mach_commands.py b/taskcluster/mach_commands.py index 15e88fb9cd72..de6cefa208ab 100644 --- a/taskcluster/mach_commands.py +++ b/taskcluster/mach_commands.py @@ -272,7 +272,6 @@ class MachCommands(MachCommandBase): help='Action callback name (Python function name)') def test_action_callback(self, **options): import taskgraph.parameters - from taskgraph.util.taskcluster import get_task_definition import taskgraph.actions import yaml @@ -288,12 +287,6 @@ class MachCommands(MachCommandBase): try: self.setup_logging() task_id = options['task_id'] - if options['task']: - task = load_data(options['task']) - elif task_id: - task = get_task_definition(task_id) - else: - task = None if options['input']: input = load_data(options['input']) @@ -308,7 +301,6 @@ class MachCommands(MachCommandBase): return taskgraph.actions.trigger_action_callback( task_group_id=options['task_group_id'], task_id=task_id, - task=task, input=input, callback=options['callback'], parameters=parameters, diff --git a/taskcluster/taskgraph/actions/backfill.py b/taskcluster/taskgraph/actions/backfill.py index 39ce116f9203..9a9822534a65 100644 --- a/taskcluster/taskgraph/actions/backfill.py +++ b/taskcluster/taskgraph/actions/backfill.py @@ -44,6 +44,23 @@ logger = logging.getLogger(__name__) 'title': 'Depth', 'description': ('The number of previous pushes before the current ' 'push to attempt to trigger this task on.') + }, + 'inclusive': { + 'type': 'boolean', + 'default': False, + 'title': 'Inclusive Range', + 'description': ('If true, the backfill will also retrigger the task ' + 'on the selected push.') + }, + 'addGeckoProfile': { + 'type': 'boolean', + 'default': False, + 'title': 'Add Gecko Profile', + 'description': 'If true, appends --geckoProfile to mozharness options.' + }, + 'testPath': { + 'type': 'string', + 'title': 'Test Path', } }, 'additionalProperties': False @@ -53,8 +70,9 @@ logger = logging.getLogger(__name__) def backfill_action(parameters, graph_config, input, task_group_id, task_id, task): label = task['metadata']['name'] pushes = [] - depth = input.get('depth', 5) - end_id = int(parameters['pushlog_id']) - 1 + inclusive_tweak = 1 if input.get('inclusive') else 0 + depth = input.get('depth', 5) + inclusive_tweak + end_id = int(parameters['pushlog_id']) - (1 - inclusive_tweak) while True: start_id = max(end_id - depth, 0) @@ -90,8 +108,22 @@ def backfill_action(parameters, graph_config, input, task_group_id, task_id, tas continue if label in full_task_graph.tasks.keys(): - create_tasks( - [label], full_task_graph, label_to_taskid, - push_params, push_decision_task_id, push) + def modifier(task): + if task.label != label: + return task + if input.get('addGeckoProfile'): + mh = task.task['payload'].setdefault('env', {}) \ + .get('MOZHARNESS_OPTIONS', '') + task.task['payload']['env']['MOZHARNESS_OPTIONS'] = mh + ' --geckoProfile' + task.task['extra']['treeherder']['symbol'] += '-p' + + if input.get('testPath'): + env = task.task['payload'].setdefault('env', {}) + env['MOZHARNESS_TEST_PATHS'] = input.get('testPath') + task.task['extra']['treeherder']['symbol'] += '-b' + return task + + create_tasks([label], full_task_graph, label_to_taskid, + push_params, push_decision_task_id, push, modifier=modifier) else: logging.info('Could not find {} on {}. Skipping.'.format(label, push)) diff --git a/taskcluster/taskgraph/actions/util.py b/taskcluster/taskgraph/actions/util.py index 78af1667b041..b4391c619bb7 100644 --- a/taskcluster/taskgraph/actions/util.py +++ b/taskcluster/taskgraph/actions/util.py @@ -111,13 +111,18 @@ def update_parent(task, graph): def create_tasks(to_run, full_task_graph, label_to_taskid, - params, decision_task_id=None, suffix=''): + params, decision_task_id=None, suffix='', modifier=lambda t: t): """Create new tasks. The task definition will have {relative-datestamp': '..'} rendered just like in a decision task. Action callbacks should use this function to create new tasks, allowing easy debugging with `mach taskgraph action-callback --test`. This builds up all required tasks to run in order to run the tasks requested. + Optionally this function takes a `modifier` function that is passed in each + task before it is put into a new graph. It should return a valid task. Note + that this is passed _all_ tasks in the graph, not just the set in to_run. You + may want to skip modifying tasks not in your to_run list. + If you wish to create the tasks in a new group, leave out decision_task_id.""" if suffix != '': suffix = '-{}'.format(suffix) @@ -129,7 +134,7 @@ def create_tasks(to_run, full_task_graph, label_to_taskid, target_graph = full_task_graph.graph.transitive_closure(to_run) target_task_graph = TaskGraph( - {l: full_task_graph[l] for l in target_graph.nodes}, + {l: modifier(full_task_graph[l]) for l in target_graph.nodes}, target_graph) target_task_graph.for_each_task(update_parent) optimized_task_graph, label_to_taskid = optimize_task_graph(target_task_graph, diff --git a/taskcluster/taskgraph/task.py b/taskcluster/taskgraph/task.py index 1f348e54e0f9..ef27aa297571 100644 --- a/taskcluster/taskgraph/task.py +++ b/taskcluster/taskgraph/task.py @@ -21,7 +21,7 @@ class Task(object): - task_id -- TaskCluster taskId under which this task will be created - This class is just a convenience wraper for the data type and managing + This class is just a convenience wrapper for the data type and managing display, comparison, serialization, etc. It has no functionality of its own. """ def __init__(self, kind, label, attributes, task, diff --git a/testing/web-platform/meta/MANIFEST.json b/testing/web-platform/meta/MANIFEST.json index b996019c41fa..53da4285223a 100644 --- a/testing/web-platform/meta/MANIFEST.json +++ b/testing/web-platform/meta/MANIFEST.json @@ -597463,7 +597463,7 @@ "testharness" ], "payment-request/payment-request-canmakepayment-method-manual.https.html": [ - "7531ea3b11f6e8de8cf71666b19861dbc5267cf7", + "11df3310832e043e533a11245b012b9de1c0bb26", "manual" ], "payment-request/payment-request-constructor-crash.https.html": [ diff --git a/testing/web-platform/tests/payment-request/payment-request-canmakepayment-method-manual.https.html b/testing/web-platform/tests/payment-request/payment-request-canmakepayment-method-manual.https.html index 0311e84c942d..6729e9bc64f6 100644 --- a/testing/web-platform/tests/payment-request/payment-request-canmakepayment-method-manual.https.html +++ b/testing/web-platform/tests/payment-request/payment-request-canmakepayment-method-manual.https.html @@ -126,9 +126,12 @@ promise_test(async t => { function manualTest1(elem){ elem.disabled = true; + + // NB: request.show has to be called outside of promise_test to ensure the + // user's click is still visible to PaymentRequest.show. + const request = new PaymentRequest(defaultMethods, defaultDetails); + const acceptPromise = request.show(); // Sets state to "interactive" promise_test(async t => { - const request = new PaymentRequest(defaultMethods, defaultDetails); - const acceptPromise = request.show(); // Sets state to "interactive" const canMakePaymentPromise = request.canMakePayment(); try { const result = await canMakePaymentPromise; @@ -148,11 +151,14 @@ function manualTest1(elem){ } function manualTest2(elem){ - elem.disabled = true; + elem.disabled = true; + + // See above for why it's important for these lines to be outside of + // promise_test. + const request = new PaymentRequest(defaultMethods, defaultDetails); + const acceptPromise = request.show(); // The state is now "interactive" + acceptPromise.catch(() => {}); // no-op, just to silence unhandled rejection in devtools. promise_test(async t => { - const request = new PaymentRequest(defaultMethods, defaultDetails); - const acceptPromise = request.show(); // The state is now "interactive" - acceptPromise.catch(() => {}); // no-op, just to silence unhandled rejection in devtools. await request.abort(); // The state is now "closed" await promise_rejects(t, "InvalidStateError", request.canMakePayment()); try { diff --git a/toolkit/library/moz.build b/toolkit/library/moz.build index a5cbd3e0f467..6d597d879c8e 100644 --- a/toolkit/library/moz.build +++ b/toolkit/library/moz.build @@ -24,6 +24,8 @@ def Libxul(name): DELAYLOAD_DLLS += [ 'comdlg32.dll', + 'hid.dll', + 'msimg32.dll', 'netapi32.dll', 'secur32.dll', 'wininet.dll',