diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index 13d5fb8c19b9d..2c604e405bae7 100755 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -4302,6 +4302,8 @@ function updateEditUIVisibility() { let contextMenuPopupState = document.getElementById("contentAreaContextMenu").state; let placesContextMenuPopupState = document.getElementById("placesContext").state; + let oldVisible = gEditUIVisible; + // The UI is visible if the Edit menu is opening or open, if the context menu // is open, or if the toolbar has been customized to include the Cut, Copy, // or Paste toolbar buttons. @@ -4310,18 +4312,35 @@ function updateEditUIVisibility() { contextMenuPopupState == "showing" || contextMenuPopupState == "open" || placesContextMenuPopupState == "showing" || - placesContextMenuPopupState == "open" || - document.getElementById("edit-controls") ? true : false; + placesContextMenuPopupState == "open"; + if (!gEditUIVisible) { + // Now check the edit-controls toolbar buttons. + let placement = CustomizableUI.getPlacementOfWidget("edit-controls"); + let areaType = placement ? CustomizableUI.getAreaType(placement.area) : ""; + if (areaType == CustomizableUI.TYPE_MENU_PANEL) { + let panelUIMenuPopupState = document.getElementById("PanelUI-popup").state; + if (panelUIMenuPopupState == "showing" || panelUIMenuPopupState == "open") { + gEditUIVisible = true; + } + } else if (areaType == CustomizableUI.TYPE_TOOLBAR) { + // The edit controls are on a toolbar, so they are visible. + gEditUIVisible = true; + } + } + + // No need to update commands if the edit UI visibility has not changed. + if (gEditUIVisible == oldVisible) { + return; + } // If UI is visible, update the edit commands' enabled state to reflect // whether or not they are actually enabled for the current focus/selection. - if (gEditUIVisible) + if (gEditUIVisible) { goUpdateGlobalEditMenuItems(); - - // Otherwise, enable all commands, so that keyboard shortcuts still work, - // then lazily determine their actual enabled state when the user presses - // a keyboard shortcut. - else { + } else { + // Otherwise, enable all commands, so that keyboard shortcuts still work, + // then lazily determine their actual enabled state when the user presses + // a keyboard shortcut. goSetCommandEnabled("cmd_undo", true); goSetCommandEnabled("cmd_redo", true); goSetCommandEnabled("cmd_cut", true); diff --git a/browser/base/content/tabbrowser.xml b/browser/base/content/tabbrowser.xml index 9ec0c50338c79..2ce561cd08b03 100644 --- a/browser/base/content/tabbrowser.xml +++ b/browser/base/content/tabbrowser.xml @@ -1065,6 +1065,8 @@ return; if (!aForceUpdate) { + document.commandDispatcher.lock(); + TelemetryStopwatch.start("FX_TAB_SWITCH_UPDATE_MS"); if (!gMultiProcessBrowser) { // old way of measuring tab paint which is not valid with e10s. @@ -1274,6 +1276,8 @@ this.tabContainer._setPositionalAttributes(); if (!gMultiProcessBrowser) { + document.commandDispatcher.unlock(); + let event = new CustomEvent("TabSwitchDone", { bubbles: true, cancelable: true @@ -3981,6 +3985,8 @@ fromBrowser.removeAttribute("primary"); } + document.commandDispatcher.unlock(); + let event = new CustomEvent("TabSwitchDone", { bubbles: true, cancelable: true diff --git a/browser/base/content/test/tabs/browser.ini b/browser/base/content/test/tabs/browser.ini index 524cd8e3adeb9..ef575a3c033f4 100644 --- a/browser/base/content/test/tabs/browser.ini +++ b/browser/base/content/test/tabs/browser.ini @@ -16,4 +16,5 @@ skip-if = os == 'mac' skip-if = !e10s # Pref and test only relevant for e10s. [browser_opened_file_tab_navigated_to_web.js] [browser_reload_deleted_file.js] +[browser_tabswitch_updatecommands.js] [browser_viewsource_of_data_URI_in_file_process.js] diff --git a/browser/base/content/test/tabs/browser_tabswitch_updatecommands.js b/browser/base/content/test/tabs/browser_tabswitch_updatecommands.js new file mode 100644 index 0000000000000..726efc8b983ae --- /dev/null +++ b/browser/base/content/test/tabs/browser_tabswitch_updatecommands.js @@ -0,0 +1,21 @@ +// This test ensures that only one command update happens when switching tabs. + +"use strict"; + +add_task(function* () { + const uri = "data:text/html,"; + let tab1 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, uri); + let tab2 = yield BrowserTestUtils.openNewForegroundTab(gBrowser, uri); + + let updates = 0; + function countUpdates(event) { updates++; } + let updater = document.getElementById("editMenuCommandSetAll"); + updater.addEventListener("commandupdate", countUpdates, true); + yield BrowserTestUtils.switchTab(gBrowser, tab1); + + is(updates, 1, "only one command update per tab switch"); + + updater.removeEventListener("commandupdate", countUpdates, true); + yield BrowserTestUtils.removeTab(tab1); + yield BrowserTestUtils.removeTab(tab2); +}); diff --git a/browser/components/customizableui/content/panelUI.js b/browser/components/customizableui/content/panelUI.js index 67b869aee58f3..0f983b75a4932 100644 --- a/browser/components/customizableui/content/panelUI.js +++ b/browser/components/customizableui/content/panelUI.js @@ -160,11 +160,6 @@ const PanelUI = { return; } - let editControlPlacement = CustomizableUI.getPlacementOfWidget("edit-controls"); - if (editControlPlacement && editControlPlacement.area == CustomizableUI.AREA_PANEL) { - updateEditUIVisibility(); - } - let personalBookmarksPlacement = CustomizableUI.getPlacementOfWidget("personal-bookmarks"); if (personalBookmarksPlacement && personalBookmarksPlacement.area == CustomizableUI.AREA_PANEL) { @@ -292,10 +287,14 @@ const PanelUI = { switch (aEvent.type) { case "popupshowing": this._adjustLabelsForAutoHyphens(); + updateEditUIVisibility(); // Fall through case "popupshown": // Fall through case "popuphiding": + if (aEvent.type == "popuphiding") { + updateEditUIVisibility(); + } // Fall through case "popuphidden": this._updateNotifications(); diff --git a/browser/components/customizableui/test/browser.ini b/browser/components/customizableui/test/browser.ini index cbe7d2ac79459..cdeb5fbf4bff0 100644 --- a/browser/components/customizableui/test/browser.ini +++ b/browser/components/customizableui/test/browser.ini @@ -155,3 +155,5 @@ skip-if = os == "mac" [browser_switch_to_customize_mode.js] [browser_synced_tabs_menu.js] [browser_check_tooltips_in_navbar.js] +[browser_editcontrols_update.js] +subsuite = clipboard diff --git a/browser/components/customizableui/test/browser_editcontrols_update.js b/browser/components/customizableui/test/browser_editcontrols_update.js new file mode 100644 index 0000000000000..e98339b360d4e --- /dev/null +++ b/browser/components/customizableui/test/browser_editcontrols_update.js @@ -0,0 +1,162 @@ +// This test checks that the edit command enabled state (cut/paste) is updated +// properly when the edit controls are on the toolbar, popup and not present. +// It also verifies that the performance optimiation implemented by +// updateEditUIVisibility in browser.js is applied. + +let isMac = navigator.platform.indexOf("Mac") == 0; + +function checkState(allowCut, desc, testWindow = window) { + is(testWindow.document.getElementById("cmd_cut").getAttribute("disabled") == "true", !allowCut, desc + " - cut"); + is(testWindow.document.getElementById("cmd_paste").getAttribute("disabled") == "true", false, desc + " - paste"); +} + +// Add a special controller to the urlbar and browser to listen in on when +// commands are being updated. Return a promise that resolves when 'count' +// updates have occurred. +function expectCommandUpdate(count, testWindow = window) { + return new Promise((resolve, reject) => { + let overrideController = { + supportsCommand(cmd) { return cmd == "cmd_delete"; }, + isCommandEnabled(cmd) { + if (!count) { + ok(false, "unexpected update"); + reject(); + } + + if (!--count) { + testWindow.gURLBar.controllers.removeControllerAt(0, overrideController); + testWindow.gBrowser.selectedBrowser.controllers.removeControllerAt(0, overrideController); + resolve(true); + } + } + }; + + if (!count) { + SimpleTest.executeSoon(() => { + testWindow.gURLBar.controllers.removeControllerAt(0, overrideController); + testWindow.gBrowser.selectedBrowser.controllers.removeControllerAt(0, overrideController); + resolve(false); + }); + } + + testWindow.gURLBar.controllers.insertControllerAt(0, overrideController); + testWindow.gBrowser.selectedBrowser.controllers.insertControllerAt(0, overrideController); + }); +} + +add_task(function* test_init() { + // Put something on the clipboard to verify that the paste button is properly enabled during the test. + let clipboardHelper = Cc["@mozilla.org/widget/clipboardhelper;1"].getService(Ci.nsIClipboardHelper); + yield new Promise(resolve => { + SimpleTest.waitForClipboard("Sample", function() { clipboardHelper.copyString("Sample"); }, resolve); + }); + + // Open and close the panel first so that it is fully initialized. + yield PanelUI.show(); + let hiddenPromise = promisePanelHidden(window); + PanelUI.hide(); + yield hiddenPromise; +}); + +// Test updating when the panel is open with the edit-controls on the panel. +// Updates should occur. +add_task(function* test_panelui_opened() { + gURLBar.focus(); + gURLBar.value = "test"; + + yield PanelUI.show(); + + checkState(false, "Update when edit-controls is on panel and visible"); + + let overridePromise = expectCommandUpdate(1); + gURLBar.select(); + yield overridePromise; + + checkState(true, "Update when edit-controls is on panel and selection changed"); + + overridePromise = expectCommandUpdate(0); + let hiddenPromise = promisePanelHidden(window); + PanelUI.hide(); + yield hiddenPromise; + yield overridePromise; + + // Check that updates do not occur after the panel has been closed. + checkState(true, "Update when edit-controls is on panel and hidden"); + + // Mac will update the enabled st1ate even when the panel is closed so that + // main menubar shortcuts will work properly. + overridePromise = expectCommandUpdate(isMac ? 1 : 0); + gURLBar.select(); + yield overridePromise; + checkState(true, "Update when edit-controls is on panel, hidden and selection changed"); +}); + +// Test updating when the edit-controls are moved to the toolbar. +add_task(function* test_panelui_customize_to_toolbar() { + yield startCustomizing(); + let navbar = document.getElementById("nav-bar").customizationTarget; + simulateItemDrag(document.getElementById("edit-controls"), navbar); + yield endCustomizing(); + + // updateEditUIVisibility should be called when customization ends but isn't. See bug 1359790. + updateEditUIVisibility(); + + let overridePromise = expectCommandUpdate(1); + gURLBar.select(); + gURLBar.focus(); + gURLBar.value = "other"; + yield overridePromise; + checkState(false, "Update when edit-controls on toolbar and focused"); + + overridePromise = expectCommandUpdate(1); + gURLBar.select(); + yield overridePromise; + checkState(true, "Update when edit-controls on toolbar and selection changed"); +}); + +// Test updating when the edit-controls are moved to the palette. +add_task(function* test_panelui_customize_to_palette() { + yield startCustomizing(); + let palette = document.getElementById("customization-palette"); + simulateItemDrag(document.getElementById("edit-controls"), palette); + yield endCustomizing(); + + // updateEditUIVisibility should be called when customization ends but isn't. See bug 1359790. + updateEditUIVisibility(); + + let overridePromise = expectCommandUpdate(isMac ? 1 : 0); + gURLBar.focus(); + gURLBar.value = "other"; + gURLBar.select(); + yield overridePromise; + + // If the UI isn't found, the command is set to be enabled. + checkState(true, "Update when edit-controls is on palette, hidden and selection changed"); +}); + +add_task(function* finish() { + yield resetCustomization(); +}); + +// Test updating in the initial state when the edit-controls are on the panel but +// have not yet been created. This needs to be done in a new window to ensure that +// other tests haven't opened the panel. +add_task(function* test_initial_state() { + let testWindow = yield BrowserTestUtils.openNewBrowserWindow(); + yield SimpleTest.promiseFocus(testWindow); + + let overridePromise = expectCommandUpdate(isMac, testWindow); + + testWindow.gURLBar.focus(); + testWindow.gURLBar.value = "test"; + + yield overridePromise; + + // Commands won't update when no edit UI is present. They default to being + // enabled so that keyboard shortcuts will work. The real enabled state will + // be checked when shortcut is pressed. + checkState(!isMac, "No update when edit-controls is on panel and not visible", testWindow); + + yield BrowserTestUtils.closeWindow(testWindow); + yield SimpleTest.promiseFocus(window); +}); diff --git a/browser/config/mozconfigs/linux32/devedition b/browser/config/mozconfigs/linux32/devedition index 196012d640187..a43fe9a0c4cad 100644 --- a/browser/config/mozconfigs/linux32/devedition +++ b/browser/config/mozconfigs/linux32/devedition @@ -12,5 +12,7 @@ STRIP_FLAGS="--strip-debug" ac_add_options --with-branding=browser/branding/aurora +mk_add_options MOZ_PGO=1 + . "$topsrcdir/build/mozconfig.common.override" . "$topsrcdir/build/mozconfig.cache" diff --git a/browser/config/mozconfigs/linux64/devedition b/browser/config/mozconfigs/linux64/devedition index 71379590275fb..7a69a65c619d5 100644 --- a/browser/config/mozconfigs/linux64/devedition +++ b/browser/config/mozconfigs/linux64/devedition @@ -12,5 +12,7 @@ STRIP_FLAGS="--strip-debug" ac_add_options --with-branding=browser/branding/aurora +mk_add_options MOZ_PGO=1 + . "$topsrcdir/build/mozconfig.common.override" . "$topsrcdir/build/mozconfig.cache" diff --git a/browser/config/mozconfigs/win32/devedition b/browser/config/mozconfigs/win32/devedition index 181dd67972c65..66176f5db7956 100644 --- a/browser/config/mozconfigs/win32/devedition +++ b/browser/config/mozconfigs/win32/devedition @@ -8,5 +8,7 @@ ac_add_options --enable-verify-mar ac_add_options --with-branding=browser/branding/aurora +mk_add_options MOZ_PGO=1 + . "$topsrcdir/build/mozconfig.common.override" . "$topsrcdir/build/mozconfig.cache" diff --git a/browser/config/mozconfigs/win64/devedition b/browser/config/mozconfigs/win64/devedition index 9dff9cca7b8b0..8b3b774fbf444 100644 --- a/browser/config/mozconfigs/win64/devedition +++ b/browser/config/mozconfigs/win64/devedition @@ -9,5 +9,7 @@ ac_add_options --enable-verify-mar ac_add_options --with-branding=browser/branding/aurora +mk_add_options MOZ_PGO=1 + . "$topsrcdir/build/mozconfig.common.override" . "$topsrcdir/build/mozconfig.cache" diff --git a/devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js b/devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js index 464dfb9175ba3..4f812ee005465 100644 --- a/devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js +++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-breakpoints-cond.js @@ -13,6 +13,8 @@ function setConditionalBreakpoint(dbg, index, condition) { selectMenuItem(dbg, 2); yield waitForElement(dbg, ".conditional-breakpoint-panel input"); findElementWithSelector(dbg, ".conditional-breakpoint-panel input").focus(); + // Position cursor reliably at the end of the text. + pressKey(dbg, "End") type(dbg, condition); pressKey(dbg, "Enter"); }); diff --git a/devtools/client/debugger/new/test/mochitest/browser_dbg-expressions.js b/devtools/client/debugger/new/test/mochitest/browser_dbg-expressions.js index 152fd47f2190f..86feaf516a517 100644 --- a/devtools/client/debugger/new/test/mochitest/browser_dbg-expressions.js +++ b/devtools/client/debugger/new/test/mochitest/browser_dbg-expressions.js @@ -32,6 +32,8 @@ async function addExpression(dbg, input) { async function editExpression(dbg, input) { info("updating the expression"); dblClickElement(dbg, "expressionNode", 1); + // Position cursor reliably at the end of the text. + pressKey(dbg, "End") type(dbg, input); pressKey(dbg, "Enter"); await waitForDispatch(dbg, "EVALUATE_EXPRESSION"); diff --git a/devtools/client/debugger/new/test/mochitest/head.js b/devtools/client/debugger/new/test/mochitest/head.js index 2efe4b43eefb9..7721fab322d95 100644 --- a/devtools/client/debugger/new/test/mochitest/head.js +++ b/devtools/client/debugger/new/test/mochitest/head.js @@ -560,6 +560,10 @@ const keyMappings = { Enter: { code: "VK_RETURN" }, Up: { code: "VK_UP" }, Down: { code: "VK_DOWN" }, + Right: { code: "VK_RIGHT" }, + Left: { code: "VK_LEFT" }, + End: { code: "VK_RIGHT", modifiers: cmdOrCtrl }, + Start: { code: "VK_LEFT", modifiers: cmdOrCtrl }, Tab: { code: "VK_TAB" }, Escape: { code: "VK_ESCAPE" }, pauseKey: { code: "VK_F8" }, diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 69d675ff80ac4..0aa5fc47ac5f6 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -297,6 +297,7 @@ bool nsContentUtils::sUseActivityCursor = false; bool nsContentUtils::sAnimationsAPICoreEnabled = false; bool nsContentUtils::sAnimationsAPIElementAnimateEnabled = false; bool nsContentUtils::sGetBoxQuadsEnabled = false; +bool nsContentUtils::sSkipCursorMoveForSameValueSet = false; int32_t nsContentUtils::sPrivacyMaxInnerWidth = 1000; int32_t nsContentUtils::sPrivacyMaxInnerHeight = 1000; @@ -645,6 +646,10 @@ nsContentUtils::Init() Preferences::AddBoolVarCache(&sGetBoxQuadsEnabled, "layout.css.getBoxQuads.enabled", false); + Preferences::AddBoolVarCache(&sSkipCursorMoveForSameValueSet, + "dom.input.skip_cursor_move_for_same_value_set", + true); + Element::InitCCCallbacks(); nsCOMPtr uuidGenerator = diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index 4c793645f3b23..df705faac70d3 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -2888,6 +2888,13 @@ class nsContentUtils */ static uint64_t GenerateTabId(); + /** + * Check whether we should skip moving the cursor for a same-value .value set + * on a text input or textarea. + */ + static bool + SkipCursorMoveForSameValueSet() { return sSkipCursorMoveForSameValueSet; } + private: static bool InitializeEventTable(); @@ -3012,6 +3019,7 @@ class nsContentUtils static bool sAnimationsAPICoreEnabled; static bool sAnimationsAPIElementAnimateEnabled; static bool sGetBoxQuadsEnabled; + static bool sSkipCursorMoveForSameValueSet; static uint32_t sCookiesLifetimePolicy; static uint32_t sCookiesBehavior; diff --git a/dom/bindings/parser/WebIDL.py b/dom/bindings/parser/WebIDL.py index 2c6f7be3f5704..c5e482aec29fd 100644 --- a/dom/bindings/parser/WebIDL.py +++ b/dom/bindings/parser/WebIDL.py @@ -573,7 +573,10 @@ def isConsequential(self): return False def addExtendedAttributes(self, attrs): - assert len(attrs) == 0 + if len(attrs) != 0: + raise WebIDLError("There are no extended attributes that are " + "allowed on external interfaces", + [attrs[0].location, self.location]) def resolve(self, parentScope): pass @@ -1932,7 +1935,10 @@ def dictionaryContainsDictionary(dictMember, dictionary): [member.location] + locations) def addExtendedAttributes(self, attrs): - assert len(attrs) == 0 + if len(attrs) != 0: + raise WebIDLError("There are no extended attributes that are " + "allowed on dictionaries", + [attrs[0].location, self.location]) def _getDependentObjects(self): deps = set(self.members) @@ -1966,7 +1972,10 @@ def isEnum(self): return True def addExtendedAttributes(self, attrs): - assert len(attrs) == 0 + if len(attrs) != 0: + raise WebIDLError("There are no extended attributes that are " + "allowed on enums", + [attrs[0].location, self.location]) def _getDependentObjects(self): return set() @@ -2139,7 +2148,11 @@ def treatNonObjectAsNull(self): return self.nullable() and self.inner.callback._treatNonObjectAsNull def addExtendedAttributes(self, attrs): - assert len(attrs) == 0 + if len(attrs) != 0: + raise WebIDLError("There are no extended attributes that are " + "allowed on types, for now (but this is " + "changing; see bug 1359269)", + [attrs[0].location, self.location]) def resolveType(self, parentScope): pass @@ -2707,7 +2720,10 @@ def isTypedef(self): return True def addExtendedAttributes(self, attrs): - assert len(attrs) == 0 + if len(attrs) != 0: + raise WebIDLError("There are no extended attributes that are " + "allowed on typedefs", + [attrs[0].location, self.location]) def _getDependentObjects(self): return self.innerType._getDependentObjects() @@ -5108,7 +5124,10 @@ def validate(self): pass def addExtendedAttributes(self, attrs): - assert len(attrs) == 0 + if len(attrs) != 0: + raise WebIDLError("There are no extended attributes that are " + "allowed on implements statements", + [attrs[0].location, self.location]) class IDLExtendedAttribute(IDLObject): diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index cb0b5b04db4f7..92ea4ed292f61 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -1974,9 +1974,10 @@ HTMLInputElement::SetValue(const nsAString& aValue, CallerType aCallerType, GetValue(currentValue, aCallerType); nsresult rv = - SetValueInternal(aValue, nsTextEditorState::eSetValue_ByContent | - nsTextEditorState::eSetValue_Notify | - nsTextEditorState::eSetValue_MoveCursorToEnd); + SetValueInternal(aValue, + nsTextEditorState::eSetValue_ByContent | + nsTextEditorState::eSetValue_Notify | + nsTextEditorState::eSetValue_MoveCursorToEndIfValueChanged); if (NS_FAILED(rv)) { aRv.Throw(rv); return; @@ -1987,9 +1988,10 @@ HTMLInputElement::SetValue(const nsAString& aValue, CallerType aCallerType, } } else { nsresult rv = - SetValueInternal(aValue, nsTextEditorState::eSetValue_ByContent | - nsTextEditorState::eSetValue_Notify | - nsTextEditorState::eSetValue_MoveCursorToEnd); + SetValueInternal(aValue, + nsTextEditorState::eSetValue_ByContent | + nsTextEditorState::eSetValue_Notify | + nsTextEditorState::eSetValue_MoveCursorToEndIfValueChanged); if (NS_FAILED(rv)) { aRv.Throw(rv); return; @@ -2843,9 +2845,9 @@ HTMLInputElement::SetUserInput(const nsAString& aValue) } else { nsresult rv = SetValueInternal(aValue, - nsTextEditorState::eSetValue_BySetUserInput | - nsTextEditorState::eSetValue_Notify| - nsTextEditorState::eSetValue_MoveCursorToEnd); + nsTextEditorState::eSetValue_BySetUserInput | + nsTextEditorState::eSetValue_Notify| + nsTextEditorState::eSetValue_MoveCursorToEndIfValueChanged); NS_ENSURE_SUCCESS(rv, rv); } diff --git a/dom/html/HTMLTextAreaElement.cpp b/dom/html/HTMLTextAreaElement.cpp index 98a7891654c75..a4c95b9ad88da 100644 --- a/dom/html/HTMLTextAreaElement.cpp +++ b/dom/html/HTMLTextAreaElement.cpp @@ -404,9 +404,10 @@ HTMLTextAreaElement::SetValue(const nsAString& aValue) GetValueInternal(currentValue, true); nsresult rv = - SetValueInternal(aValue, nsTextEditorState::eSetValue_ByContent | - nsTextEditorState::eSetValue_Notify | - nsTextEditorState::eSetValue_MoveCursorToEnd); + SetValueInternal(aValue, + nsTextEditorState::eSetValue_ByContent | + nsTextEditorState::eSetValue_Notify | + nsTextEditorState::eSetValue_MoveCursorToEndIfValueChanged); NS_ENSURE_SUCCESS(rv, rv); if (mFocusedValue.Equals(currentValue)) { @@ -420,9 +421,9 @@ NS_IMETHODIMP HTMLTextAreaElement::SetUserInput(const nsAString& aValue) { return SetValueInternal(aValue, - nsTextEditorState::eSetValue_BySetUserInput | - nsTextEditorState::eSetValue_Notify| - nsTextEditorState::eSetValue_MoveCursorToEnd); + nsTextEditorState::eSetValue_BySetUserInput | + nsTextEditorState::eSetValue_Notify| + nsTextEditorState::eSetValue_MoveCursorToEndIfValueChanged); } NS_IMETHODIMP diff --git a/dom/html/nsTextEditorState.cpp b/dom/html/nsTextEditorState.cpp index d08e9e2e26bdb..a81773c1a9b93 100644 --- a/dom/html/nsTextEditorState.cpp +++ b/dom/html/nsTextEditorState.cpp @@ -2503,6 +2503,12 @@ nsTextEditorState::SetValue(const nsAString& aValue, uint32_t aFlags) } } + // \r is an illegal character in the dom, but people use them, + // so convert windows and mac platform linebreaks to \n: + if (!nsContentUtils::PlatformToDOMLineBreaks(newValue, fallible)) { + return false; + } + if (mEditor && mBoundFrame) { // The InsertText call below might flush pending notifications, which // could lead into a scheduled PrepareEditor to be called. That will @@ -2528,14 +2534,6 @@ nsTextEditorState::SetValue(const nsAString& aValue, uint32_t aFlags) { ValueSetter valueSetter(mEditor); - // \r is an illegal character in the dom, but people use them, - // so convert windows and mac platform linebreaks to \n: - if (newValue.FindChar(char16_t('\r')) != -1) { - if (!nsContentUtils::PlatformToDOMLineBreaks(newValue, fallible)) { - return false; - } - } - nsCOMPtr domDoc; mEditor->GetDocument(getter_AddRefs(domDoc)); if (!domDoc) { @@ -2638,33 +2636,42 @@ nsTextEditorState::SetValue(const nsAString& aValue, uint32_t aFlags) if (!mValue) { mValue.emplace(); } - nsString value; - if (!value.Assign(newValue, fallible)) { - return false; - } - if (!nsContentUtils::PlatformToDOMLineBreaks(value, fallible)) { - return false; - } - if (!mValue->Assign(value, fallible)) { - return false; - } - // Since we have no editor we presumably have cached selection state. - if (IsSelectionCached()) { - SelectionProperties& props = GetSelectionProperties(); - if (aFlags & eSetValue_MoveCursorToEnd) { - props.SetStart(value.Length()); - props.SetEnd(value.Length()); - } else { - // Make sure our cached selection position is not outside the new value. - props.SetStart(std::min(props.GetStart(), value.Length())); - props.SetEnd(std::min(props.GetEnd(), value.Length())); + // We can't just early-return here if mValue->Equals(newValue), because + // ValueWasChanged and OnValueChanged below still need to be called. + if (!mValue->Equals(newValue) || + !nsContentUtils::SkipCursorMoveForSameValueSet()) { + if (!mValue->Assign(newValue, fallible)) { + return false; + } + + // Since we have no editor we presumably have cached selection state. + if (IsSelectionCached()) { + SelectionProperties& props = GetSelectionProperties(); + if (aFlags & eSetValue_MoveCursorToEndIfValueChanged) { + props.SetStart(newValue.Length()); + props.SetEnd(newValue.Length()); + props.SetDirection(nsITextControlFrame::eForward); + } else { + // Make sure our cached selection position is not outside the new value. + props.SetStart(std::min(props.GetStart(), newValue.Length())); + props.SetEnd(std::min(props.GetEnd(), newValue.Length())); + } } - } - // Update the frame display if needed - if (mBoundFrame) { - mBoundFrame->UpdateValueDisplay(true); + // Update the frame display if needed + if (mBoundFrame) { + mBoundFrame->UpdateValueDisplay(true); + } + } else { + // Even if our value is not actually changing, apparently we need to mark + // our SelectionProperties dirty to make accessibility tests happy. + // Probably because they depend on the SetSelectionRange() call we make on + // our frame in RestoreSelectionState, but I have no idea why they do. + if (IsSelectionCached()) { + SelectionProperties& props = GetSelectionProperties(); + props.SetIsDirty(); + } } } diff --git a/dom/html/nsTextEditorState.h b/dom/html/nsTextEditorState.h index 1e143afe21e7a..c98f4cda59b4d 100644 --- a/dom/html/nsTextEditorState.h +++ b/dom/html/nsTextEditorState.h @@ -173,9 +173,11 @@ class nsTextEditorState : public mozilla::SupportsWeakPtr { // Whether the value change should be notified to the frame/contet nor not. eSetValue_Notify = 1 << 2, // Whether to move the cursor to end of the value (in the case when we have - // cached selection offsets). If this is not set, the cached selection - // offsets will simply be clamped to be within the length of the new value. - eSetValue_MoveCursorToEnd = 1 << 3, + // cached selection offsets), in the case when the value has changed. If + // this is not set, the cached selection offsets will simply be clamped to + // be within the length of the new value. In either case, if the value has + // not changed the cursor won't move. + eSetValue_MoveCursorToEndIfValueChanged = 1 << 3, }; MOZ_MUST_USE bool SetValue(const nsAString& aValue, uint32_t aFlags); void GetValue(nsAString& aValue, bool aIgnoreWrap) const; @@ -280,11 +282,16 @@ class nsTextEditorState : public mozilla::SupportsWeakPtr { mIsDirty = true; mDirection = value; } - // return true only if mStart, mEnd, or mDirection have been modified + // return true only if mStart, mEnd, or mDirection have been modified, + // or if SetIsDirty() was explicitly called. bool IsDirty() const { return mIsDirty; } + void SetIsDirty() + { + mIsDirty = true; + } private: uint32_t mStart, mEnd; bool mIsDirty = false; diff --git a/dom/interfaces/xul/nsIDOMXULCommandDispatcher.idl b/dom/interfaces/xul/nsIDOMXULCommandDispatcher.idl index 995feaf22a502..e53bdc13ef593 100644 --- a/dom/interfaces/xul/nsIDOMXULCommandDispatcher.idl +++ b/dom/interfaces/xul/nsIDOMXULCommandDispatcher.idl @@ -31,4 +31,9 @@ interface nsIDOMXULCommandDispatcher : nsISupports void rewindFocus(); void advanceFocusIntoSubtree(in nsIDOMElement elt); attribute boolean suppressFocusScroll; + + // When locked, command updating is batched until unlocked. Always ensure that + // lock and unlock is called in a pair. + void lock(); + void unlock(); }; diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index da60b75193805..f247938df1006 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -5055,7 +5055,7 @@ ContentParent::ForceTabPaint(TabParent* aTabParent, uint64_t aLayerObserverEpoch } nsresult -ContentParent::AboutToLoadDocumentForChild(nsIChannel* aChannel) +ContentParent::AboutToLoadHttpFtpWyciwygDocumentForChild(nsIChannel* aChannel) { MOZ_ASSERT(aChannel); diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index fdd1c49942ba3..167bbab2e48e7 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -656,7 +656,7 @@ class ContentParent final : public PContentParent // HTTP(S), FTP or wyciwyg channel for a content process. It is a useful // place to start to kick off work as early as possible in response to such // document loads. - nsresult AboutToLoadDocumentForChild(nsIChannel* aChannel); + nsresult AboutToLoadHttpFtpWyciwygDocumentForChild(nsIChannel* aChannel); nsresult TransmitPermissionsForPrincipal(nsIPrincipal* aPrincipal); diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index 955b1ca013550..2e9ff5f76f2bf 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -941,6 +941,10 @@ TabParent::RecvPDocAccessibleConstructor(PDocAccessibleParent* aDoc, #ifdef XP_WIN a11y::WrapperFor(doc)->SetID(aMsaaID); MOZ_ASSERT(!aDocCOMProxy.IsNull()); + if (aDocCOMProxy.IsNull()) { + return IPC_FAIL(this, "Constructing a top-level PDocAccessible with null COM proxy"); + } + RefPtr proxy(aDocCOMProxy.Get()); doc->SetCOMInterface(proxy); doc->MaybeInitWindowEmulation(); diff --git a/dom/xul/nsXULCommandDispatcher.cpp b/dom/xul/nsXULCommandDispatcher.cpp index 2d222c24011b7..8345c69579f4e 100644 --- a/dom/xul/nsXULCommandDispatcher.cpp +++ b/dom/xul/nsXULCommandDispatcher.cpp @@ -42,7 +42,7 @@ static LazyLogModule gCommandLog("nsXULCommandDispatcher"); //////////////////////////////////////////////////////////////////////// nsXULCommandDispatcher::nsXULCommandDispatcher(nsIDocument* aDocument) - : mDocument(aDocument), mUpdaters(nullptr) + : mDocument(aDocument), mUpdaters(nullptr), mLocked(false) { } @@ -350,6 +350,14 @@ nsXULCommandDispatcher::RemoveCommandUpdater(nsIDOMElement* aElement) NS_IMETHODIMP nsXULCommandDispatcher::UpdateCommands(const nsAString& aEventName) { + if (mLocked) { + if (!mPendingUpdates.Contains(aEventName)) { + mPendingUpdates.AppendElement(aEventName); + } + + return NS_OK; + } + nsAutoString id; nsCOMPtr element; GetFocusedElement(getter_AddRefs(element)); @@ -457,3 +465,30 @@ nsXULCommandDispatcher::SetSuppressFocusScroll(bool aSuppressFocusScroll) return NS_OK; } +NS_IMETHODIMP +nsXULCommandDispatcher::Lock() +{ + // Since locking is used only as a performance optimization, we don't worry + // about nested lock calls. If that does happen, it just means we will unlock + // and process updates earlier. + mLocked = true; + return NS_OK; +} + +NS_IMETHODIMP +nsXULCommandDispatcher::Unlock() +{ + if (mLocked) { + mLocked = false; + + // Handle any pending updates one at a time. In the unlikely case where a + // lock is added during the update, break out. + while (!mLocked && mPendingUpdates.Length() > 0) { + nsString name = mPendingUpdates.ElementAt(0); + mPendingUpdates.RemoveElementAt(0); + UpdateCommands(name); + } + } + + return NS_OK; +} diff --git a/dom/xul/nsXULCommandDispatcher.h b/dom/xul/nsXULCommandDispatcher.h index bb33edc8e0b88..e0ce5ede27a4e 100644 --- a/dom/xul/nsXULCommandDispatcher.h +++ b/dom/xul/nsXULCommandDispatcher.h @@ -68,6 +68,9 @@ class nsXULCommandDispatcher : public nsIDOMXULCommandDispatcher, bool Matches(const nsString& aList, const nsAString& aElement); + + bool mLocked; + nsTArray mPendingUpdates; }; #endif // nsXULCommandDispatcher_h__ diff --git a/gfx/skia/skia/src/core/SkBitmapProcState.cpp b/gfx/skia/skia/src/core/SkBitmapProcState.cpp index 5bc1b47f6e498..183016e69595d 100644 --- a/gfx/skia/skia/src/core/SkBitmapProcState.cpp +++ b/gfx/skia/skia/src/core/SkBitmapProcState.cpp @@ -300,7 +300,7 @@ bool SkBitmapProcState::chooseScanlineProcs(bool trivialMatrix, bool clampClamp) return false; } -#if !defined(SK_ARM_HAS_NEON) +#if !defined(SK_ARM_HAS_NEON) || defined(SK_ARM_HAS_OPTIONAL_NEON) static const SampleProc32 gSkBitmapProcStateSample32[] = { S32_opaque_D32_nofilter_DXDY, S32_alpha_D32_nofilter_DXDY, diff --git a/gfx/skia/skia/src/core/SkBitmapProcState_matrixProcs.cpp b/gfx/skia/skia/src/core/SkBitmapProcState_matrixProcs.cpp index 970ea47a3c444..50b59b8bdfcd7 100644 --- a/gfx/skia/skia/src/core/SkBitmapProcState_matrixProcs.cpp +++ b/gfx/skia/skia/src/core/SkBitmapProcState_matrixProcs.cpp @@ -56,7 +56,7 @@ extern const SkBitmapProcState::MatrixProc RepeatX_RepeatY_Procs_neon[]; #endif // defined(SK_ARM_HAS_NEON) // Compile non-neon code path if needed -#if !defined(SK_ARM_HAS_NEON) +#if !defined(SK_ARM_HAS_NEON) || defined(SK_ARM_HAS_OPTIONAL_NEON) #define MAKENAME(suffix) ClampX_ClampY ## suffix #define TILEX_PROCF(fx, max) SkClampMax((fx) >> 16, max) #define TILEY_PROCF(fy, max) SkClampMax((fy) >> 16, max) diff --git a/gfx/thebes/gfxFT2FontList.cpp b/gfx/thebes/gfxFT2FontList.cpp index 44a8498ca074e..8a3c3d22db602 100644 --- a/gfx/thebes/gfxFT2FontList.cpp +++ b/gfx/thebes/gfxFT2FontList.cpp @@ -1217,8 +1217,8 @@ gfxFT2FontList::FindFonts() FinalizeFamilyMemberList(key, family, /* aSortFaces */ false ); } - LOG(("got font list from chrome process: %d faces in %d families " - "and %d in hidden families", + LOG(("got font list from chrome process: %" PRIdPTR " faces in %" + PRIu32 " families and %" PRIu32 " in hidden families", fonts.Length(), mFontFamilies.Count(), mHiddenFontFamilies.Count())); return; diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index a24a5e17eeafd..ac63acb09ee41 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -1552,13 +1552,19 @@ class MOZ_STACK_CLASS TryEmitter // stream and fixed-up later. // // If ShouldUseControl is DontUseControl, all that handling is skipped. - // DontUseControl is used by yield*, that matches to the following: - // * has only one catch block - // * has no catch guard - // * has JSOP_GOTO at the end of catch-block - // * has no non-local-jump - // * doesn't use finally block for normal completion of try-block and + // DontUseControl is used by yield* and the internal try-catch around + // IteratorClose. These internal uses must: + // * have only one catch block + // * have no catch guard + // * have JSOP_GOTO at the end of catch-block + // * have no non-local-jump + // * don't use finally block for normal completion of try-block and // catch-block + // + // Additionally, a finally block may be emitted when ShouldUseControl is + // DontUseControl, even if the kind is not TryCatchFinally or TryFinally, + // because GOSUBs are not emitted. This internal use shares the + // requirements as above. Maybe controlInfo_; int depth_; @@ -1726,7 +1732,17 @@ class MOZ_STACK_CLASS TryEmitter public: bool emitFinally(const Maybe& finallyPos = Nothing()) { - MOZ_ASSERT(hasFinally()); + // If we are using controlInfo_ (i.e., emitting a syntactic try + // blocks), we must have specified up front if there will be a finally + // close. For internal try blocks, like those emitted for yield* and + // IteratorClose inside for-of loops, we can emitFinally even without + // specifying up front, since the internal try blocks emit no GOSUBs. + if (!controlInfo_) { + if (kind_ == TryCatch) + kind_ = TryCatchFinally; + } else { + MOZ_ASSERT(hasFinally()); + } if (state_ == Try) { if (!emitTryEnd()) @@ -2022,6 +2038,10 @@ class ForOfLoopControl : public LoopControl // } Maybe tryCatch_; + // Used to track if any yields were emitted between calls to to + // emitBeginCodeNeedingIteratorClose and emitEndCodeNeedingIteratorClose. + uint32_t numYieldsAtBeginCodeNeedingIterClose_; + bool allowSelfHosted_; IteratorKind iterKind_; @@ -2031,16 +2051,22 @@ class ForOfLoopControl : public LoopControl IteratorKind iterKind) : LoopControl(bce, StatementKind::ForOfLoop), iterDepth_(iterDepth), + numYieldsAtBeginCodeNeedingIterClose_(UINT32_MAX), allowSelfHosted_(allowSelfHosted), iterKind_(iterKind) { } bool emitBeginCodeNeedingIteratorClose(BytecodeEmitter* bce) { - tryCatch_.emplace(bce, TryEmitter::TryCatch, TryEmitter::DontUseRetVal); + tryCatch_.emplace(bce, TryEmitter::TryCatch, TryEmitter::DontUseRetVal, + TryEmitter::DontUseControl); if (!tryCatch_->emitTry()) return false; + + MOZ_ASSERT(numYieldsAtBeginCodeNeedingIterClose_ == UINT32_MAX); + numYieldsAtBeginCodeNeedingIterClose_ = bce->yieldAndAwaitOffsetList.numYields; + return true; } @@ -2078,10 +2104,33 @@ class ForOfLoopControl : public LoopControl if (!bce->emit1(JSOP_THROW)) // ITER ... return false; + // If any yields were emitted, then this for-of loop is inside a star + // generator and must handle the case of Generator.return. Like in + // yield*, it is handled with a finally block. + uint32_t numYieldsEmitted = bce->yieldAndAwaitOffsetList.numYields; + if (numYieldsEmitted > numYieldsAtBeginCodeNeedingIterClose_) { + if (!tryCatch_->emitFinally()) + return false; + + IfThenElseEmitter ifGeneratorClosing(bce); + if (!bce->emit1(JSOP_ISGENCLOSING)) // ITER ... FTYPE FVALUE CLOSING + return false; + if (!ifGeneratorClosing.emitIf()) // ITER ... FTYPE FVALUE + return false; + if (!bce->emitDupAt(slotFromTop + 1)) // ITER ... FTYPE FVALUE ITER + return false; + if (!emitIteratorClose(bce, CompletionKind::Normal)) // ITER ... FTYPE FVALUE + return false; + if (!ifGeneratorClosing.emitEnd()) // ITER ... FTYPE FVALUE + return false; + } + if (!tryCatch_->emitEnd()) return false; tryCatch_.reset(); + numYieldsAtBeginCodeNeedingIterClose_ = UINT32_MAX; + return true; } @@ -4829,6 +4878,11 @@ BytecodeEmitter::emitYieldOp(JSOp op) return false; } + if (op == JSOP_AWAIT) + yieldAndAwaitOffsetList.numAwaits++; + else + yieldAndAwaitOffsetList.numYields++; + SET_UINT24(code(off), yieldAndAwaitIndex); if (!yieldAndAwaitOffsetList.append(offset())) diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index e57f997679763..fcceccf59d1e6 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -101,7 +101,9 @@ struct CGScopeNoteList { struct CGYieldAndAwaitOffsetList { Vector list; - explicit CGYieldAndAwaitOffsetList(JSContext* cx) : list(cx) {} + uint32_t numYields; + uint32_t numAwaits; + explicit CGYieldAndAwaitOffsetList(JSContext* cx) : list(cx), numYields(0), numAwaits(0) {} MOZ_MUST_USE bool append(uint32_t offset) { return list.append(offset); } size_t length() const { return list.length(); } diff --git a/js/src/jit/CacheIRCompiler.cpp b/js/src/jit/CacheIRCompiler.cpp index 6ada8ff0b2638..825f936a91766 100644 --- a/js/src/jit/CacheIRCompiler.cpp +++ b/js/src/jit/CacheIRCompiler.cpp @@ -1277,12 +1277,12 @@ CacheIRCompiler::emitGuardIsInt32Index() return true; } + ValueOperand input = allocator.useValueRegister(masm, inputId); + FailurePath* failure; if (!addFailurePath(&failure)) return false; - ValueOperand input = allocator.useValueRegister(masm, inputId); - Label notInt32, done; masm.branchTestInt32(Assembler::NotEqual, input, ¬Int32); masm.unboxInt32(input, output); diff --git a/js/src/jsapi-tests/testGCAllocator.cpp b/js/src/jsapi-tests/testGCAllocator.cpp index 229e56422fdf7..ec5407e8cca1e 100644 --- a/js/src/jsapi-tests/testGCAllocator.cpp +++ b/js/src/jsapi-tests/testGCAllocator.cpp @@ -315,7 +315,7 @@ mapMemoryAt(void* desired, size_t length) #if defined(__ia64__) || defined(__aarch64__) || \ (defined(__sparc__) && defined(__arch64__) && (defined(__NetBSD__) || defined(__linux__))) - MOZ_RELEASE_ASSERT(0xffff800000000000ULL & (uintptr_t(desired) + length - 1) == 0); + MOZ_RELEASE_ASSERT((0xffff800000000000ULL & (uintptr_t(desired) + length - 1)) == 0); #endif void* region = mmap(desired, length, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); if (region == MAP_FAILED) diff --git a/js/src/jsmath.cpp b/js/src/jsmath.cpp index 570674c14a265..4c8df7995b60c 100644 --- a/js/src/jsmath.cpp +++ b/js/src/jsmath.cpp @@ -66,6 +66,8 @@ # define GETRANDOM_NR 318 # elif defined(__i386__) # define GETRANDOM_NR 355 +# elif defined(__aarch64__) +# define GETRANDOM_NR 278 # elif defined(__arm__) # define GETRANDOM_NR 384 // Added other architectures: diff --git a/js/src/jsnativestack.cpp b/js/src/jsnativestack.cpp index 05928ea3dff3a..40b6802317261 100644 --- a/js/src/jsnativestack.cpp +++ b/js/src/jsnativestack.cpp @@ -16,7 +16,7 @@ # include # endif -# if defined(ANDROID) +# if defined(ANDROID) && !defined(__aarch64__) # include # include # endif @@ -120,11 +120,11 @@ js::GetNativeStackBaseImpl() rc = pthread_stackseg_np(pthread_self(), &ss); stackBase = (void*)((size_t) ss.ss_sp - ss.ss_size); stackSize = ss.ss_size; -# elif defined(ANDROID) +# elif defined(ANDROID) && !defined(__aarch64__) if (gettid() == getpid()) { - // bionic's pthread_attr_getstack doesn't tell the truth for the main - // thread (see bug 846670). So we scan /proc/self/maps to find the - // segment which contains the stack. + // bionic's pthread_attr_getstack prior to API 21 doesn't tell the truth + // for the main thread (see bug 846670). So we scan /proc/self/maps to + // find the segment which contains the stack. rc = -1; // Put the string on the stack, otherwise there is the danger that it diff --git a/js/src/tests/ecma_6/Generators/yield-iterator-close.js b/js/src/tests/ecma_6/Generators/yield-iterator-close.js new file mode 100644 index 0000000000000..970ad494d2b77 --- /dev/null +++ b/js/src/tests/ecma_6/Generators/yield-iterator-close.js @@ -0,0 +1,58 @@ +// Test that IteratorClose is called when a Generator is abruptly completed by +// Generator.return. + +var returnCalled = 0; +function* wrapNoThrow() { + let iter = { + [Symbol.iterator]() { + return this; + }, + next() { + return { value: 10, done: false }; + }, + return() { + returnCalled++; + return {}; + } + }; + for (const i of iter) { + yield i; + } +} + +// Breaking calls Generator.return, which causes the yield above to resume with +// an abrupt completion of kind "return", which then calls +// iter.return. +for (const i of wrapNoThrow()) { + break; +} +assertEq(returnCalled, 1); + +function* wrapThrow() { + let iter = { + [Symbol.iterator]() { + return this; + }, + next() { + return { value: 10, done: false }; + }, + return() { + throw 42; + } + }; + for (const i of iter) { + yield i; + } +} + +// Breaking calls Generator.return, which, like above, calls iter.return. If +// iter.return throws, since the yield is resuming with an abrupt completion of +// kind "return", the newly thrown value takes precedence over returning. +assertThrowsValue(() => { + for (const i of wrapThrow()) { + break; + } +}, 42); + +if (typeof reportCompare === "function") + reportCompare(0, 0); diff --git a/js/src/vm/TraceLogging.cpp b/js/src/vm/TraceLogging.cpp index c7bc6a84b2e99..82fdf7c77e8ac 100644 --- a/js/src/vm/TraceLogging.cpp +++ b/js/src/vm/TraceLogging.cpp @@ -61,7 +61,7 @@ rdtsc(void) return result; } -#elif defined(__arm__) +#elif defined(__arm__) || defined(__aarch64__) #include diff --git a/js/src/wasm/WasmSignalHandlers.cpp b/js/src/wasm/WasmSignalHandlers.cpp index 3f65bbb44f993..1ab76eeb4bd99 100644 --- a/js/src/wasm/WasmSignalHandlers.cpp +++ b/js/src/wasm/WasmSignalHandlers.cpp @@ -1374,6 +1374,7 @@ ProcessHasSignalHandlers() sTriedInstallSignalHandlers = true; #if defined(ANDROID) +# if !defined(__aarch64__) // Before Android 4.4 (SDK version 19), there is a bug // https://android-review.googlesource.com/#/c/52333 // in Bionic's pthread_join which causes pthread_join to return early when @@ -1385,6 +1386,7 @@ ProcessHasSignalHandlers() if (atol(version_string) < 19) return false; } +# endif # if defined(MOZ_LINKER) // Signal handling is broken on some android systems. if (IsSignalHandlingBroken()) diff --git a/media/omx-plugin/OmxPlugin.cpp b/media/omx-plugin/OmxPlugin.cpp index 4b6641c46e47b..602f46e5e9241 100644 --- a/media/omx-plugin/OmxPlugin.cpp +++ b/media/omx-plugin/OmxPlugin.cpp @@ -503,7 +503,7 @@ bool OmxDecoder::Init() int64_t durationUs; if (videoTrack->getFormat()->findInt64(kKeyDuration, &durationUs)) { if (durationUs < 0) - LOG("video duration %lld should be nonnegative", durationUs); + LOG("video duration %" PRId64 " should be nonnegative", durationUs); if (durationUs > totalDurationUs) totalDurationUs = durationUs; } @@ -536,7 +536,7 @@ bool OmxDecoder::Init() int64_t durationUs; if (audioTrack->getFormat()->findInt64(kKeyDuration, &durationUs)) { if (durationUs < 0) - LOG("audio duration %lld should be nonnegative", durationUs); + LOG("audio duration %" PRId64 " should be nonnegative", durationUs); if (durationUs > totalDurationUs) totalDurationUs = durationUs; } @@ -913,7 +913,7 @@ bool OmxDecoder::ReadVideo(VideoFrame *aFrame, int64_t aSeekTimeUs, } if (timeUs < 0) { - LOG("frame time %lld must be nonnegative", timeUs); + LOG("frame time %" PRId64 " must be nonnegative", timeUs); return false; } @@ -977,7 +977,7 @@ bool OmxDecoder::ReadAudio(AudioFrame *aFrame, int64_t aSeekTimeUs) } if (timeUs < 0) { - LOG("frame time %lld must be nonnegative", timeUs); + LOG("frame time %" PRId64 " must be nonnegative", timeUs); return false; } diff --git a/media/webrtc/signaling/src/media-conduit/WebrtcMediaCodecVP8VideoCodec.cpp b/media/webrtc/signaling/src/media-conduit/WebrtcMediaCodecVP8VideoCodec.cpp index 1be585d0a76a9..96a37248e2ef2 100644 --- a/media/webrtc/signaling/src/media-conduit/WebrtcMediaCodecVP8VideoCodec.cpp +++ b/media/webrtc/signaling/src/media-conduit/WebrtcMediaCodecVP8VideoCodec.cpp @@ -1155,10 +1155,10 @@ int32_t WebrtcMediaCodecVP8VideoDecoder::Decode( const webrtc::CodecSpecificInfo* codecSpecificInfo, int64_t renderTimeMs) { - CSFLogDebug(logTag, "%s, renderTimeMs = %lld ", __FUNCTION__, renderTimeMs); + CSFLogDebug(logTag, "%s, renderTimeMs = %" PRId64, __FUNCTION__, renderTimeMs); if (inputImage._length== 0 || !inputImage._buffer) { - CSFLogDebug(logTag, "%s, input Image invalid. length = %d", __FUNCTION__, inputImage._length); + CSFLogDebug(logTag, "%s, input Image invalid. length = %" PRIdPTR, __FUNCTION__, inputImage._length); return WEBRTC_VIDEO_CODEC_ERROR; } diff --git a/mfbt/IntegerPrintfMacros.h b/mfbt/IntegerPrintfMacros.h index 7db0ab3392ba8..b0c9153923fba 100644 --- a/mfbt/IntegerPrintfMacros.h +++ b/mfbt/IntegerPrintfMacros.h @@ -49,4 +49,36 @@ # define PRIXPTR "X" /* uintptr_t */ #endif +/* + * Fix up Android's broken macros for [u]int_fastN_t. On ARM64, Android's + * PRI*FAST16/32 macros are defined as "d", but the types themselves are defined + * as long and unsigned long. + */ +#if defined(ANDROID) && defined(__LP64__) +# undef PRIdFAST16 /* int_fast16_t */ +# define PRIdFAST16 PRId64 /* int_fast16_t */ +# undef PRIiFAST16 /* int_fast16_t */ +# define PRIiFAST16 PRIi64 /* int_fast16_t */ +# undef PRIoFAST16 /* uint_fast16_t */ +# define PRIoFAST16 PRIo64 /* uint_fast16_t */ +# undef PRIuFAST16 /* uint_fast16_t */ +# define PRIuFAST16 PRIu64 /* uint_fast16_t */ +# undef PRIxFAST16 /* uint_fast16_t */ +# define PRIxFAST16 PRIx64 /* uint_fast16_t */ +# undef PRIXFAST16 /* uint_fast16_t */ +# define PRIXFAST16 PRIX64 /* uint_fast16_t */ +# undef PRIdFAST32 /* int_fast32_t */ +# define PRIdFAST32 PRId64 /* int_fast32_t */ +# undef PRIiFAST32 /* int_fast32_t */ +# define PRIiFAST32 PRIi64 /* int_fast32_t */ +# undef PRIoFAST32 /* uint_fast32_t */ +# define PRIoFAST32 PRIo64 /* uint_fast32_t */ +# undef PRIuFAST32 /* uint_fast32_t */ +# define PRIuFAST32 PRIu64 /* uint_fast32_t */ +# undef PRIxFAST32 /* uint_fast32_t */ +# define PRIxFAST32 PRIx64 /* uint_fast32_t */ +# undef PRIXFAST32 /* uint_fast32_t */ +# define PRIXFAST32 PRIX64 /* uint_fast32_t */ +#endif + #endif /* mozilla_IntegerPrintfMacros_h_ */ diff --git a/mobile/android/app/mobile.js b/mobile/android/app/mobile.js index 6d0066dd0b8ce..2f0d8728a9705 100644 --- a/mobile/android/app/mobile.js +++ b/mobile/android/app/mobile.js @@ -921,3 +921,7 @@ pref("webchannel.allowObject.urlWhitelist", "https://accounts.firefox.com https: pref("media.openUnsupportedTypeWithExternalApp", true); pref("dom.keyboardevent.dispatch_during_composition", true); + +#if CPU_ARCH == aarch64 +pref("javascript.options.native_regexp", false); +#endif diff --git a/mobile/android/app/moz.build b/mobile/android/app/moz.build index 77f28cb58a597..1b027078a04df 100644 --- a/mobile/android/app/moz.build +++ b/mobile/android/app/moz.build @@ -34,7 +34,7 @@ with Files('src/test/**'): for var in ('APP_NAME', 'APP_VERSION'): DEFINES[var] = CONFIG['MOZ_%s' % var] -for var in ('MOZ_UPDATER', 'MOZ_APP_UA_NAME', 'ANDROID_PACKAGE_NAME'): +for var in ('MOZ_UPDATER', 'MOZ_APP_UA_NAME', 'ANDROID_PACKAGE_NAME', 'CPU_ARCH'): DEFINES[var] = CONFIG[var] for var in ('MOZ_ANDROID_GCM', ): diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index c6659cce23012..30ca916325ed3 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1251,6 +1251,10 @@ pref("dom.select_popup_in_parent.enabled", false); // Enable Directory API. By default, disabled. pref("dom.input.dirpicker", false); +// Enable not moving the cursor to end when a text input or textarea has .value +// set to the value it already has. By default, enabled. +pref("dom.input.skip_cursor_move_for_same_value_set", false); + // Enables system messages and activities pref("dom.sysmsg.enabled", false); diff --git a/mozglue/misc/StackWalk.cpp b/mozglue/misc/StackWalk.cpp index c06ffb85adb71..768d531fb74e9 100644 --- a/mozglue/misc/StackWalk.cpp +++ b/mozglue/misc/StackWalk.cpp @@ -248,6 +248,17 @@ PrintError(const char* aPrefix) LocalFree(lpMsgBuf); } +static void +InitializeDbgHelpCriticalSection() +{ + static bool initialized = false; + if (initialized) { + return; + } + ::InitializeCriticalSection(&gDbgHelpCS); + initialized = true; +} + static unsigned int WINAPI WalkStackThread(void* aData); static bool @@ -300,8 +311,7 @@ EnsureWalkThreadReady() stackWalkThread = nullptr; readyEvent = nullptr; - - ::InitializeCriticalSection(&gDbgHelpCS); + InitializeDbgHelpCriticalSection(); return walkThreadReady = true; } @@ -851,9 +861,7 @@ EnsureSymInitialized() return gInitialized; } - if (!EnsureWalkThreadReady()) { - return false; - } + InitializeDbgHelpCriticalSection(); SymSetOptions(SYMOPT_LOAD_LINES | SYMOPT_UNDNAME); retStat = SymInitialize(GetCurrentProcess(), nullptr, TRUE); diff --git a/netwerk/protocol/ftp/FTPChannelParent.cpp b/netwerk/protocol/ftp/FTPChannelParent.cpp index 6f0eb5382a71d..cf3577bb15019 100644 --- a/netwerk/protocol/ftp/FTPChannelParent.cpp +++ b/netwerk/protocol/ftp/FTPChannelParent.cpp @@ -462,7 +462,7 @@ FTPChannelParent::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext) // performing a document load. PContentParent* pcp = Manager()->Manager(); DebugOnly rv = - static_cast(pcp)->AboutToLoadDocumentForChild(chan); + static_cast(pcp)->AboutToLoadHttpFtpWyciwygDocumentForChild(chan); MOZ_ASSERT(NS_SUCCEEDED(rv)); int64_t contentLength; diff --git a/netwerk/protocol/http/HttpChannelParent.cpp b/netwerk/protocol/http/HttpChannelParent.cpp index be73d38388c6a..a4297e7123f49 100644 --- a/netwerk/protocol/http/HttpChannelParent.cpp +++ b/netwerk/protocol/http/HttpChannelParent.cpp @@ -1148,7 +1148,7 @@ HttpChannelParent::OnStartRequest(nsIRequest *aRequest, nsISupports *aContext) PContentParent* pcp = Manager()->Manager(); MOZ_ASSERT(pcp, "We should have a manager if our IPC isn't closed"); DebugOnly rv = - static_cast(pcp)->AboutToLoadDocumentForChild(chan); + static_cast(pcp)->AboutToLoadHttpFtpWyciwygDocumentForChild(chan); MOZ_ASSERT(NS_SUCCEEDED(rv)); } diff --git a/netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp b/netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp index 7edf39e79b306..2d6b3084b3448 100644 --- a/netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp +++ b/netwerk/protocol/wyciwyg/WyciwygChannelParent.cpp @@ -326,7 +326,7 @@ WyciwygChannelParent::OnStartRequest(nsIRequest *aRequest, nsISupports *aContext // Send down any permissions which are relevant to this URL if we are // performing a document load. PContentParent* pcp = Manager()->Manager(); - rv = static_cast(pcp)->AboutToLoadDocumentForChild(chan); + rv = static_cast(pcp)->AboutToLoadHttpFtpWyciwygDocumentForChild(chan); MOZ_ASSERT(NS_SUCCEEDED(rv)); nsresult status; diff --git a/taskcluster/ci/build/linux.yml b/taskcluster/ci/build/linux.yml index b8a7bcef43a3a..94f01913b0a69 100644 --- a/taskcluster/ci/build/linux.yml +++ b/taskcluster/ci/build/linux.yml @@ -77,7 +77,7 @@ linux64-devedition/opt: job-name: linux64-opt treeherder: platform: linux64-devedition/opt - symbol: tc(DE) + symbol: tc(B) worker-type: aws-provisioner-v1/gecko-{level}-b-linux worker: implementation: docker-worker @@ -176,7 +176,7 @@ linux-devedition/opt: job-name: linux-opt treeherder: platform: linux32-devedition/opt - symbol: tc(DE) + symbol: tc(B) worker-type: aws-provisioner-v1/gecko-{level}-b-linux worker: implementation: docker-worker diff --git a/taskcluster/ci/build/macosx.yml b/taskcluster/ci/build/macosx.yml index c2a597919d68e..a45a0b8ab858b 100644 --- a/taskcluster/ci/build/macosx.yml +++ b/taskcluster/ci/build/macosx.yml @@ -6,7 +6,7 @@ macosx64/debug: treeherder: platform: osx-10-7/debug symbol: tc(B) - tier: 2 + tier: 1 worker-type: aws-provisioner-v1/gecko-{level}-b-macosx64 worker: implementation: docker-worker diff --git a/taskcluster/taskgraph/target_tasks.py b/taskcluster/taskgraph/target_tasks.py index ea9ab9f12f7fa..351ef4636ec09 100644 --- a/taskcluster/taskgraph/target_tasks.py +++ b/taskcluster/taskgraph/target_tasks.py @@ -238,9 +238,9 @@ def filter(task): platform = task.attributes.get('build_platform') if platform in ('linux64-pgo', 'linux-pgo', 'win32-pgo', 'win64-pgo', 'android-api-15-nightly', 'android-x86-nightly', - 'win32', 'win64', 'macosx64'): + 'win32', 'win64'): return False - if platform in ('linux64', 'linux'): + if platform in ('linux64', 'linux', 'macosx64'): if task.attributes['build_type'] == 'opt': return False # skip l10n, beetmover, balrog diff --git a/testing/mozharness/configs/builds/releng_sub_mac_configs/64_devedition.py b/testing/mozharness/configs/builds/releng_sub_mac_configs/64_devedition.py index 76890d168b919..b9ebbab124d83 100644 --- a/testing/mozharness/configs/builds/releng_sub_mac_configs/64_devedition.py +++ b/testing/mozharness/configs/builds/releng_sub_mac_configs/64_devedition.py @@ -1,4 +1,6 @@ config = { + 'src_mozconfig': 'browser/config/mozconfigs/macosx64/devedition', 'force_clobber': True, 'stage_platform': 'macosx64-devedition', + 'stage_product': 'devedition', } diff --git a/testing/mozharness/configs/builds/releng_sub_windows_configs/32_devedition.py b/testing/mozharness/configs/builds/releng_sub_windows_configs/32_devedition.py index 86644a4d1259c..22bbfcef20ace 100644 --- a/testing/mozharness/configs/builds/releng_sub_windows_configs/32_devedition.py +++ b/testing/mozharness/configs/builds/releng_sub_windows_configs/32_devedition.py @@ -1,4 +1,6 @@ config = { + 'src_mozconfig': 'browser/config/mozconfigs/win32/devedition', 'force_clobber': True, 'stage_platform': 'win32-devedition', + 'stage_product': 'devedition', } diff --git a/testing/mozharness/configs/builds/releng_sub_windows_configs/64_devedition.py b/testing/mozharness/configs/builds/releng_sub_windows_configs/64_devedition.py index ca7b1354c2469..7fa3f0e50ddbb 100644 --- a/testing/mozharness/configs/builds/releng_sub_windows_configs/64_devedition.py +++ b/testing/mozharness/configs/builds/releng_sub_windows_configs/64_devedition.py @@ -1,4 +1,6 @@ config = { + 'src_mozconfig': 'browser/config/mozconfigs/win64/devedition', 'force_clobber': True, 'stage_platform': 'win64-devedition', + 'stage_product': 'devedition', } diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index d134ad190619d..7a3f09156d0ad 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -2044,9 +2044,8 @@ CreateJSHangHistogram(JSContext* cx, const Telemetry::HangHistogram& hang) } if (!hang.GetNativeStack().empty()) { - const Telemetry::HangStack& stack = hang.GetNativeStack(); - const std::vector& frames = stack.GetNativeFrames(); - Telemetry::ProcessedStack processed = Telemetry::GetStackAndModules(frames); + const Telemetry::NativeHangStack& stack = hang.GetNativeStack(); + Telemetry::ProcessedStack processed = Telemetry::GetStackAndModules(stack); CombinedStacks singleStack; singleStack.AddStack(processed); diff --git a/toolkit/components/telemetry/ThreadHangStats.h b/toolkit/components/telemetry/ThreadHangStats.h index ad8154a83f710..c9a65d5bac1e5 100644 --- a/toolkit/components/telemetry/ThreadHangStats.h +++ b/toolkit/components/telemetry/ThreadHangStats.h @@ -45,13 +45,19 @@ class TimeHistogram : public mozilla::Array void Add(PRIntervalTime aTime); }; +/* A native stack is a simple list of pointers, so rather than building a + wrapper type, we typdef the type here. */ +typedef std::vector NativeHangStack; + /* HangStack stores an array of const char pointers, with optional internal storage for strings. */ class HangStack { public: static const size_t sMaxInlineStorage = 8; + // The maximum depth for the native stack frames that we might collect. + // XXX: Consider moving this to a different object? static const size_t sMaxNativeFrames = 25; private: @@ -61,11 +67,6 @@ class HangStack // Stack entries can either be a static const char* // or a pointer to within this buffer. mozilla::Vector mBuffer; - // When a native stack is gathered, this vector holds the raw program counter - // values that MozStackWalk will return to us after it walks the stack. - // When gathering the Telemetry payload, Telemetry will take care of mapping - // these program counters to proper addresses within modules. - std::vector mNativeFrames; public: HangStack() {} @@ -76,6 +77,12 @@ class HangStack { } + HangStack& operator=(HangStack&& aOther) { + mImpl = mozilla::Move(aOther.mImpl); + mBuffer = mozilla::Move(aOther.mBuffer); + return *this; + } + bool operator==(const HangStack& aOther) const { for (size_t i = 0; i < length(); i++) { if (!IsSameAsEntry(operator[](i), aOther[i])) { @@ -118,7 +125,6 @@ class HangStack void clear() { mImpl.clear(); mBuffer.clear(); - mNativeFrames.clear(); } bool IsInBuffer(const char* aEntry) const { @@ -144,21 +150,6 @@ class HangStack const char* InfallibleAppendViaBuffer(const char* aText, size_t aLength); const char* AppendViaBuffer(const char* aText, size_t aLength); - - void EnsureNativeFrameCapacity(size_t aCapacity) { - mNativeFrames.reserve(aCapacity); - } - - void AppendNativeFrame(uintptr_t aPc) { - MOZ_ASSERT(mNativeFrames.size() <= sMaxNativeFrames); - if (mNativeFrames.size() < sMaxNativeFrames) { - mNativeFrames.push_back(aPc); - } - } - - const std::vector& GetNativeFrames() const { - return mNativeFrames; - } }; /* A hang histogram consists of a stack associated with the @@ -170,7 +161,7 @@ class HangHistogram : public TimeHistogram HangStack mStack; // Native stack that corresponds to the pseudostack in mStack - HangStack mNativeStack; + NativeHangStack mNativeStack; // Use a hash to speed comparisons const uint32_t mHash; // Annotations attributed to this stack @@ -182,6 +173,15 @@ class HangHistogram : public TimeHistogram , mHash(GetHash(mStack)) { } + + explicit HangHistogram(HangStack&& aStack, + NativeHangStack&& aNativeStack) + : mStack(mozilla::Move(aStack)) + , mNativeStack(mozilla::Move(aNativeStack)) + , mHash(GetHash(mStack)) + { + } + HangHistogram(HangHistogram&& aOther) : TimeHistogram(mozilla::Move(aOther)) , mStack(mozilla::Move(aOther.mStack)) @@ -198,10 +198,10 @@ class HangHistogram : public TimeHistogram const HangStack& GetStack() const { return mStack; } - HangStack& GetNativeStack() { + NativeHangStack& GetNativeStack() { return mNativeStack; } - const HangStack& GetNativeStack() const { + const NativeHangStack& GetNativeStack() const { return mNativeStack; } const HangMonitor::HangAnnotationsVector& GetAnnotations() const { @@ -231,16 +231,20 @@ class ThreadHangStats public: TimeHistogram mActivity; mozilla::Vector mHangs; + uint32_t mNativeStackCnt; explicit ThreadHangStats(const char* aName) : mName(aName) + , mNativeStackCnt(0) { } ThreadHangStats(ThreadHangStats&& aOther) : mName(mozilla::Move(aOther.mName)) , mActivity(mozilla::Move(aOther.mActivity)) , mHangs(mozilla::Move(aOther.mHangs)) + , mNativeStackCnt(aOther.mNativeStackCnt) { + aOther.mNativeStackCnt = 0; } const char* GetName() const { return mName.get(); diff --git a/toolkit/components/telemetry/tests/unit/test_ThreadHangStats.js b/toolkit/components/telemetry/tests/unit/test_ThreadHangStats.js index bbf38107df260..04885441246b6 100644 --- a/toolkit/components/telemetry/tests/unit/test_ThreadHangStats.js +++ b/toolkit/components/telemetry/tests/unit/test_ThreadHangStats.js @@ -84,8 +84,8 @@ function run_test() { notEqual(endHangs.hangs[0].stack.length, 0); equal(typeof endHangs.hangs[0].stack[0], "string"); - // Native stack gathering is only enabled on Windows. - if (mozinfo.os == "win") { + // Native stack gathering is only enabled on Windows x86. + if (mozinfo.os == "win" && mozinfo.bits == 32) { // Make sure one of the hangs is a permanent // hang containing a native stack. ok(endHangs.hangs.some((hang) => ( diff --git a/toolkit/xre/nsUpdateDriver.cpp b/toolkit/xre/nsUpdateDriver.cpp index 20305bd43cc6e..2dc3af1fcf6ef 100644 --- a/toolkit/xre/nsUpdateDriver.cpp +++ b/toolkit/xre/nsUpdateDriver.cpp @@ -701,9 +701,17 @@ ApplyUpdate(nsIFile *greDir, nsIFile *updateDir, nsIFile *appDir, int appArgc, } #elif defined(XP_MACOSX) UpdateDriverSetupMacCommandLine(argc, argv, restart); - // LaunchChildMac uses posix_spawnp and prefers the current - // architecture when launching. It doesn't require a - // null-terminated string but it doesn't matter if we pass one. + // We need to detect whether elevation is required for this update. This can + // occur when an admin user installs the application, but another admin + // user attempts to update (see bug 394984). + if (restart && !IsRecursivelyWritable(installDirPath.get())) { + if (!LaunchElevatedUpdate(argc, argv, outpid)) { + LOG(("Failed to launch elevated update!")); + exit(1); + } + exit(0); + } + if (isStaged) { // Launch the updater to replace the installation with the staged updated. LaunchChildMac(argc, argv); @@ -711,6 +719,9 @@ ApplyUpdate(nsIFile *greDir, nsIFile *updateDir, nsIFile *appDir, int appArgc, // Launch the updater to either stage or apply an update. LaunchChildMac(argc, argv, outpid); } + if (restart) { + exit(0); + } #else if (isStaged) { // Launch the updater to replace the installation with the staged updated. diff --git a/tools/profiler/core/platform.cpp b/tools/profiler/core/platform.cpp index b5612c3881a9c..ebae6486fc6af 100644 --- a/tools/profiler/core/platform.cpp +++ b/tools/profiler/core/platform.cpp @@ -22,7 +22,6 @@ #include "mozilla/StaticPtr.h" #include "mozilla/ThreadLocal.h" #include "mozilla/TimeStamp.h" -#include "mozilla/Sprintf.h" #include "mozilla/StaticPtr.h" #include "PseudoStack.h" #include "ThreadInfo.h" @@ -675,7 +674,7 @@ AddDynamicCodeLocationTag(ProfileBuffer* aBuffer, const char* aStr) } } -static const int SAMPLER_MAX_STRING_LENGTH = 128; +static const int SAMPLER_MAX_STRING_LENGTH = 512; static void AddPseudoEntry(PSLockRef aLock, ProfileBuffer* aBuffer, @@ -700,9 +699,17 @@ AddPseudoEntry(PSLockRef aLock, ProfileBuffer* aBuffer, if (entry.isCopyLabel() || dynamicString) { if (dynamicString) { - int bytesWritten = - SprintfLiteral(combinedStringBuffer, "%s %s", sampleLabel, dynamicString); - if (bytesWritten > 0) { + // Create a string that is sampleLabel + ' ' + annotationString. + // Avoid sprintf because it can take a lock on Windows, and this + // code runs during the profiler's "critical section" as defined + // in SamplerThread::SuspendAndSampleAndResumeThread. + size_t labelLength = strlen(sampleLabel); + size_t dynamicLength = strlen(dynamicString); + if (labelLength + 1 + dynamicLength < ArrayLength(combinedStringBuffer)) { + PodCopy(combinedStringBuffer, sampleLabel, labelLength); + combinedStringBuffer[labelLength] = ' '; + PodCopy(&combinedStringBuffer[labelLength + 1], dynamicString, dynamicLength); + combinedStringBuffer[labelLength + 1 + dynamicLength] = '\0'; sampleLabel = combinedStringBuffer; } } @@ -3069,5 +3076,16 @@ profiler_call_exit(void* aHandle) pseudoStack->pop(); } +void* +profiler_get_stack_top() +{ + PSAutoLock lock(gPSMutex); + ThreadInfo* threadInfo = FindLiveThreadInfo(lock); + if (threadInfo) { + return threadInfo->StackTop(); + } + return nullptr; +} + // END externally visible functions //////////////////////////////////////////////////////////////////////// diff --git a/tools/profiler/public/GeckoProfiler.h b/tools/profiler/public/GeckoProfiler.h index eaca45daf38a0..a2b4bc0ead478 100644 --- a/tools/profiler/public/GeckoProfiler.h +++ b/tools/profiler/public/GeckoProfiler.h @@ -315,6 +315,12 @@ PROFILER_FUNC(double profiler_time(), 0) PROFILER_FUNC_VOID(profiler_log(const char *str)) +// Gets the stack top of the current thread. +// +// The thread must have been previously registered with the profiler, otherwise +// this method will return nullptr. +PROFILER_FUNC(void* profiler_get_stack_top(), nullptr) + // End of the functions defined whether the profiler is enabled or not. #if defined(MOZ_GECKO_PROFILER) diff --git a/widget/cocoa/nsCocoaWindow.mm b/widget/cocoa/nsCocoaWindow.mm index efe8b3c79190d..854553c8fb062 100644 --- a/widget/cocoa/nsCocoaWindow.mm +++ b/widget/cocoa/nsCocoaWindow.mm @@ -3418,7 +3418,7 @@ - (void)setTemporaryBackgroundColor - (void)restoreBackgroundColor { - [super setBackgroundColor:mColor]; + [super setBackgroundColor:mBackgroundColor]; } - (void)setTitlebarNeedsDisplayInRect:(NSRect)aRect diff --git a/xpcom/threads/BackgroundHangMonitor.cpp b/xpcom/threads/BackgroundHangMonitor.cpp index 87bb5c25f8e55..0893c8b971d11 100644 --- a/xpcom/threads/BackgroundHangMonitor.cpp +++ b/xpcom/threads/BackgroundHangMonitor.cpp @@ -32,6 +32,16 @@ // It can be scaled back again in the future #define BHR_BETA_MOD 1; +// This variable controls the maximum number of native hang stacks which may be +// attached to a ping. This is due to how large native stacks can be. We want to +// reduce the chance of a ping being discarded due to it exceeding the maximum +// ping size. +// +// NOTE: 300 native hang stacks would, by a rough estimation based on stacks +// collected from nightly on March 21, 2017, take up approximately 168kb of +// space. +static const uint32_t kMaximumNativeHangStacks = 300; + // Maximum depth of the call stack in the reported thread hangs. This value represents // the 99.9th percentile of the thread hangs stack depths reported by Telemetry. static const size_t kMaxThreadHangStackDepth = 30; @@ -180,6 +190,8 @@ class BackgroundHangThread : public LinkedListElement ThreadStackHelper mStackHelper; // Stack of current hang Telemetry::HangStack mHangStack; + // Native stack of current hang + Telemetry::NativeHangStack mNativeHangStack; // Statistics for telemetry Telemetry::ThreadHangStats mStats; // Annotations for the current hang @@ -332,7 +344,19 @@ BackgroundHangManager::RunMonitorThread() if (MOZ_LIKELY(!currentThread->mHanging)) { if (MOZ_UNLIKELY(hangTime >= currentThread->mTimeout)) { // A hang started - currentThread->mStackHelper.GetStack(currentThread->mHangStack); +#ifdef NIGHTLY_BUILD + if (currentThread->mStats.mNativeStackCnt < kMaximumNativeHangStacks) { + // NOTE: In nightly builds of firefox we want to collect native stacks + // for all hangs, not just permahangs. + currentThread->mStats.mNativeStackCnt += 1; + currentThread->mStackHelper.GetPseudoAndNativeStack( + currentThread->mHangStack, currentThread->mNativeHangStack); + } else { + currentThread->mStackHelper.GetPseudoStack(currentThread->mHangStack); + } +#else + currentThread->mStackHelper.GetPseudoStack(currentThread->mHangStack); +#endif currentThread->mHangStart = interval; currentThread->mHanging = true; currentThread->mAnnotations = @@ -451,7 +475,7 @@ BackgroundHangThread::ReportHang(PRIntervalTime aHangTime) mHangStack.erase(mHangStack.begin() + 1, mHangStack.begin() + elementsToRemove); } - Telemetry::HangHistogram newHistogram(Move(mHangStack)); + Telemetry::HangHistogram newHistogram(Move(mHangStack), Move(mNativeHangStack)); for (Telemetry::HangHistogram* oldHistogram = mStats.mHangs.begin(); oldHistogram != mStats.mHangs.end(); oldHistogram++) { if (newHistogram == *oldHistogram) { @@ -475,9 +499,12 @@ BackgroundHangThread::ReportPermaHang() // mManager->mLock IS locked Telemetry::HangHistogram& hang = ReportHang(mMaxTimeout); - Telemetry::HangStack& stack = hang.GetNativeStack(); + Telemetry::NativeHangStack& stack = hang.GetNativeStack(); if (stack.empty()) { - mStackHelper.GetNativeStack(stack); + mStats.mNativeStackCnt += 1; + if (mStats.mNativeStackCnt <= kMaximumNativeHangStacks) { + mStackHelper.GetNativeStack(stack); + } } } diff --git a/xpcom/threads/ThreadStackHelper.cpp b/xpcom/threads/ThreadStackHelper.cpp index 0973e33ed01b7..e0c82ea223031 100644 --- a/xpcom/threads/ThreadStackHelper.cpp +++ b/xpcom/threads/ThreadStackHelper.cpp @@ -134,6 +134,7 @@ ThreadStackHelper::ThreadStackHelper() | THREAD_GET_CONTEXT | THREAD_QUERY_INFORMATION #endif , FALSE, 0); + mStackTop = profiler_get_stack_top(); MOZ_ASSERT(mInitialized); #elif defined(XP_MACOSX) mThreadID = mach_thread_self(); @@ -164,40 +165,42 @@ class ScopedSetPtr } // namespace void -ThreadStackHelper::GetStack(Stack& aStack) +ThreadStackHelper::GetPseudoStack(Stack& aStack) { - GetStackInternal(aStack, /* aAppendNativeStack */ false); + GetStacksInternal(&aStack, nullptr); } void -ThreadStackHelper::GetStackInternal(Stack& aStack, bool aAppendNativeStack) +ThreadStackHelper::GetStacksInternal(Stack* aStack, NativeStack* aNativeStack) { // Always run PrepareStackBuffer first to clear aStack - if (!PrepareStackBuffer(aStack)) { + if (aStack && !PrepareStackBuffer(*aStack)) { // Skip and return empty aStack return; } - ScopedSetPtr stackPtr(mStackToFill, &aStack); + ScopedSetPtr stackPtr(mStackToFill, aStack); #if defined(XP_LINUX) if (!sInitialized) { MOZ_ASSERT(false); return; } - siginfo_t uinfo = {}; - uinfo.si_signo = sFillStackSignum; - uinfo.si_code = SI_QUEUE; - uinfo.si_pid = getpid(); - uinfo.si_uid = getuid(); - uinfo.si_value.sival_ptr = this; - if (::syscall(SYS_rt_tgsigqueueinfo, uinfo.si_pid, - mThreadID, sFillStackSignum, &uinfo)) { - // rt_tgsigqueueinfo was added in Linux 2.6.31. - // Could have failed because the syscall did not exist. - return; + if (aStack) { + siginfo_t uinfo = {}; + uinfo.si_signo = sFillStackSignum; + uinfo.si_code = SI_QUEUE; + uinfo.si_pid = getpid(); + uinfo.si_uid = getuid(); + uinfo.si_value.sival_ptr = this; + if (::syscall(SYS_rt_tgsigqueueinfo, uinfo.si_pid, + mThreadID, sFillStackSignum, &uinfo)) { + // rt_tgsigqueueinfo was added in Linux 2.6.31. + // Could have failed because the syscall did not exist. + return; + } + MOZ_ALWAYS_TRUE(!::sem_wait(&mSem)); } - MOZ_ALWAYS_TRUE(!::sem_wait(&mSem)); #elif defined(XP_WIN) if (!mInitialized) { @@ -205,9 +208,14 @@ ThreadStackHelper::GetStackInternal(Stack& aStack, bool aAppendNativeStack) return; } - if (aAppendNativeStack) { - aStack.EnsureNativeFrameCapacity(Telemetry::HangStack::sMaxNativeFrames); + // NOTE: We can only perform frame pointer stack walking on non win64 + // platforms, because Win64 always omits frame pointers. We don't want to use + // MozStackWalk here, so we just skip collecting stacks entirely. +#ifndef MOZ_THREADSTACKHELPER_X64 + if (aNativeStack) { + aNativeStack->reserve(Telemetry::HangStack::sMaxNativeFrames); } +#endif if (::SuspendThread(mThreadID) == DWORD(-1)) { MOZ_ASSERT(false); @@ -218,21 +226,49 @@ ThreadStackHelper::GetStackInternal(Stack& aStack, bool aAppendNativeStack) // GetThreadContext to ensure it's really suspended. // See https://blogs.msdn.microsoft.com/oldnewthing/20150205-00/?p=44743. CONTEXT context; + memset(&context, 0, sizeof(context)); context.ContextFlags = CONTEXT_CONTROL; if (::GetThreadContext(mThreadID, &context)) { - FillStackBuffer(); - } - - if (aAppendNativeStack) { - auto callback = [](uint32_t, void* aPC, void*, void* aClosure) { - Stack* stack = static_cast(aClosure); - stack->AppendNativeFrame(reinterpret_cast(aPC)); - }; + if (aStack) { + FillStackBuffer(); + } - MozStackWalk(callback, /* skipFrames */ 0, - /* maxFrames */ Telemetry::HangStack::sMaxNativeFrames, - reinterpret_cast(&aStack), - reinterpret_cast(mThreadID), nullptr); +#ifndef MOZ_THREADSTACKHELPER_X64 + if (aNativeStack) { + auto callback = [](uint32_t, void* aPC, void*, void* aClosure) { + NativeStack* stack = static_cast(aClosure); + stack->push_back(reinterpret_cast(aPC)); + }; + + // Now we need to get our frame pointer, our stack pointer, and our stack + // top. Rather than registering and storing the stack tops ourselves, we use + // the gecko profiler to look it up. + void** framePointer = reinterpret_cast(context.Ebp); + void** stackPointer = reinterpret_cast(context.Esp); + + MOZ_ASSERT(mStackTop, "The thread should be registered by the profiler"); + + // Double check that the values we pulled for the thread make sense before + // walking the stack. + if (mStackTop && framePointer >= stackPointer && framePointer < mStackTop) { + // NOTE: In bug 1346415 this was changed to use FramePointerStackWalk. + // This was done because lowering the background hang timer threshold + // would cause it to fire on infra early during the boot process, causing + // a deadlock in MozStackWalk when the target thread was holding the + // windows-internal lock on the function table, as it would be suspended + // before we tried to grab the lock to walk its stack. + // + // FramePointerStackWalk is implemented entirely in userspace and thus + // doesn't have the same issues with deadlocking. Unfortunately as 64-bit + // windows is not guaranteed to have frame pointers, the stack walking + // code is only enabled on 32-bit windows builds (bug 1357829). + FramePointerStackWalk(callback, /* skipFrames */ 0, + /* maxFrames */ Telemetry::HangStack::sMaxNativeFrames, + reinterpret_cast(aNativeStack), framePointer, + mStackTop); + } + } +#endif } MOZ_ALWAYS_TRUE(::ResumeThread(mThreadID) != DWORD(-1)); @@ -246,26 +282,34 @@ ThreadStackHelper::GetStackInternal(Stack& aStack, bool aAppendNativeStack) } # endif - if (::thread_suspend(mThreadID) != KERN_SUCCESS) { - MOZ_ASSERT(false); - return; - } + if (aStack) { + if (::thread_suspend(mThreadID) != KERN_SUCCESS) { + MOZ_ASSERT(false); + return; + } - FillStackBuffer(); + FillStackBuffer(); - MOZ_ALWAYS_TRUE(::thread_resume(mThreadID) == KERN_SUCCESS); + MOZ_ALWAYS_TRUE(::thread_resume(mThreadID) == KERN_SUCCESS); + } #endif } void -ThreadStackHelper::GetNativeStack(Stack& aStack) +ThreadStackHelper::GetNativeStack(NativeStack& aNativeStack) { #ifdef MOZ_THREADSTACKHELPER_NATIVE - GetStackInternal(aStack, /* aAppendNativeStack */ true); + GetStacksInternal(nullptr, &aNativeStack); #endif // MOZ_THREADSTACKHELPER_NATIVE } +void +ThreadStackHelper::GetPseudoAndNativeStack(Stack& aStack, NativeStack& aNativeStack) +{ + GetStacksInternal(&aStack, &aNativeStack); +} + #ifdef XP_LINUX int ThreadStackHelper::sInitialized; diff --git a/xpcom/threads/ThreadStackHelper.h b/xpcom/threads/ThreadStackHelper.h index b3ae4b3083268..a1dafda6f63ca 100644 --- a/xpcom/threads/ThreadStackHelper.h +++ b/xpcom/threads/ThreadStackHelper.h @@ -60,6 +60,12 @@ class ThreadStackHelper public: typedef Telemetry::HangStack Stack; + // When a native stack is gathered, this vector holds the raw program counter + // values that FramePointerStackWalk will return to us after it walks the + // stack. When gathering the Telemetry payload, Telemetry will take care of + // mapping these program counters to proper addresses within modules. + typedef Telemetry::NativeHangStack NativeStack; + private: Stack* mStackToFill; #ifdef MOZ_THREADSTACKHELPER_PSEUDO @@ -99,18 +105,31 @@ class ThreadStackHelper * * @param aStack Stack instance to be filled. */ - void GetStack(Stack& aStack); + void GetPseudoStack(Stack& aStack); /** * Retrieve the current native stack of the thread associated * with this ThreadStackHelper. * - * @param aNativeStack Stack instance to be filled. + * @param aNativeStack NativeStack instance to be filled. + */ + void GetNativeStack(NativeStack& aNativeStack); + + /** + * Retrieve the current pseudostack and native stack of the thread associated + * with this ThreadStackHelper. This method only pauses the target thread once + * to get both stacks. + * + * @param aStack Stack instance to be filled with the pseudostack. + * @param aNativeStack NativeStack instance to be filled with the native stack. */ - void GetNativeStack(Stack& aStack); + void GetPseudoAndNativeStack(Stack& aStack, NativeStack& aNativeStack); private: - void GetStackInternal(Stack& aStack, bool aAppendNativeStack); + // Fill in the passed aStack and aNativeStack datastructures with backtraces. + // If only aStack needs to be collected, nullptr may be passed for + // aNativeStack, and vice versa. + void GetStacksInternal(Stack* aStack, NativeStack* aNativeStack); #if defined(XP_LINUX) private: static int sInitialized; @@ -125,6 +144,7 @@ class ThreadStackHelper private: bool mInitialized; HANDLE mThreadID; + void* mStackTop; #elif defined(XP_MACOSX) private: