From 09c22c28a0f0fc92c670ce118028422aa4693db7 Mon Sep 17 00:00:00 2001 From: Matt Woodrow Date: Tue, 15 Jan 2019 23:55:37 +0000 Subject: [PATCH 01/18] Bug 1520319 - Store an array of pending transactions in LayerTransactionParent to avoid overwriting data when we get multiple. r=jrmuizel Differential Revision: https://phabricator.services.mozilla.com/D16603 --HG-- extra : moz-landing-system : lando --- gfx/layers/ipc/LayerTransactionParent.cpp | 68 +++++++++++++---------- gfx/layers/ipc/LayerTransactionParent.h | 34 +++++------- 2 files changed, 53 insertions(+), 49 deletions(-) diff --git a/gfx/layers/ipc/LayerTransactionParent.cpp b/gfx/layers/ipc/LayerTransactionParent.cpp index ec84ca1e1ffe..017fa051d668 100644 --- a/gfx/layers/ipc/LayerTransactionParent.cpp +++ b/gfx/layers/ipc/LayerTransactionParent.cpp @@ -57,7 +57,6 @@ LayerTransactionParent::LayerTransactionParent( mChildEpoch{0}, mParentEpoch{0}, mVsyncRate(aVsyncRate), - mPendingTransaction{0}, mDestroyed(false), mIPCOpen(false), mUpdateHitTestingTree(false) { @@ -884,39 +883,52 @@ bool LayerTransactionParent::IsSameProcess() const { return OtherPid() == base::GetCurrentProcId(); } +void LayerTransactionParent::SetPendingTransactionId( + TransactionId aId, const VsyncId& aVsyncId, + const TimeStamp& aVsyncStartTime, const TimeStamp& aRefreshStartTime, + const TimeStamp& aTxnStartTime, const TimeStamp& aTxnEndTime, bool aContainsSVG, + const nsCString& aURL, const TimeStamp& aFwdTime) { + mPendingTransactions.AppendElement( + PendingTransaction{aId, aVsyncId, aVsyncStartTime, aRefreshStartTime, + aTxnStartTime, aTxnEndTime, aFwdTime, aURL, aContainsSVG}); +} + TransactionId LayerTransactionParent::FlushTransactionId( - const VsyncId& aId, TimeStamp& aCompositeEnd) { - if (mId.IsValid() && mPendingTransaction.IsValid() && !mVsyncRate.IsZero()) { - RecordContentFrameTime(mTxnVsyncId, mVsyncStartTime, mTxnStartTime, aId, - aCompositeEnd, mTxnEndTime - mTxnStartTime, - mVsyncRate, mContainsSVG, false); - } + const VsyncId& aCompositeId, TimeStamp& aCompositeEnd) { + TransactionId id; + for (auto& transaction : mPendingTransactions) { + id = transaction.mId; + if (mId.IsValid() && transaction.mId.IsValid() && !mVsyncRate.IsZero()) { + RecordContentFrameTime( + transaction.mTxnVsyncId, transaction.mVsyncStartTime, + transaction.mTxnStartTime, aCompositeId, aCompositeEnd, + transaction.mTxnEndTime - transaction.mTxnStartTime, mVsyncRate, + transaction.mContainsSVG, false); + } #if defined(ENABLE_FRAME_LATENCY_LOG) - if (mPendingTransaction.IsValid()) { - if (mRefreshStartTime) { - int32_t latencyMs = - lround((aCompositeEnd - mRefreshStartTime).ToMilliseconds()); - printf_stderr( - "From transaction start to end of generate frame latencyMs %d this " - "%p\n", - latencyMs, this); + if (transaction.mId.IsValid()) { + if (transaction.mRefreshStartTime) { + int32_t latencyMs = lround( + (aCompositeEnd - transaction.mRefreshStartTime).ToMilliseconds()); + printf_stderr( + "From transaction start to end of generate frame latencyMs %d this " + "%p\n", + latencyMs, this); + } + if (transaction.mFwdTime) { + int32_t latencyMs = + lround((aCompositeEnd - transaction.mFwdTime).ToMilliseconds()); + printf_stderr( + "From forwarding transaction to end of generate frame latencyMs %d " + "this %p\n", + latencyMs, this); + } } - if (mFwdTime) { - int32_t latencyMs = lround((aCompositeEnd - mFwdTime).ToMilliseconds()); - printf_stderr( - "From forwarding transaction to end of generate frame latencyMs %d " - "this %p\n", - latencyMs, this); - } - } #endif + } - mRefreshStartTime = TimeStamp(); - mTxnStartTime = TimeStamp(); - mFwdTime = TimeStamp(); - TransactionId id = mPendingTransaction; - mPendingTransaction = TransactionId{0}; + mPendingTransactions.Clear(); return id; } diff --git a/gfx/layers/ipc/LayerTransactionParent.h b/gfx/layers/ipc/LayerTransactionParent.h index cd438f69e0a8..db0eb9afce7c 100644 --- a/gfx/layers/ipc/LayerTransactionParent.h +++ b/gfx/layers/ipc/LayerTransactionParent.h @@ -71,24 +71,13 @@ class LayerTransactionParent final : public PLayerTransactionParent, bool IsSameProcess() const override; - const TransactionId& GetPendingTransactionId() { return mPendingTransaction; } void SetPendingTransactionId(TransactionId aId, const VsyncId& aVsyncId, const TimeStamp& aVsyncStartTime, const TimeStamp& aRefreshStartTime, const TimeStamp& aTxnStartTime, const TimeStamp& aTxnEndTime, bool aContainsSVG, const nsCString& aURL, - const TimeStamp& aFwdTime) { - mPendingTransaction = aId; - mTxnVsyncId = aVsyncId; - mVsyncStartTime = aVsyncStartTime; - mRefreshStartTime = aRefreshStartTime; - mTxnStartTime = aTxnStartTime; - mTxnEndTime = aTxnEndTime; - mContainsSVG = aContainsSVG; - mTxnURL = aURL; - mFwdTime = aFwdTime; - } + const TimeStamp& aFwdTime); TransactionId FlushTransactionId(const VsyncId& aId, TimeStamp& aCompositeEnd); @@ -208,15 +197,18 @@ class LayerTransactionParent final : public PLayerTransactionParent, TimeDuration mVsyncRate; - TransactionId mPendingTransaction; - VsyncId mTxnVsyncId; - TimeStamp mVsyncStartTime; - TimeStamp mRefreshStartTime; - TimeStamp mTxnStartTime; - TimeStamp mTxnEndTime; - TimeStamp mFwdTime; - nsCString mTxnURL; - bool mContainsSVG; + struct PendingTransaction { + TransactionId mId; + VsyncId mTxnVsyncId; + TimeStamp mVsyncStartTime; + TimeStamp mRefreshStartTime; + TimeStamp mTxnStartTime; + TimeStamp mTxnEndTime; + TimeStamp mFwdTime; + nsCString mTxnURL; + bool mContainsSVG; + }; + AutoTArray mPendingTransactions; // When the widget/frame/browser stuff in this process begins its // destruction process, we need to Disconnect() all the currently From 12f566f87b6579b872c45b65892ae44e76f2a611 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Tue, 15 Jan 2019 18:13:44 +0000 Subject: [PATCH 02/18] Bug 1519733 - Stop calling umask in nsOSHelperAppService. r=paolo And remove the mPermissions field, which became unused after bug 878144. Differential Revision: https://phabricator.services.mozilla.com/D16418 --HG-- extra : moz-landing-system : lando --- uriloader/exthandler/mac/nsOSHelperAppService.h | 4 ---- uriloader/exthandler/mac/nsOSHelperAppService.mm | 7 ------- uriloader/exthandler/unix/nsOSHelperAppService.cpp | 6 ------ uriloader/exthandler/unix/nsOSHelperAppService.h | 3 --- 4 files changed, 20 deletions(-) diff --git a/uriloader/exthandler/mac/nsOSHelperAppService.h b/uriloader/exthandler/mac/nsOSHelperAppService.h index 90e2c74ce4c0..7a05a22665e0 100644 --- a/uriloader/exthandler/mac/nsOSHelperAppService.h +++ b/uriloader/exthandler/mac/nsOSHelperAppService.h @@ -19,7 +19,6 @@ class nsOSHelperAppService : public nsExternalHelperAppService { public: - nsOSHelperAppService(); virtual ~nsOSHelperAppService(); // override nsIExternalProtocolService methods @@ -52,9 +51,6 @@ class nsOSHelperAppService : public nsExternalHelperAppService { MOZ_MUST_USE nsresult OSProtocolHandlerExists(const char* aScheme, bool* aHandlerExists) override; - - private: - uint32_t mPermissions; }; #endif // nsOSHelperAppService_h__ diff --git a/uriloader/exthandler/mac/nsOSHelperAppService.mm b/uriloader/exthandler/mac/nsOSHelperAppService.mm index 09b2bafb4508..8eef9d1c014f 100644 --- a/uriloader/exthandler/mac/nsOSHelperAppService.mm +++ b/uriloader/exthandler/mac/nsOSHelperAppService.mm @@ -54,13 +54,6 @@ using mozilla::LogLevel; - (NSArray*)extensionsForMIMEType:(NSString*)aString; @end -nsOSHelperAppService::nsOSHelperAppService() : nsExternalHelperAppService() -{ - mode_t mask = umask(0777); - umask(mask); - mPermissions = 0666 & ~mask; -} - nsOSHelperAppService::~nsOSHelperAppService() {} diff --git a/uriloader/exthandler/unix/nsOSHelperAppService.cpp b/uriloader/exthandler/unix/nsOSHelperAppService.cpp index b1b5a242d947..448269761f82 100644 --- a/uriloader/exthandler/unix/nsOSHelperAppService.cpp +++ b/uriloader/exthandler/unix/nsOSHelperAppService.cpp @@ -49,12 +49,6 @@ static nsresult ParseMIMEType(const nsAString::const_iterator& aStart_iter, inline bool IsNetscapeFormat(const nsACString& aBuffer); -nsOSHelperAppService::nsOSHelperAppService() : nsExternalHelperAppService() { - mode_t mask = umask(0777); - umask(mask); - mPermissions = 0666 & ~mask; -} - nsOSHelperAppService::~nsOSHelperAppService() {} /* diff --git a/uriloader/exthandler/unix/nsOSHelperAppService.h b/uriloader/exthandler/unix/nsOSHelperAppService.h index db2b8db43525..b66c46a6e8a4 100644 --- a/uriloader/exthandler/unix/nsOSHelperAppService.h +++ b/uriloader/exthandler/unix/nsOSHelperAppService.h @@ -21,7 +21,6 @@ class nsILineInputStream; class nsOSHelperAppService : public nsExternalHelperAppService { public: - nsOSHelperAppService(); virtual ~nsOSHelperAppService(); // method overrides for mime.types and mime.info look up steps @@ -52,8 +51,6 @@ class nsOSHelperAppService : public nsExternalHelperAppService { already_AddRefed GetFromExtension(const nsCString& aFileExt); private: - uint32_t mPermissions; - // Helper methods which have to access static members static nsresult UnescapeCommand(const nsAString& aEscapedCommand, const nsAString& aMajorType, From f11cf08512efb434e03086c615164aee072f6ba7 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Mon, 14 Jan 2019 21:51:59 +0000 Subject: [PATCH 03/18] Bug 1519737 - Move pluginProblemBinding.css to the UA style sheet cache. r=emilio,timdream Differential Revision: https://phabricator.services.mozilla.com/D16430 --HG-- rename : toolkit/pluginproblem/content/pluginProblemBinding.css => layout/style/res/pluginproblem.css extra : moz-landing-system : lando --- browser/installer/package-manifest.in | 1 - layout/base/nsDocumentViewer.cpp | 2 +- layout/style/UserAgentStyleSheetList.h | 1 + layout/style/jar.mn | 1 + .../style/res/pluginproblem.css | 0 mobile/android/installer/package-manifest.in | 1 - toolkit/pluginproblem/jar.mn | 1 - toolkit/pluginproblem/moz.build | 4 ---- toolkit/pluginproblem/pluginGlue.manifest | 1 - 9 files changed, 3 insertions(+), 9 deletions(-) rename toolkit/pluginproblem/content/pluginProblemBinding.css => layout/style/res/pluginproblem.css (100%) delete mode 100644 toolkit/pluginproblem/pluginGlue.manifest diff --git a/browser/installer/package-manifest.in b/browser/installer/package-manifest.in index df20841bdec6..84a1642500a9 100644 --- a/browser/installer/package-manifest.in +++ b/browser/installer/package-manifest.in @@ -235,7 +235,6 @@ @RESPATH@/components/nsUpdateTimerManager.js @RESPATH@/components/utils.manifest @RESPATH@/components/simpleServices.js -@RESPATH@/components/pluginGlue.manifest @RESPATH@/components/ProcessSingleton.manifest @RESPATH@/components/MainProcessSingleton.js @RESPATH@/components/ContentProcessSingleton.js diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index b36bf465c3c7..bb291b9f5f60 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/layout/base/nsDocumentViewer.cpp @@ -2347,9 +2347,9 @@ UniquePtr nsDocumentViewer::CreateStyleSet(Document* aDocument) { styleSet->AppendStyleSheet(SheetType::Agent, cache->XULSheet()); } - // Append chrome sheets (scrollbars + forms). styleSet->AppendStyleSheet(SheetType::Agent, cache->FormsSheet()); styleSet->AppendStyleSheet(SheetType::Agent, cache->ScrollbarsSheet()); + styleSet->AppendStyleSheet(SheetType::Agent, cache->PluginProblemSheet()); for (StyleSheet* sheet : *sheetService->AgentStyleSheets()) { styleSet->AppendStyleSheet(SheetType::Agent, sheet); diff --git a/layout/style/UserAgentStyleSheetList.h b/layout/style/UserAgentStyleSheetList.h index 0e8fe4216703..e25c51866942 100644 --- a/layout/style/UserAgentStyleSheetList.h +++ b/layout/style/UserAgentStyleSheetList.h @@ -28,6 +28,7 @@ STYLE_SHEET(MathML, "resource://gre-resources/mathml.css", true) STYLE_SHEET(MinimalXUL, "chrome://global/content/minimal-xul.css", false) STYLE_SHEET(NoFrames, "resource://gre-resources/noframes.css", true) STYLE_SHEET(NoScript, "resource://gre-resources/noscript.css", true) +STYLE_SHEET(PluginProblem, "resource://gre-resources/pluginproblem.css", true) STYLE_SHEET(Quirk, "resource://gre-resources/quirk.css", false) STYLE_SHEET(Scrollbars, "chrome://global/skin/scrollbars.css", true) STYLE_SHEET(SVG, "resource://gre/res/svg.css", false) diff --git a/layout/style/jar.mn b/layout/style/jar.mn index b90dc0516f2a..7d9c60eff541 100644 --- a/layout/style/jar.mn +++ b/layout/style/jar.mn @@ -10,6 +10,7 @@ toolkit.jar: res/noscript.css (res/noscript.css) res/noframes.css (res/noframes.css) * res/forms.css (res/forms.css) + res/pluginproblem.css (res/pluginproblem.css) res/arrow.gif (res/arrow.gif) res/arrow-left.gif (res/arrow-left.gif) res/arrow-right.gif (res/arrow-right.gif) diff --git a/toolkit/pluginproblem/content/pluginProblemBinding.css b/layout/style/res/pluginproblem.css similarity index 100% rename from toolkit/pluginproblem/content/pluginProblemBinding.css rename to layout/style/res/pluginproblem.css diff --git a/mobile/android/installer/package-manifest.in b/mobile/android/installer/package-manifest.in index 61e841a1e1af..8d43767f72fb 100644 --- a/mobile/android/installer/package-manifest.in +++ b/mobile/android/installer/package-manifest.in @@ -167,7 +167,6 @@ @BINPATH@/components/nsUpdateTimerManager.manifest @BINPATH@/components/nsUpdateTimerManager.js -@BINPATH@/components/pluginGlue.manifest @BINPATH@/components/ProcessSingleton.manifest @BINPATH@/components/MainProcessSingleton.js @BINPATH@/components/ContentProcessSingleton.js diff --git a/toolkit/pluginproblem/jar.mn b/toolkit/pluginproblem/jar.mn index 7a2198e74c2c..a9c1665be2cc 100644 --- a/toolkit/pluginproblem/jar.mn +++ b/toolkit/pluginproblem/jar.mn @@ -6,4 +6,3 @@ toolkit.jar: % content pluginproblem %pluginproblem/ contentaccessible=yes pluginproblem/pluginProblem.xml (content/pluginProblem.xml) pluginproblem/pluginProblemContent.css (content/pluginProblemContent.css) - pluginproblem/pluginProblemBinding.css (content/pluginProblemBinding.css) diff --git a/toolkit/pluginproblem/moz.build b/toolkit/pluginproblem/moz.build index 070967341498..aac3a838c4c2 100644 --- a/toolkit/pluginproblem/moz.build +++ b/toolkit/pluginproblem/moz.build @@ -4,8 +4,4 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. -EXTRA_COMPONENTS += [ - 'pluginGlue.manifest', -] - JAR_MANIFESTS += ['jar.mn'] diff --git a/toolkit/pluginproblem/pluginGlue.manifest b/toolkit/pluginproblem/pluginGlue.manifest deleted file mode 100644 index 96ebc9bd4cf6..000000000000 --- a/toolkit/pluginproblem/pluginGlue.manifest +++ /dev/null @@ -1 +0,0 @@ -category agent-style-sheets pluginGlue-pluginProblem chrome://pluginproblem/content/pluginProblemBinding.css From d56e504ec23aa696ede244f155a7ce992630fdcc Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Wed, 16 Jan 2019 00:06:39 +0000 Subject: [PATCH 04/18] Bug 1517642: Make the installer and updater disable the launcher process by default on beta and release; r=mhowell Differential Revision: https://phabricator.services.mozilla.com/D15758 --HG-- extra : moz-landing-system : lando --- browser/installer/windows/nsis/defines.nsi.in | 9 +++ browser/installer/windows/nsis/installer.nsi | 6 ++ browser/installer/windows/nsis/shared.nsh | 26 +++++++ .../installer/windows/nsis/uninstaller.nsi | 6 ++ .../mozapps/installer/windows/nsis/common.nsh | 68 +++++++++++++++++++ 5 files changed, 115 insertions(+) diff --git a/browser/installer/windows/nsis/defines.nsi.in b/browser/installer/windows/nsis/defines.nsi.in index 348df88d4d4f..654659008320 100644 --- a/browser/installer/windows/nsis/defines.nsi.in +++ b/browser/installer/windows/nsis/defines.nsi.in @@ -79,6 +79,15 @@ !define AccessibleHandlerCLSID "{4A195748-DCA2-45FB-9295-0A139E76A9E7}" !endif +#ifdef MOZ_LAUNCHER_PROCESS +!define MOZ_LAUNCHER_PROCESS +!define MOZ_LAUNCHER_SUBKEY "Software\Mozilla\${AppName}\Launcher" +#endif + +#ifdef RELEASE_OR_BETA +!define RELEASE_OR_BETA +#endif + # Due to official and beta using the same branding this is needed to # differentiante between the url used by the stub for downloading. !if "@MOZ_UPDATE_CHANNEL@" == "beta" diff --git a/browser/installer/windows/nsis/installer.nsi b/browser/installer/windows/nsis/installer.nsi index 30a00bd18fff..3e39aa80110b 100755 --- a/browser/installer/windows/nsis/installer.nsi +++ b/browser/installer/windows/nsis/installer.nsi @@ -489,6 +489,12 @@ Section "-Application" APP_IDX ${SetAppLSPCategories} ${LSP_CATEGORIES} ${EndIf} +!ifdef MOZ_LAUNCHER_PROCESS +!ifdef RELEASE_OR_BETA + ${DisableLauncherProcessByDefault} +!endif +!endif + ; Create shortcuts ${LogHeader} "Adding Shortcuts" diff --git a/browser/installer/windows/nsis/shared.nsh b/browser/installer/windows/nsis/shared.nsh index adbf563dd221..15fefb9212ee 100755 --- a/browser/installer/windows/nsis/shared.nsh +++ b/browser/installer/windows/nsis/shared.nsh @@ -162,6 +162,13 @@ ${EndIf} ${EndIf} !endif + +!ifdef MOZ_LAUNCHER_PROCESS +!ifdef RELEASE_OR_BETA + ${DisableLauncherProcessByDefault} +!endif +!endif + !macroend !define PostUpdate "!insertmacro PostUpdate" @@ -1607,3 +1614,22 @@ FunctionEnd !define SetAsDefaultAppUser "Call SetAsDefaultAppUser" !endif ; NO_LOG + +!ifdef MOZ_LAUNCHER_PROCESS +!ifdef RELEASE_OR_BETA +!macro DisableLauncherProcessByDefault + ClearErrors + ${ReadRegQWORD} $0 HKCU ${MOZ_LAUNCHER_SUBKEY} "$INSTDIR\${FileMainEXE}|Launcher" + ${If} ${Errors} + ClearErrors + ${ReadRegQWORD} $0 HKCU ${MOZ_LAUNCHER_SUBKEY} "$INSTDIR\${FileMainEXE}|Browser" + ${If} ${Errors} + ClearErrors + ; New install that hasn't seen this yet; disable by default + ${WriteRegQWORD} HKCU ${MOZ_LAUNCHER_SUBKEY} "$INSTDIR\${FileMainEXE}|Browser" 0 + ${EndIf} + ${EndIf} +!macroend +!define DisableLauncherProcessByDefault "!insertmacro DisableLauncherProcessByDefault" +!endif +!endif diff --git a/browser/installer/windows/nsis/uninstaller.nsi b/browser/installer/windows/nsis/uninstaller.nsi index dd1b4ba324ac..4634e3582507 100755 --- a/browser/installer/windows/nsis/uninstaller.nsi +++ b/browser/installer/windows/nsis/uninstaller.nsi @@ -411,6 +411,12 @@ Section "Uninstall" ${UnregisterDLL} "$INSTDIR\AccessibleHandler.dll" ${EndIf} +!ifdef MOZ_LAUNCHER_PROCESS + DeleteRegValue HKCU ${MOZ_LAUNCHER_SUBKEY} "$INSTDIR\${FileMainEXE}|Launcher" + DeleteRegValue HKCU ${MOZ_LAUNCHER_SUBKEY} "$INSTDIR\${FileMainEXE}|Browser" + DeleteRegValue HKCU ${MOZ_LAUNCHER_SUBKEY} "$INSTDIR\${FileMainEXE}|Image" +!endif + ${un.RemovePrecompleteEntries} "false" ${If} ${FileExists} "$INSTDIR\defaults\pref\channel-prefs.js" diff --git a/toolkit/mozapps/installer/windows/nsis/common.nsh b/toolkit/mozapps/installer/windows/nsis/common.nsh index 4d2cd60233b6..9f47e5b2f2ef 100755 --- a/toolkit/mozapps/installer/windows/nsis/common.nsh +++ b/toolkit/mozapps/installer/windows/nsis/common.nsh @@ -8402,3 +8402,71 @@ end: Pop $1 Pop $0 !macroend + +Function WriteRegQWORD + ; Stack contents: + ; VALUE, VALUE_NAME, SUBKEY, ROOTKEY + Exch $3 ; $3, VALUE_NAME, SUBKEY, ROOTKEY + Exch 1 ; VALUE_NAME, $3, SUBKEY, ROOTKEY + Exch $2 ; $2, $3, SUBKEY, ROOTKEY + Exch 2 ; SUBKEY, $3, $2, ROOTKEY + Exch $1 ; $1, $3, $2, ROOTKEY + Exch 3 ; ROOTKEY, $3, $2, $1 + Exch $0 ; $0, $3, $2, $1 + System::Call "advapi32::RegSetKeyValueW(p r0, w r1, w r2, i 11, *l r3, i 8) i.r0" + ${IfNot} $0 = 0 + SetErrors + ${EndIf} + Pop $0 + Pop $3 + Pop $2 + Pop $1 +FunctionEnd +!macro WriteRegQWORD ROOTKEY SUBKEY VALUE_NAME VALUE + ${If} "${ROOTKEY}" == "HKCR" + Push 0x80000000 + ${ElseIf} "${ROOTKEY}" == "HKCU" + Push 0x80000001 + ${ElseIf} "${ROOTKEY}" == "HKLM" + Push 0x80000002 + ${Endif} + Push "${SUBKEY}" + Push "${VALUE_NAME}" + System::Int64Op ${VALUE} + 0 ; The result is pushed on the stack + Call WriteRegQWORD +!macroend +!define WriteRegQWORD "!insertmacro WriteRegQWORD" + +Function ReadRegQWORD + ; Stack contents: + ; VALUE_NAME, SUBKEY, ROOTKEY + Exch $2 ; $2, SUBKEY, ROOTKEY + Exch 1 ; SUBKEY, $2, ROOTKEY + Exch $1 ; $1, $2, ROOTKEY + Exch 2 ; ROOTKEY, $2, $1 + Exch $0 ; $0, $2, $1 + System::Call "advapi32::RegGetValueW(p r0, w r1, w r2, i 0x48, p 0, *l s, *i 8) i.r0" + ${IfNot} $0 = 0 + SetErrors + ${EndIf} + ; VALUE, $0, $2, $1 + Exch 3 ; $1, $0, $2, VALUE + Pop $1 ; $0, $2, VALUE + Pop $0 ; $2, VALUE + Pop $2 ; VALUE +FunctionEnd +!macro ReadRegQWORD DEST ROOTKEY SUBKEY VALUE_NAME + ${If} "${ROOTKEY}" == "HKCR" + Push 0x80000000 + ${ElseIf} "${ROOTKEY}" == "HKCU" + Push 0x80000001 + ${ElseIf} "${ROOTKEY}" == "HKLM" + Push 0x80000002 + ${Endif} + Push "${SUBKEY}" + Push "${VALUE_NAME}" + Call ReadRegQWORD + Pop ${DEST} +!macroend +!define ReadRegQWORD "!insertmacro ReadRegQWORD" + From 7d58da52ae23483ba94c788e181ccbbcd4edf62b Mon Sep 17 00:00:00 2001 From: Johann Hofmann Date: Tue, 15 Jan 2019 20:17:47 +0000 Subject: [PATCH 05/18] Bug 1519415 - Don't use https://example.com in test_getUserMedia_permission to avoid timeouts on Android. r=jib For some reason that I could not figure out, usage of any https host in these getUserMedia mochitests on Android causes a network proxy error and will, in this particular case, lead to the iframe not loading the test page. This causes our test to timeout. I don't think testing https here is particularly important, and so I'd rather save some effort for myself and just remove it. Differential Revision: https://phabricator.services.mozilla.com/D16538 --HG-- extra : moz-landing-system : lando --- dom/media/tests/mochitest/test_getUserMedia_permission.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/media/tests/mochitest/test_getUserMedia_permission.html b/dom/media/tests/mochitest/test_getUserMedia_permission.html index 3e8934635e9b..996f42448dbd 100644 --- a/dom/media/tests/mochitest/test_getUserMedia_permission.html +++ b/dom/media/tests/mochitest/test_getUserMedia_permission.html @@ -55,7 +55,7 @@ runTest(async () => { // Test gUM in sandboxed vs. regular iframe. - for (const origin of ["http://mochi.test:8888", "https://example.com", "http://test1.mochi.test:8888"]) { + for (const origin of ["http://mochi.test:8888", "http://test1.mochi.test:8888"]) { const src = origin + path; is(await iframeGum({ src, sandbox: "allow-scripts" }), "NotAllowedError", "gUM fails in sandboxed iframe " + origin); From ff86999793e5f7770cbd8172b4b4709f339cf83a Mon Sep 17 00:00:00 2001 From: Johann Hofmann Date: Tue, 15 Jan 2019 20:36:00 +0000 Subject: [PATCH 06/18] Bug 1513378 - Link to the content blocking tour from the "Learn How" link in about:preferences. r=ewright Differential Revision: https://phabricator.services.mozilla.com/D16543 --HG-- extra : moz-landing-system : lando --- browser/components/preferences/in-content/privacy.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/browser/components/preferences/in-content/privacy.js b/browser/components/preferences/in-content/privacy.js index 0c8a06aca9f1..76363495f7b6 100644 --- a/browser/components/preferences/in-content/privacy.js +++ b/browser/components/preferences/in-content/privacy.js @@ -472,9 +472,11 @@ var gPrivacyPane = { let contentBlockingUrl = Services.urlFormatter.formatURLPref("app.support.baseURL") + "content-blocking"; link.setAttribute("href", contentBlockingUrl); + let contentBlockingTour = Services.urlFormatter.formatURLPref("privacy.trackingprotection.introURL") + + `?step=3&newtab=true`; let warningLinks = document.getElementsByClassName("content-blocking-warning-learn-how"); for (let warningLink of warningLinks) { - warningLink.setAttribute("href", contentBlockingUrl); + warningLink.setAttribute("href", contentBlockingTour); } }, From 15a43729b0f0c08b62be7ed76fa709787838383d Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Wed, 16 Jan 2019 00:22:19 +0000 Subject: [PATCH 07/18] Bug 1517636: Add launcher process state to about:support; r=Felipe,flod Differential Revision: https://phabricator.services.mozilla.com/D15759 --HG-- extra : moz-landing-system : lando --- toolkit/content/aboutSupport.js | 13 +++++++++++++ toolkit/content/aboutSupport.xhtml | 9 +++++++++ .../en-US/toolkit/about/aboutSupport.ftl | 6 ++++++ toolkit/modules/Troubleshoot.jsm | 4 ++++ .../tests/browser/browser_Troubleshoot.js | 3 +++ toolkit/xre/LauncherRegistryInfo.h | 8 ++++---- toolkit/xre/nsAppRunner.cpp | 18 ++++++++++++++++++ xpcom/system/nsIXULRuntime.idl | 6 ++++++ 8 files changed, 63 insertions(+), 4 deletions(-) diff --git a/toolkit/content/aboutSupport.js b/toolkit/content/aboutSupport.js index ae7772e415c5..995a1600dcfb 100644 --- a/toolkit/content/aboutSupport.js +++ b/toolkit/content/aboutSupport.js @@ -58,6 +58,19 @@ var snapshotFormatters = { $("updatechannel-box").textContent = data.updateChannel; $("profile-dir-box").textContent = Services.dirsvc.get("ProfD", Ci.nsIFile).path; + try { + let launcherStatusTextId = "launcher-process-status-unknown"; + switch (data.launcherProcessState) { + case 0: + case 1: + case 2: + launcherStatusTextId = "launcher-process-status-" + data.launcherProcessState; + break; + } + + document.l10n.setAttributes($("launcher-process-box"), launcherStatusTextId); + } catch (e) {} + let statusTextId = "multi-process-status-unknown"; // Whitelist of known values with string descriptions: diff --git a/toolkit/content/aboutSupport.xhtml b/toolkit/content/aboutSupport.xhtml index 271c05e54831..beffd1fabbcc 100644 --- a/toolkit/content/aboutSupport.xhtml +++ b/toolkit/content/aboutSupport.xhtml @@ -163,6 +163,15 @@ +#if defined(XP_WIN) && defined(MOZ_LAUNCHER_PROCESS) + + + + + + +#endif + diff --git a/toolkit/locales/en-US/toolkit/about/aboutSupport.ftl b/toolkit/locales/en-US/toolkit/about/aboutSupport.ftl index 29e999e1b3b0..4583aedaaf68 100644 --- a/toolkit/locales/en-US/toolkit/about/aboutSupport.ftl +++ b/toolkit/locales/en-US/toolkit/about/aboutSupport.ftl @@ -48,6 +48,7 @@ app-basics-memory-use = Memory Use app-basics-performance = Performance app-basics-service-workers = Registered Service Workers app-basics-profiles = Profiles +app-basics-launcher-process-status = Launcher Process app-basics-multi-process-support = Multiprocess Windows app-basics-process-count = Web Content Processes app-basics-enterprise-policies = Enterprise Policies @@ -260,6 +261,11 @@ sandbox-proc-type-content = content sandbox-proc-type-file = file content sandbox-proc-type-media-plugin = media plugin +launcher-process-status-0 = Enabled +launcher-process-status-1 = Disabled due to failure +launcher-process-status-2 = Disabled forcibly +launcher-process-status-unknown = Unknown status + # Variables # $remoteWindows (integer) - Number of remote windows # $totalWindows (integer) - Number of total windows diff --git a/toolkit/modules/Troubleshoot.jsm b/toolkit/modules/Troubleshoot.jsm index 0fdec5a3a392..07a5eb80e36e 100644 --- a/toolkit/modules/Troubleshoot.jsm +++ b/toolkit/modules/Troubleshoot.jsm @@ -197,6 +197,10 @@ var dataProviders = { } } + try { + data.launcherProcessState = Services.appinfo.launcherProcessState; + } catch(e) {} + data.remoteAutoStart = Services.appinfo.browserTabsRemoteAutostart; // Services.ppmm.childCount is a count of how many processes currently diff --git a/toolkit/modules/tests/browser/browser_Troubleshoot.js b/toolkit/modules/tests/browser/browser_Troubleshoot.js index 16062cf91095..7cdfeb332ccb 100644 --- a/toolkit/modules/tests/browser/browser_Troubleshoot.js +++ b/toolkit/modules/tests/browser/browser_Troubleshoot.js @@ -125,6 +125,9 @@ const SNAPSHOT_SCHEMA = { supportURL: { type: "string", }, + launcherProcessState: { + type: "number", + }, remoteAutoStart: { type: "boolean", required: true, diff --git a/toolkit/xre/LauncherRegistryInfo.h b/toolkit/xre/LauncherRegistryInfo.h index 04b6df6a5097..4de22011f3a2 100644 --- a/toolkit/xre/LauncherRegistryInfo.h +++ b/toolkit/xre/LauncherRegistryInfo.h @@ -26,10 +26,10 @@ class LauncherRegistryInfo final { public: enum class ProcessType { Launcher, Browser }; - enum class EnabledState { - Enabled, - FailDisabled, - ForceDisabled, + enum class EnabledState : uint32_t { + Enabled = 0, + FailDisabled = 1, + ForceDisabled = 2, }; LauncherRegistryInfo() : mBinPath(GetFullBinaryPath().get()) {} diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp index 7dde3d88f4dc..3d9f0daa895c 100644 --- a/toolkit/xre/nsAppRunner.cpp +++ b/toolkit/xre/nsAppRunner.cpp @@ -945,6 +945,24 @@ nsXULAppInfo::GetRestartedByOS(bool* aResult) { return NS_OK; } +NS_IMETHODIMP +nsXULAppInfo::GetLauncherProcessState(uint32_t* aResult) { +#if defined(XP_WIN) && defined(MOZ_LAUNCHER_PROCESS) + LauncherRegistryInfo launcherInfo; + + LauncherResult state = + launcherInfo.IsEnabled(); + if (state.isErr()) { + return NS_ERROR_UNEXPECTED; + } + + *aResult = static_cast(state.unwrap()); + return NS_OK; +#else + return NS_ERROR_NOT_AVAILABLE; +#endif +} + #ifdef XP_WIN // Matches the enum in WinNT.h for the Vista SDK but renamed so that we can // safely build with the Vista SDK and without it. diff --git a/xpcom/system/nsIXULRuntime.idl b/xpcom/system/nsIXULRuntime.idl index 991f05f8939a..636bee1517a3 100644 --- a/xpcom/system/nsIXULRuntime.idl +++ b/xpcom/system/nsIXULRuntime.idl @@ -202,4 +202,10 @@ interface nsIXULRuntime : nsISupports * restart mechanism (such as RegisterApplicationRestart on Windows). */ readonly attribute boolean restartedByOS; + + /** + * Returns a value corresponding to one of the + * |mozilla::LauncherRegistryInfo::EnabledState| values. + */ + readonly attribute uint32_t launcherProcessState; }; From 59d279f51f3de9f6379086ef39feac3edcab281f Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Tue, 15 Jan 2019 17:39:18 -0700 Subject: [PATCH 08/18] Bug 1517636: Follow-up - fix eslint error; r=bustage --- toolkit/modules/Troubleshoot.jsm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/modules/Troubleshoot.jsm b/toolkit/modules/Troubleshoot.jsm index 07a5eb80e36e..f02504cfdbc3 100644 --- a/toolkit/modules/Troubleshoot.jsm +++ b/toolkit/modules/Troubleshoot.jsm @@ -199,7 +199,7 @@ var dataProviders = { try { data.launcherProcessState = Services.appinfo.launcherProcessState; - } catch(e) {} + } catch (e) {} data.remoteAutoStart = Services.appinfo.browserTabsRemoteAutostart; From a94714d9aa5af16d24c3d668cbb1a715d9b9ac76 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Wed, 16 Jan 2019 00:34:24 +0000 Subject: [PATCH 09/18] Bug 1515816 - Fix missing OOM check in ReadableStreamCreateReadResult. r=arai Differential Revision: https://phabricator.services.mozilla.com/D16361 --HG-- extra : moz-landing-system : lando --- js/src/builtin/Stream.cpp | 3 +++ js/src/jit-test/tests/stream/bug-1515816.js | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 js/src/jit-test/tests/stream/bug-1515816.js diff --git a/js/src/builtin/Stream.cpp b/js/src/builtin/Stream.cpp index 1927f2fb3f48..25306c16e7ea 100644 --- a/js/src/builtin/Stream.cpp +++ b/js/src/builtin/Stream.cpp @@ -1540,6 +1540,9 @@ static MOZ_MUST_USE JSObject* ReadableStreamCreateReadResult( ? cx->realm()->getOrCreateIterResultTemplateObject(cx) : cx->realm()->getOrCreateIterResultWithoutPrototypeTemplateObject( cx)); + if (!templateObject) { + return nullptr; + } // Step 3: Assert: Type(done) is Boolean (implicit). diff --git a/js/src/jit-test/tests/stream/bug-1515816.js b/js/src/jit-test/tests/stream/bug-1515816.js new file mode 100644 index 000000000000..7117f22f95e9 --- /dev/null +++ b/js/src/jit-test/tests/stream/bug-1515816.js @@ -0,0 +1,17 @@ +// |jit-test| --no-ion; --no-baseline; skip-if: !('oomAfterAllocations' in this) +// Don't crash on OOM in ReadableStreamDefaultReader.prototype.read(). + +for (let n = 1; n < 1000; n++) { + let stream = new ReadableStream({ + start(controller) { + controller.enqueue(7); + } + }); + let reader = stream.getReader(); + oomAfterAllocations(n); + try { + reader.read(); + n = 1000; + } catch {} + resetOOMFailure(); +} From c54c918b663bc20694f67fb1e3fdb324d66bfe81 Mon Sep 17 00:00:00 2001 From: Junior Hsu Date: Tue, 15 Jan 2019 23:15:38 +0000 Subject: [PATCH 10/18] Bug 1510979 - add a telemetry xpcshell-test for socket process r=janerik We need a test-only IPC message to socket process to trigger the Telemetry::Scalar set since no js engine in the socket process. And hook the IPC call to AddPendingEvent |CallOrWaitSocketProcess| introduced by bug 1496257. Differential Revision: https://phabricator.services.mozilla.com/D14822 --HG-- extra : moz-landing-system : lando --- netwerk/base/nsINetUtil.idl | 5 ++ netwerk/base/nsIOService.cpp | 9 ++++ netwerk/ipc/PSocketProcess.ipdl | 2 + netwerk/ipc/SocketProcessChild.cpp | 7 +++ netwerk/ipc/SocketProcessChild.h | 1 + .../tests/unit/test_SocketScalars.js | 46 +++++++++++++++++++ .../telemetry/tests/unit/xpcshell.ini | 2 + 7 files changed, 72 insertions(+) create mode 100644 toolkit/components/telemetry/tests/unit/test_SocketScalars.js diff --git a/netwerk/base/nsINetUtil.idl b/netwerk/base/nsINetUtil.idl index 8fb7a13b28f8..83e2634da6b7 100644 --- a/netwerk/base/nsINetUtil.idl +++ b/netwerk/base/nsINetUtil.idl @@ -224,6 +224,11 @@ interface nsINetUtil : nsISupports */ ACString getReferrerPolicyString(in unsigned long aPolicy); + /** + * This is test-only. Send an IPC message to let socket process send a + * telemetry. + */ + void socketProcessTelemetryPing(); /** * This is a void method that is C++ implemented and always diff --git a/netwerk/base/nsIOService.cpp b/netwerk/base/nsIOService.cpp index e02be85283d7..f5c3e44e78b3 100644 --- a/netwerk/base/nsIOService.cpp +++ b/netwerk/base/nsIOService.cpp @@ -507,6 +507,15 @@ RefPtr nsIOService::GetSocketProcessMemoryReporter() { return new SocketProcessMemoryReporter(); } +NS_IMETHODIMP +nsIOService::SocketProcessTelemetryPing() { + CallOrWaitForSocketProcess([]() { + Unused << gIOService->mSocketProcess->GetActor() + ->SendSocketProcessTelemetryPing(); + }); + return NS_OK; +} + NS_IMPL_ISUPPORTS(nsIOService, nsIIOService, nsINetUtil, nsISpeculativeConnect, nsIObserver, nsIIOServiceInternal, nsISupportsWeakReference) diff --git a/netwerk/ipc/PSocketProcess.ipdl b/netwerk/ipc/PSocketProcess.ipdl index fac64aaca504..a80064803a85 100644 --- a/netwerk/ipc/PSocketProcess.ipdl +++ b/netwerk/ipc/PSocketProcess.ipdl @@ -43,6 +43,8 @@ child: async SetOffline(bool offline); async InitSocketProcessBridgeParent(ProcessId processId, Endpoint endpoint); async InitProfiler(Endpoint aEndpoint); + // test-only + async SocketProcessTelemetryPing(); }; } // namespace net diff --git a/netwerk/ipc/SocketProcessChild.cpp b/netwerk/ipc/SocketProcessChild.cpp index 192c6ed7d328..974584548a38 100644 --- a/netwerk/ipc/SocketProcessChild.cpp +++ b/netwerk/ipc/SocketProcessChild.cpp @@ -151,6 +151,13 @@ mozilla::ipc::IPCResult SocketProcessChild::RecvInitProfiler( return IPC_OK(); } +mozilla::ipc::IPCResult SocketProcessChild::RecvSocketProcessTelemetryPing() { + const uint32_t kExpectedUintValue = 42; + Telemetry::ScalarSet(Telemetry::ScalarID::TELEMETRY_TEST_SOCKET_ONLY_UINT, + kExpectedUintValue); + return IPC_OK(); +} + void SocketProcessChild::DestroySocketProcessBridgeParent(ProcessId aId) { MOZ_ASSERT(NS_IsMainThread()); diff --git a/netwerk/ipc/SocketProcessChild.h b/netwerk/ipc/SocketProcessChild.h index 99c2a4480189..da71dab830bd 100644 --- a/netwerk/ipc/SocketProcessChild.h +++ b/netwerk/ipc/SocketProcessChild.h @@ -42,6 +42,7 @@ class SocketProcessChild final : public PSocketProcessChild { Endpoint&& aEndpoint) override; mozilla::ipc::IPCResult RecvInitProfiler( Endpoint&& aEndpoint) override; + mozilla::ipc::IPCResult RecvSocketProcessTelemetryPing() override; void CleanUp(); void DestroySocketProcessBridgeParent(ProcessId aId); diff --git a/toolkit/components/telemetry/tests/unit/test_SocketScalars.js b/toolkit/components/telemetry/tests/unit/test_SocketScalars.js new file mode 100644 index 000000000000..145747a1475e --- /dev/null +++ b/toolkit/components/telemetry/tests/unit/test_SocketScalars.js @@ -0,0 +1,46 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ +*/ + +ChromeUtils.import("resource://gre/modules/Services.jsm"); +ChromeUtils.import("resource://gre/modules/TelemetryController.jsm"); +ChromeUtils.import("resource://testing-common/ContentTaskUtils.jsm"); + +const SOCKET_ONLY_UINT_SCALAR = "telemetry.test.socket_only_uint"; + +/** + * This function waits until socket scalars are reported into the + * scalar snapshot. + */ +async function waitForSocketScalars() { + await ContentTaskUtils.waitForCondition(() => { + const scalars = Telemetry.getSnapshotForScalars("main", false); + return Object.keys(scalars).includes("socket"); + }); +} + +add_task(async function() { + if (!Services.prefs.getBoolPref("network.process.enabled")) { + Assert.ok(true, "Test finished: no point to test telemetry from socket process with lanuching the process"); + return; + } + + do_test_pending(); + + do_get_profile(true); + await TelemetryController.testSetup(); + + Services.netUtils.socketProcessTelemetryPing(); + + // Once scalars are set by the socket process, they don't immediately get + // sent to the parent process. Wait for the Telemetry IPC Timer to trigger + // and batch send the data back to the parent process. + await waitForSocketScalars(); + + Assert.equal(Telemetry.getSnapshotForScalars("main", false) + .socket[SOCKET_ONLY_UINT_SCALAR], + 42, + `${SOCKET_ONLY_UINT_SCALAR} must have the correct value (socket process).`); + do_test_finished(); +}); + diff --git a/toolkit/components/telemetry/tests/unit/xpcshell.ini b/toolkit/components/telemetry/tests/unit/xpcshell.ini index e72281c706de..4476578a2120 100644 --- a/toolkit/components/telemetry/tests/unit/xpcshell.ini +++ b/toolkit/components/telemetry/tests/unit/xpcshell.ini @@ -67,6 +67,8 @@ skip-if = os == "android" # Disabled due to crashes (see bug 1331366) tags = addons [test_ChildScalars.js] skip-if = os == "android" # Disabled due to crashes (see bug 1331366) +[test_SocketScalars.js] +skip-if = os == "android" [test_TelemetryReportingPolicy.js] skip-if = os == "android" # Disabled due to crashes (see bug 1367762) tags = addons From de46e66fcd3535d91c1896c4ed2e7a3f75013388 Mon Sep 17 00:00:00 2001 From: Dorel Luca Date: Wed, 16 Jan 2019 03:25:22 +0200 Subject: [PATCH 11/18] Backed out changeset a85699150a8b (bug 1519737) for browser chrome failure in browser/base/content/test/static/browser_parsable_css.js --HG-- rename : layout/style/res/pluginproblem.css => toolkit/pluginproblem/content/pluginProblemBinding.css --- browser/installer/package-manifest.in | 1 + layout/base/nsDocumentViewer.cpp | 2 +- layout/style/UserAgentStyleSheetList.h | 1 - layout/style/jar.mn | 1 - mobile/android/installer/package-manifest.in | 1 + .../pluginproblem/content/pluginProblemBinding.css | 0 toolkit/pluginproblem/jar.mn | 1 + toolkit/pluginproblem/moz.build | 4 ++++ toolkit/pluginproblem/pluginGlue.manifest | 1 + 9 files changed, 9 insertions(+), 3 deletions(-) rename layout/style/res/pluginproblem.css => toolkit/pluginproblem/content/pluginProblemBinding.css (100%) create mode 100644 toolkit/pluginproblem/pluginGlue.manifest diff --git a/browser/installer/package-manifest.in b/browser/installer/package-manifest.in index 84a1642500a9..df20841bdec6 100644 --- a/browser/installer/package-manifest.in +++ b/browser/installer/package-manifest.in @@ -235,6 +235,7 @@ @RESPATH@/components/nsUpdateTimerManager.js @RESPATH@/components/utils.manifest @RESPATH@/components/simpleServices.js +@RESPATH@/components/pluginGlue.manifest @RESPATH@/components/ProcessSingleton.manifest @RESPATH@/components/MainProcessSingleton.js @RESPATH@/components/ContentProcessSingleton.js diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index bb291b9f5f60..b36bf465c3c7 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/layout/base/nsDocumentViewer.cpp @@ -2347,9 +2347,9 @@ UniquePtr nsDocumentViewer::CreateStyleSet(Document* aDocument) { styleSet->AppendStyleSheet(SheetType::Agent, cache->XULSheet()); } + // Append chrome sheets (scrollbars + forms). styleSet->AppendStyleSheet(SheetType::Agent, cache->FormsSheet()); styleSet->AppendStyleSheet(SheetType::Agent, cache->ScrollbarsSheet()); - styleSet->AppendStyleSheet(SheetType::Agent, cache->PluginProblemSheet()); for (StyleSheet* sheet : *sheetService->AgentStyleSheets()) { styleSet->AppendStyleSheet(SheetType::Agent, sheet); diff --git a/layout/style/UserAgentStyleSheetList.h b/layout/style/UserAgentStyleSheetList.h index e25c51866942..0e8fe4216703 100644 --- a/layout/style/UserAgentStyleSheetList.h +++ b/layout/style/UserAgentStyleSheetList.h @@ -28,7 +28,6 @@ STYLE_SHEET(MathML, "resource://gre-resources/mathml.css", true) STYLE_SHEET(MinimalXUL, "chrome://global/content/minimal-xul.css", false) STYLE_SHEET(NoFrames, "resource://gre-resources/noframes.css", true) STYLE_SHEET(NoScript, "resource://gre-resources/noscript.css", true) -STYLE_SHEET(PluginProblem, "resource://gre-resources/pluginproblem.css", true) STYLE_SHEET(Quirk, "resource://gre-resources/quirk.css", false) STYLE_SHEET(Scrollbars, "chrome://global/skin/scrollbars.css", true) STYLE_SHEET(SVG, "resource://gre/res/svg.css", false) diff --git a/layout/style/jar.mn b/layout/style/jar.mn index 7d9c60eff541..b90dc0516f2a 100644 --- a/layout/style/jar.mn +++ b/layout/style/jar.mn @@ -10,7 +10,6 @@ toolkit.jar: res/noscript.css (res/noscript.css) res/noframes.css (res/noframes.css) * res/forms.css (res/forms.css) - res/pluginproblem.css (res/pluginproblem.css) res/arrow.gif (res/arrow.gif) res/arrow-left.gif (res/arrow-left.gif) res/arrow-right.gif (res/arrow-right.gif) diff --git a/mobile/android/installer/package-manifest.in b/mobile/android/installer/package-manifest.in index 8d43767f72fb..61e841a1e1af 100644 --- a/mobile/android/installer/package-manifest.in +++ b/mobile/android/installer/package-manifest.in @@ -167,6 +167,7 @@ @BINPATH@/components/nsUpdateTimerManager.manifest @BINPATH@/components/nsUpdateTimerManager.js +@BINPATH@/components/pluginGlue.manifest @BINPATH@/components/ProcessSingleton.manifest @BINPATH@/components/MainProcessSingleton.js @BINPATH@/components/ContentProcessSingleton.js diff --git a/layout/style/res/pluginproblem.css b/toolkit/pluginproblem/content/pluginProblemBinding.css similarity index 100% rename from layout/style/res/pluginproblem.css rename to toolkit/pluginproblem/content/pluginProblemBinding.css diff --git a/toolkit/pluginproblem/jar.mn b/toolkit/pluginproblem/jar.mn index a9c1665be2cc..7a2198e74c2c 100644 --- a/toolkit/pluginproblem/jar.mn +++ b/toolkit/pluginproblem/jar.mn @@ -6,3 +6,4 @@ toolkit.jar: % content pluginproblem %pluginproblem/ contentaccessible=yes pluginproblem/pluginProblem.xml (content/pluginProblem.xml) pluginproblem/pluginProblemContent.css (content/pluginProblemContent.css) + pluginproblem/pluginProblemBinding.css (content/pluginProblemBinding.css) diff --git a/toolkit/pluginproblem/moz.build b/toolkit/pluginproblem/moz.build index aac3a838c4c2..070967341498 100644 --- a/toolkit/pluginproblem/moz.build +++ b/toolkit/pluginproblem/moz.build @@ -4,4 +4,8 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +EXTRA_COMPONENTS += [ + 'pluginGlue.manifest', +] + JAR_MANIFESTS += ['jar.mn'] diff --git a/toolkit/pluginproblem/pluginGlue.manifest b/toolkit/pluginproblem/pluginGlue.manifest new file mode 100644 index 000000000000..96ebc9bd4cf6 --- /dev/null +++ b/toolkit/pluginproblem/pluginGlue.manifest @@ -0,0 +1 @@ +category agent-style-sheets pluginGlue-pluginProblem chrome://pluginproblem/content/pluginProblemBinding.css From 31a8e2ccae6d2a6419a4fecf63b1b48b462f3b60 Mon Sep 17 00:00:00 2001 From: Chris Martin Date: Wed, 16 Jan 2019 01:21:16 +0000 Subject: [PATCH 12/18] Bug 1508312 - Add assert to catch accidental re-enter of Slim RW lock r=ccorcoran,aklotz WindowsDllBlocklist installs a callback function that fires whenever a DLL is loaded. The installer function shares an SRWLock with the callback function. SRWLock is not re-entrant, so if the installer function accidently causes a DLL load before releasing the lock, the callback function will deadlock. This occured trying to solve Bug 1402282, where the installer function used "new" to allocate memory, which called the Win32 "RtlGenRandom()" function, which loaded bcrypt.dll, which caused the callback to fire off, which tried to lock the mutex that was already locked by the installer function. Hopefully this will save another developer lots of debug time in the future by turning a difficult-to-debug deadlock into a nice, loud assertion. Differential Revision: https://phabricator.services.mozilla.com/D15840 --HG-- extra : moz-landing-system : lando --- mozglue/build/MozglueUtils.h | 104 +++++++++++++++++++++++--- mozglue/build/WindowsDllBlocklist.cpp | 2 +- 2 files changed, 96 insertions(+), 10 deletions(-) diff --git a/mozglue/build/MozglueUtils.h b/mozglue/build/MozglueUtils.h index 6383cf01c1fa..2291a352a5a3 100644 --- a/mozglue/build/MozglueUtils.h +++ b/mozglue/build/MozglueUtils.h @@ -9,18 +9,104 @@ #include +#include "mozilla/Atomics.h" #include "mozilla/Attributes.h" namespace mozilla { namespace glue { -class MOZ_RAII AutoSharedLock final { +#ifdef DEBUG + +class MOZ_STATIC_CLASS Win32SRWLock final { public: - explicit AutoSharedLock(SRWLOCK& aLock) : mLock(aLock) { - ::AcquireSRWLockShared(&aLock); + // Microsoft guarantees that '0' is never a valid thread id + // https://docs.microsoft.com/en-ca/windows/desktop/ProcThread/thread-handles-and-identifiers + static const DWORD kInvalidThreadId = 0; + + constexpr Win32SRWLock() + : mExclusiveThreadId(kInvalidThreadId), mLock(SRWLOCK_INIT) {} + + ~Win32SRWLock() { MOZ_ASSERT(mExclusiveThreadId == kInvalidThreadId); } + + void LockShared() { + MOZ_ASSERT( + mExclusiveThreadId != GetCurrentThreadId(), + "Deadlock detected - A thread attempted to acquire a shared lock on " + "a SRWLOCK when it already owns the exclusive lock on it."); + + ::AcquireSRWLockShared(&mLock); } - ~AutoSharedLock() { ::ReleaseSRWLockShared(&mLock); } + void UnlockShared() { ::ReleaseSRWLockShared(&mLock); } + + void LockExclusive() { + MOZ_ASSERT( + mExclusiveThreadId != GetCurrentThreadId(), + "Deadlock detected - A thread attempted to acquire an exclusive lock " + "on a SRWLOCK when it already owns the exclusive lock on it."); + + ::AcquireSRWLockExclusive(&mLock); + mExclusiveThreadId = GetCurrentThreadId(); + } + + void UnlockExclusive() { + MOZ_ASSERT(mExclusiveThreadId == GetCurrentThreadId()); + + mExclusiveThreadId = kInvalidThreadId; + ::ReleaseSRWLockExclusive(&mLock); + } + + Win32SRWLock(const Win32SRWLock&) = delete; + Win32SRWLock(Win32SRWLock&&) = delete; + Win32SRWLock& operator=(const Win32SRWLock&) = delete; + Win32SRWLock& operator=(Win32SRWLock&&) = delete; + + private: + // "Relaxed" memory ordering is fine. Threads will see other thread IDs + // appear here in some non-deterministic ordering (or not at all) and simply + // ignore them. + // + // But a thread will only read its own ID if it previously wrote it, and a + // single thread doesn't need a memory barrier to read its own write. + + Atomic mExclusiveThreadId; + SRWLOCK mLock; +}; + +#else // DEBUG + +class MOZ_STATIC_CLASS Win32SRWLock final { + public: + constexpr Win32SRWLock() : mLock(SRWLOCK_INIT) {} + + void LockShared() { ::AcquireSRWLockShared(&mLock); } + + void UnlockShared() { ::ReleaseSRWLockShared(&mLock); } + + void LockExclusive() { ::AcquireSRWLockExclusive(&mLock); } + + void UnlockExclusive() { ::ReleaseSRWLockExclusive(&mLock); } + + ~Win32SRWLock() = default; + + Win32SRWLock(const Win32SRWLock&) = delete; + Win32SRWLock(Win32SRWLock&&) = delete; + Win32SRWLock& operator=(const Win32SRWLock&) = delete; + Win32SRWLock& operator=(Win32SRWLock&&) = delete; + + private: + SRWLOCK mLock; +}; + +#endif + +class MOZ_RAII AutoSharedLock final { + public: + explicit AutoSharedLock(Win32SRWLock& aLock) : mLock(aLock) { + mLock.LockShared(); + } + + ~AutoSharedLock() { mLock.UnlockShared(); } AutoSharedLock(const AutoSharedLock&) = delete; AutoSharedLock(AutoSharedLock&&) = delete; @@ -28,16 +114,16 @@ class MOZ_RAII AutoSharedLock final { AutoSharedLock& operator=(AutoSharedLock&&) = delete; private: - SRWLOCK& mLock; + Win32SRWLock& mLock; }; class MOZ_RAII AutoExclusiveLock final { public: - explicit AutoExclusiveLock(SRWLOCK& aLock) : mLock(aLock) { - ::AcquireSRWLockExclusive(&aLock); + explicit AutoExclusiveLock(Win32SRWLock& aLock) : mLock(aLock) { + mLock.LockExclusive(); } - ~AutoExclusiveLock() { ::ReleaseSRWLockExclusive(&mLock); } + ~AutoExclusiveLock() { mLock.UnlockExclusive(); } AutoExclusiveLock(const AutoExclusiveLock&) = delete; AutoExclusiveLock(AutoExclusiveLock&&) = delete; @@ -45,7 +131,7 @@ class MOZ_RAII AutoExclusiveLock final { AutoExclusiveLock& operator=(AutoExclusiveLock&&) = delete; private: - SRWLOCK& mLock; + Win32SRWLock& mLock; }; } // namespace glue diff --git a/mozglue/build/WindowsDllBlocklist.cpp b/mozglue/build/WindowsDllBlocklist.cpp index 25c56847648b..15d2304b1aef 100644 --- a/mozglue/build/WindowsDllBlocklist.cpp +++ b/mozglue/build/WindowsDllBlocklist.cpp @@ -47,7 +47,7 @@ using namespace mozilla; using CrashReporter::Annotation; using CrashReporter::AnnotationToString; -static SRWLOCK gDllServicesLock = SRWLOCK_INIT; +static glue::Win32SRWLock gDllServicesLock; static glue::detail::DllServicesBase* gDllServices; #define DLL_BLOCKLIST_ENTRY(name, ...) {name, __VA_ARGS__}, From cc5d047c75de8c5611c9622a56768b493924ed0d Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Wed, 16 Jan 2019 03:37:43 +0000 Subject: [PATCH 13/18] Bug 1519737 - Move pluginProblemBinding.css to the UA style sheet cache. r=emilio,timdream Differential Revision: https://phabricator.services.mozilla.com/D16430 --HG-- rename : toolkit/pluginproblem/content/pluginProblemBinding.css => layout/style/res/pluginproblem.css extra : moz-landing-system : lando --- browser/base/content/test/static/browser_parsable_css.js | 2 +- browser/installer/package-manifest.in | 1 - layout/base/nsDocumentViewer.cpp | 2 +- layout/style/UserAgentStyleSheetList.h | 1 + layout/style/jar.mn | 1 + .../style/res/pluginproblem.css | 0 mobile/android/installer/package-manifest.in | 1 - toolkit/pluginproblem/jar.mn | 1 - toolkit/pluginproblem/moz.build | 4 ---- toolkit/pluginproblem/pluginGlue.manifest | 1 - 10 files changed, 4 insertions(+), 10 deletions(-) rename toolkit/pluginproblem/content/pluginProblemBinding.css => layout/style/res/pluginproblem.css (100%) delete mode 100644 toolkit/pluginproblem/pluginGlue.manifest diff --git a/browser/base/content/test/static/browser_parsable_css.js b/browser/base/content/test/static/browser_parsable_css.js index f799b23d0e5b..09a5f137f391 100644 --- a/browser/base/content/test/static/browser_parsable_css.js +++ b/browser/base/content/test/static/browser_parsable_css.js @@ -32,7 +32,7 @@ let whitelist = [ errorMessage: /Expected media feature name but found \u2018-moz.*/i, isFromDevTools: false}, - {sourceName: /\b(contenteditable|EditorOverride|svg|forms|html|mathml|ua)\.css$/i, + {sourceName: /\b(contenteditable|EditorOverride|svg|forms|html|mathml|ua|pluginproblem)\.css$/i, errorMessage: /Unknown pseudo-class.*-moz-/i, isFromDevTools: false}, {sourceName: /\b(html|mathml|ua)\.css$/i, diff --git a/browser/installer/package-manifest.in b/browser/installer/package-manifest.in index df20841bdec6..84a1642500a9 100644 --- a/browser/installer/package-manifest.in +++ b/browser/installer/package-manifest.in @@ -235,7 +235,6 @@ @RESPATH@/components/nsUpdateTimerManager.js @RESPATH@/components/utils.manifest @RESPATH@/components/simpleServices.js -@RESPATH@/components/pluginGlue.manifest @RESPATH@/components/ProcessSingleton.manifest @RESPATH@/components/MainProcessSingleton.js @RESPATH@/components/ContentProcessSingleton.js diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index b36bf465c3c7..bb291b9f5f60 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/layout/base/nsDocumentViewer.cpp @@ -2347,9 +2347,9 @@ UniquePtr nsDocumentViewer::CreateStyleSet(Document* aDocument) { styleSet->AppendStyleSheet(SheetType::Agent, cache->XULSheet()); } - // Append chrome sheets (scrollbars + forms). styleSet->AppendStyleSheet(SheetType::Agent, cache->FormsSheet()); styleSet->AppendStyleSheet(SheetType::Agent, cache->ScrollbarsSheet()); + styleSet->AppendStyleSheet(SheetType::Agent, cache->PluginProblemSheet()); for (StyleSheet* sheet : *sheetService->AgentStyleSheets()) { styleSet->AppendStyleSheet(SheetType::Agent, sheet); diff --git a/layout/style/UserAgentStyleSheetList.h b/layout/style/UserAgentStyleSheetList.h index 0e8fe4216703..e25c51866942 100644 --- a/layout/style/UserAgentStyleSheetList.h +++ b/layout/style/UserAgentStyleSheetList.h @@ -28,6 +28,7 @@ STYLE_SHEET(MathML, "resource://gre-resources/mathml.css", true) STYLE_SHEET(MinimalXUL, "chrome://global/content/minimal-xul.css", false) STYLE_SHEET(NoFrames, "resource://gre-resources/noframes.css", true) STYLE_SHEET(NoScript, "resource://gre-resources/noscript.css", true) +STYLE_SHEET(PluginProblem, "resource://gre-resources/pluginproblem.css", true) STYLE_SHEET(Quirk, "resource://gre-resources/quirk.css", false) STYLE_SHEET(Scrollbars, "chrome://global/skin/scrollbars.css", true) STYLE_SHEET(SVG, "resource://gre/res/svg.css", false) diff --git a/layout/style/jar.mn b/layout/style/jar.mn index b90dc0516f2a..7d9c60eff541 100644 --- a/layout/style/jar.mn +++ b/layout/style/jar.mn @@ -10,6 +10,7 @@ toolkit.jar: res/noscript.css (res/noscript.css) res/noframes.css (res/noframes.css) * res/forms.css (res/forms.css) + res/pluginproblem.css (res/pluginproblem.css) res/arrow.gif (res/arrow.gif) res/arrow-left.gif (res/arrow-left.gif) res/arrow-right.gif (res/arrow-right.gif) diff --git a/toolkit/pluginproblem/content/pluginProblemBinding.css b/layout/style/res/pluginproblem.css similarity index 100% rename from toolkit/pluginproblem/content/pluginProblemBinding.css rename to layout/style/res/pluginproblem.css diff --git a/mobile/android/installer/package-manifest.in b/mobile/android/installer/package-manifest.in index 61e841a1e1af..8d43767f72fb 100644 --- a/mobile/android/installer/package-manifest.in +++ b/mobile/android/installer/package-manifest.in @@ -167,7 +167,6 @@ @BINPATH@/components/nsUpdateTimerManager.manifest @BINPATH@/components/nsUpdateTimerManager.js -@BINPATH@/components/pluginGlue.manifest @BINPATH@/components/ProcessSingleton.manifest @BINPATH@/components/MainProcessSingleton.js @BINPATH@/components/ContentProcessSingleton.js diff --git a/toolkit/pluginproblem/jar.mn b/toolkit/pluginproblem/jar.mn index 7a2198e74c2c..a9c1665be2cc 100644 --- a/toolkit/pluginproblem/jar.mn +++ b/toolkit/pluginproblem/jar.mn @@ -6,4 +6,3 @@ toolkit.jar: % content pluginproblem %pluginproblem/ contentaccessible=yes pluginproblem/pluginProblem.xml (content/pluginProblem.xml) pluginproblem/pluginProblemContent.css (content/pluginProblemContent.css) - pluginproblem/pluginProblemBinding.css (content/pluginProblemBinding.css) diff --git a/toolkit/pluginproblem/moz.build b/toolkit/pluginproblem/moz.build index 070967341498..aac3a838c4c2 100644 --- a/toolkit/pluginproblem/moz.build +++ b/toolkit/pluginproblem/moz.build @@ -4,8 +4,4 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. -EXTRA_COMPONENTS += [ - 'pluginGlue.manifest', -] - JAR_MANIFESTS += ['jar.mn'] diff --git a/toolkit/pluginproblem/pluginGlue.manifest b/toolkit/pluginproblem/pluginGlue.manifest deleted file mode 100644 index 96ebc9bd4cf6..000000000000 --- a/toolkit/pluginproblem/pluginGlue.manifest +++ /dev/null @@ -1 +0,0 @@ -category agent-style-sheets pluginGlue-pluginProblem chrome://pluginproblem/content/pluginProblemBinding.css From cd78518c1af7336f3fcc5a611f668ccdfb53ab96 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Wed, 16 Jan 2019 04:23:29 +0000 Subject: [PATCH 14/18] Bug 1520384 - Fix an invalidation bug and improve display list correlation. r=kvark Differential Revision: https://phabricator.services.mozilla.com/D16648 --HG-- extra : moz-landing-system : lando --- gfx/wr/webrender/src/picture.rs | 146 ++++++++++++++++++++----------- gfx/wr/webrender/src/renderer.rs | 2 +- 2 files changed, 97 insertions(+), 51 deletions(-) diff --git a/gfx/wr/webrender/src/picture.rs b/gfx/wr/webrender/src/picture.rs index cfd3322067f8..7a5475d24612 100644 --- a/gfx/wr/webrender/src/picture.rs +++ b/gfx/wr/webrender/src/picture.rs @@ -36,6 +36,7 @@ use scene_builder::DocumentResources; use smallvec::SmallVec; use surface::{SurfaceDescriptor}; use std::{mem, u16}; +use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; use texture_cache::{Eviction, TextureCacheHandle}; use tiling::RenderTargetKind; use util::{ComparableVec, TransformedRectKind, MatrixHelpers, MaxRect}; @@ -78,6 +79,7 @@ impl RetainedTiles { /// Merge items from one retained tiles into another. pub fn merge(&mut self, other: RetainedTiles) { + assert!(self.tiles.is_empty() || other.tiles.is_empty()); self.tiles.extend(other.tiles); self.ref_prims.extend(other.ref_prims); } @@ -110,12 +112,13 @@ const MAX_CACHE_SIZE: f32 = 2048.0; const MAX_SURFACE_SIZE: f32 = 4096.0; -/// The number of primitives to search for, trying to correlate -/// the offset between one display list and another. -const MAX_PRIMS_TO_CORRELATE: usize = 64; -/// The minmum number of primitives we need to correlate in -/// order to consider it a success. -const MIN_PRIMS_TO_CORRELATE: usize = MAX_PRIMS_TO_CORRELATE / 4; +/// The maximum number of primitives to look for in a display +/// list, trying to find unique primitives. +const MAX_PRIMS_TO_SEARCH: usize = 128; + +/// Used to get unique tile IDs, even when the tile cache is +/// destroyed between display lists / scenes. +static NEXT_TILE_ID: AtomicUsize = ATOMIC_USIZE_INIT; /// Information about the state of an opacity binding. #[derive(Debug)] @@ -213,17 +216,23 @@ impl Tile { self.potential_clips.clear(); } - /// Update state related to whether a tile has the same - /// content and is valid to use. - fn update_validity(&mut self, tile_bounding_rect: &WorldRect) { + /// Invalidate a tile based on change in content. This + /// muct be called even if the tile is not currently + /// visible on screen. We might be able to improve this + /// later by changing how ComparableVec is used. + fn update_content_validity(&mut self) { // Check if the contents of the primitives, clips, and // other dependencies are the same. self.is_same_content &= self.descriptor.is_same_content(); + self.is_valid &= self.is_same_content; + } + /// Update state related to whether a tile has a valid rect that + /// covers the required visible part of the tile. + fn update_rect_validity(&mut self, tile_bounding_rect: &WorldRect) { // The tile is only valid if: // - The content is the same *and* // - The valid part of the tile includes the needed part. - self.is_valid &= self.is_same_content; self.is_valid &= self.valid_rect.contains_rect(tile_bounding_rect); // Update count of how many times this tile has had the same content. @@ -362,33 +371,70 @@ pub struct TileCache { /// scroll bars in gecko, when the content overflows under the /// scroll bar). world_bounding_rect: WorldRect, - /// Counter for the next id to assign for a new tile. - next_id: usize, /// List of reference primitive information used for /// correlating the position between display lists. - reference_prims: Vec, + reference_prims: ReferencePrimitiveList, /// The root clip chain for this tile cache. root_clip_chain_id: ClipChainId, } /// Stores information about a primitive in the cache that we will /// try to use to correlate positions between display lists. +#[derive(Clone)] struct ReferencePrimitive { uid: ItemUid, local_pos: LayoutPoint, spatial_node_index: SpatialNodeIndex, + ref_count: usize, +} + +/// A list of primitive with uids that only exist once in a display +/// list. Used to obtain reference points to correlate the offset +/// between two similar display lists. +struct ReferencePrimitiveList { + ref_prims: Vec, +} + +impl ReferencePrimitiveList { + fn new( + prim_instances: &[PrimitiveInstance], + pictures: &[PicturePrimitive], + ) -> Self { + let mut map = FastHashMap::default(); + let mut search_count = 0; + + // Collect a set of primitives that we can + // potentially use for correlation. + collect_ref_prims( + prim_instances, + pictures, + &mut map, + &mut search_count, + ); + + // Select only primitives where the uid is unique + // in the display list, giving the best chance + // of finding correct correlations. + let ref_prims = map.values().filter(|prim| { + prim.ref_count == 1 + }).cloned().collect(); + + ReferencePrimitiveList { + ref_prims, + } + } } /// Collect a sample of primitives from the prim list that can /// be used to correlate positions. -// TODO(gw): Investigate best how to select which primitives to select. fn collect_ref_prims( prim_instances: &[PrimitiveInstance], - ref_prims: &mut Vec, pictures: &[PicturePrimitive], + map: &mut FastHashMap, + search_count: &mut usize, ) { for prim_instance in prim_instances { - if ref_prims.len() >= MAX_PRIMS_TO_CORRELATE { + if *search_count > MAX_PRIMS_TO_SEARCH { return; } @@ -396,16 +442,25 @@ fn collect_ref_prims( PrimitiveInstanceKind::Picture { pic_index, .. } => { collect_ref_prims( &pictures[pic_index.0].prim_list.prim_instances, - ref_prims, pictures, + map, + search_count, ); } _ => { - ref_prims.push(ReferencePrimitive { - uid: prim_instance.uid(), - local_pos: prim_instance.prim_origin, - spatial_node_index: prim_instance.spatial_node_index, + let uid = prim_instance.uid(); + + let entry = map.entry(uid).or_insert_with(|| { + ReferencePrimitive { + uid, + local_pos: prim_instance.prim_origin, + spatial_node_index: prim_instance.spatial_node_index, + ref_count: 0, + } }); + entry.ref_count += 1; + + *search_count = *search_count + 1; } } } @@ -420,10 +475,8 @@ impl TileCache { ) -> Self { // Build the list of reference primitives // for this picture cache. - let mut reference_prims = Vec::with_capacity(MAX_PRIMS_TO_CORRELATE); - collect_ref_prims( + let reference_prims = ReferencePrimitiveList::new( prim_instances, - &mut reference_prims, pictures, ); @@ -444,18 +497,11 @@ impl TileCache { scroll_offset: None, pending_blits: Vec::new(), world_bounding_rect: WorldRect::zero(), - next_id: 0, reference_prims, root_clip_chain_id, } } - fn next_id(&mut self) -> TileId { - let id = TileId(self.next_id); - self.next_id += 1; - id - } - /// Get the tile coordinates for a given rectangle. fn get_tile_coords_for_rect( &self, @@ -512,7 +558,7 @@ impl TileCache { // new display list. let mut new_prim_map = FastHashMap::default(); build_ref_prims( - &self.reference_prims, + &self.reference_prims.ref_prims, &mut new_prim_map, frame_context.clip_scroll_tree, ); @@ -656,7 +702,10 @@ impl TileCache { let mut tile = match old_tiles.remove(&key) { Some(tile) => tile, - None => Tile::new(self.next_id()), + None => { + let next_id = TileId(NEXT_TILE_ID.fetch_add(1, Ordering::Relaxed)); + Tile::new(next_id) + } }; tile.world_rect = WorldRect::new( @@ -1107,18 +1156,21 @@ impl TileCache { tile.is_valid = false; } + // Invalidate the tile based on the content changing. + tile.update_content_validity(); + let visible_rect = match tile.visible_rect { Some(rect) => rect, None => continue, }; - // Check the content of the tile is the same + // Check the valid rect of the primitive is sufficient. let tile_bounding_rect = match visible_rect.intersection(&self.world_bounding_rect) { Some(rect) => rect.translate(&-tile.world_rect.origin.to_vector()), None => continue, }; - tile.update_validity(&tile_bounding_rect); + tile.update_rect_validity(&tile_bounding_rect); // If there are no primitives there is no need to draw or cache it. if tile.descriptor.prims.is_empty() { @@ -1140,13 +1192,13 @@ impl TileCache { ); _scratch.push_debug_string( label_pos, - debug_colors::WHITE, - format!("{:?}", tile.id), + debug_colors::RED, + format!("{:?} {:?}", tile.id, tile.handle), ); label_pos.y += 20.0; _scratch.push_debug_string( label_pos, - debug_colors::WHITE, + debug_colors::RED, format!("same: {} frames", tile.same_frames), ); } @@ -1751,7 +1803,7 @@ impl PicturePrimitive { // Calculate and store positions of the reference // primitives for this tile cache. build_ref_prims( - &tile_cache.reference_prims, + &tile_cache.reference_prims.ref_prims, &mut retained_tiles.ref_prims, clip_scroll_tree, ); @@ -2873,17 +2925,11 @@ fn correlate_prim_maps( map.into_iter() .max_by_key(|&(_, count)| count) .and_then(|(offset, count)| { - // We will assume we can use the calculated offset if: - // (a) We found more than one quarter of the selected - // reference primitives to have the same offset. - // (b) The display lists both had the same number of - // primitives, and we exactly matched. This handles - // edge cases like scenes where there are very - // few primitives, while excluding edge cases like - // dl_mutate that have thousands of primitives with - // the same uid. - if (count >= MIN_PRIMS_TO_CORRELATE) || - (count == old_prims.len() && count == new_prims.len()) { + // We will assume we can use the calculated offset if we + // found more than one quarter of the selected reference + // primitives to have the same offset. + let prims_available = new_prims.len().min(old_prims.len()); + if count >= prims_available / 4 { Some(offset.into()) } else { None diff --git a/gfx/wr/webrender/src/renderer.rs b/gfx/wr/webrender/src/renderer.rs index 9f91d7614472..f140f8b6184c 100644 --- a/gfx/wr/webrender/src/renderer.rs +++ b/gfx/wr/webrender/src/renderer.rs @@ -4251,7 +4251,7 @@ impl Renderer { for item in items { match item { DebugItem::Rect { rect, color } => { - let inner_color = color.scale_alpha(0.2).into(); + let inner_color = color.scale_alpha(0.1).into(); let outer_color = (*color).into(); debug_renderer.add_quad( From 126679ec704fe056a8d2100942dd06f952b7d344 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 15 Jan 2019 13:14:25 -0800 Subject: [PATCH 15/18] Bug 1506449 - Use an Option instead of Item in DataStore. r=gw This does two things: * Ensures that the T gets dropped when the item is removal, which is important for the TextRunKey case where it holds heap memory. * Eliminates the epoch handling while still ensuring that we panic on stale lookups. We also remove the Item usage for local_data, but don't bother with the Option in that case. Differential Revision: https://phabricator.services.mozilla.com/D16629 --- gfx/wr/webrender/src/intern.rs | 127 +++++++++++---------------------- 1 file changed, 40 insertions(+), 87 deletions(-) diff --git a/gfx/wr/webrender/src/intern.rs b/gfx/wr/webrender/src/intern.rs index 6120f235d16f..a5aa0273c170 100644 --- a/gfx/wr/webrender/src/intern.rs +++ b/gfx/wr/webrender/src/intern.rs @@ -2,6 +2,37 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +//! The interning module provides a generic data structure +//! interning container. It is similar in concept to a +//! traditional string interning container, but it is +//! specialized to the WR thread model. +//! +//! There is an Interner structure, that lives in the +//! scene builder thread, and a DataStore structure +//! that lives in the frame builder thread. +//! +//! Hashing, interning and handle creation is done by +//! the interner structure during scene building. +//! +//! Delta changes for the interner are pushed during +//! a transaction to the frame builder. The frame builder +//! is then able to access the content of the interned +//! handles quickly, via array indexing. +//! +//! Epoch tracking ensures that the garbage collection +//! step which the interner uses to remove items is +//! only invoked on items that the frame builder thread +//! is no longer referencing. +//! +//! Items in the data store are stored in a traditional +//! free-list structure, for content access and memory +//! usage efficiency. +//! +//! The epoch is incremented each time a scene is +//! built. The most recently used scene epoch is +//! stored inside each handle. This is then used for +//! cache invalidation. + use api::{LayoutPrimitiveInfo}; use internal_types::FastHashMap; use malloc_size_of::MallocSizeOf; @@ -13,55 +44,14 @@ use std::{mem, ops, u64}; use std::sync::atomic::{AtomicUsize, Ordering}; use util::VecHelper; -/* - - The interning module provides a generic data structure - interning container. It is similar in concept to a - traditional string interning container, but it is - specialized to the WR thread model. - - There is an Interner structure, that lives in the - scene builder thread, and a DataStore structure - that lives in the frame builder thread. - - Hashing, interning and handle creation is done by - the interner structure during scene building. - - Delta changes for the interner are pushed during - a transaction to the frame builder. The frame builder - is then able to access the content of the interned - handles quickly, via array indexing. - - Epoch tracking ensures that the garbage collection - step which the interner uses to remove items is - only invoked on items that the frame builder thread - is no longer referencing. - - Items in the data store are stored in a traditional - free-list structure, for content access and memory - usage efficiency. - - */ - -/// The epoch is incremented each time a scene is -/// built. The most recently used scene epoch is -/// stored inside each item and handle. This is -/// then used for cache invalidation (item) and -/// correctness validation (handle). #[cfg_attr(feature = "capture", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] #[derive(Debug, Copy, Clone, MallocSizeOf, PartialEq)] struct Epoch(u64); -impl Epoch { - pub const INVALID: Self = Epoch(u64::MAX); -} - /// A list of updates to be applied to the data store, /// provided by the interning structure. pub struct UpdateList { - /// The current epoch of the scene builder. - epoch: Epoch, /// The additions and removals to apply. updates: Vec, /// Actual new data to insert. @@ -109,7 +99,6 @@ impl Handle where M: Copy { pub enum UpdateKind { Insert, Remove, - UpdateEpoch, } #[cfg_attr(feature = "capture", derive(Serialize))] @@ -120,16 +109,6 @@ pub struct Update { kind: UpdateKind, } -/// The data item is stored with an epoch, for validating -/// correct access patterns. -#[cfg_attr(feature = "capture", derive(Serialize))] -#[cfg_attr(feature = "replay", derive(Deserialize))] -#[derive(MallocSizeOf)] -struct Item { - epoch: Epoch, - data: T, -} - pub trait InternDebug { fn on_interned(&self, _uid: ItemUid) {} } @@ -140,7 +119,7 @@ pub trait InternDebug { #[cfg_attr(feature = "replay", derive(Deserialize))] #[derive(MallocSizeOf)] pub struct DataStore { - items: Vec>, + items: Vec>, _source: PhantomData, _marker: PhantomData, } @@ -177,16 +156,11 @@ where for update in update_list.updates { match update.kind { UpdateKind::Insert => { - self.items.entry(update.index).set(Item { - data: T::from(data_iter.next().unwrap()), - epoch: update_list.epoch, - }); + self.items.entry(update.index). + set(Some(T::from(data_iter.next().unwrap()))); } UpdateKind::Remove => { - self.items[update.index].epoch = Epoch::INVALID; - } - UpdateKind::UpdateEpoch => { - self.items[update.index].epoch = update_list.epoch; + self.items[update.index] = None; } } } @@ -207,9 +181,7 @@ where { type Output = T; fn index(&self, handle: Handle) -> &T { - let item = &self.items[handle.index as usize]; - assert_eq!(item.epoch, handle.epoch); - &item.data + self.items[handle.index as usize].as_ref().expect("Bad datastore lookup") } } @@ -222,9 +194,7 @@ where M: Copy { fn index_mut(&mut self, handle: Handle) -> &mut T { - let item = &mut self.items[handle.index as usize]; - assert_eq!(item.epoch, handle.epoch); - &mut item.data + self.items[handle.index as usize].as_mut().expect("Bad datastore lookup") } } @@ -254,7 +224,7 @@ where current_epoch: Epoch, /// The information associated with each interned /// item that can be accessed by the interner. - local_data: Vec>, + local_data: Vec, } impl ::std::default::Default for Interner @@ -297,17 +267,6 @@ where // cloning the (sometimes large) key in the common // case, where the data already exists in the interner. if let Some(handle) = self.map.get_mut(data) { - // Update the epoch in the data store. This - // is not strictly needed for correctness, but - // is used to ensure items are only accessed - // via valid handles. - if handle.epoch != self.current_epoch { - self.updates.push(Update { - index: handle.index as usize, - kind: UpdateKind::UpdateEpoch, - }); - self.local_data[handle.index as usize].epoch = self.current_epoch; - } handle.epoch = self.current_epoch; return *handle; } @@ -344,10 +303,7 @@ where // Create the local data for this item that is // being interned. - self.local_data.entry(index).set(Item { - epoch: self.current_epoch, - data: f(), - }); + self.local_data.entry(index).set(f()); handle } @@ -389,7 +345,6 @@ where let updates = UpdateList { updates, data, - epoch: self.current_epoch, }; // Begin the next epoch @@ -408,9 +363,7 @@ where { type Output = D; fn index(&self, handle: Handle) -> &D { - let item = &self.local_data[handle.index as usize]; - assert_eq!(item.epoch, handle.epoch); - &item.data + &self.local_data[handle.index as usize] } } From e2dbdaa2537f751197d5a368d09193937697d758 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 15 Jan 2019 15:16:06 -0800 Subject: [PATCH 16/18] Bug 1506449 - Arc the glyph arrays for text runs. r=gw Differential Revision: https://phabricator.services.mozilla.com/D16630 --- .../webrender/src/display_list_flattener.rs | 3 +- gfx/wr/webrender/src/prim_store/text_run.rs | 19 ++++---- gfx/wr/webrender/src/util.rs | 48 +++++++++++++++++++ 3 files changed, 61 insertions(+), 9 deletions(-) diff --git a/gfx/wr/webrender/src/display_list_flattener.rs b/gfx/wr/webrender/src/display_list_flattener.rs index bb655f750f16..30b3697fe512 100644 --- a/gfx/wr/webrender/src/display_list_flattener.rs +++ b/gfx/wr/webrender/src/display_list_flattener.rs @@ -39,6 +39,7 @@ use scene_builder::{DocumentResources, InternerMut}; use spatial_node::{StickyFrameInfo, ScrollFrameKind, SpatialNodeType}; use std::{f32, mem, usize}; use std::collections::vec_deque::VecDeque; +use std::sync::Arc; use tiling::{CompositeOps}; use util::{MaxRect, VecHelper}; @@ -2363,7 +2364,7 @@ impl<'a> DisplayListFlattener<'a> { .collect(); TextRun { - glyphs, + glyphs: Arc::new(glyphs), font, offset, shadow: false, diff --git a/gfx/wr/webrender/src/prim_store/text_run.rs b/gfx/wr/webrender/src/prim_store/text_run.rs index a8402a562e23..5d3ace8091f6 100644 --- a/gfx/wr/webrender/src/prim_store/text_run.rs +++ b/gfx/wr/webrender/src/prim_store/text_run.rs @@ -18,7 +18,9 @@ use resource_cache::{ResourceCache}; use util::{MatrixHelpers}; use prim_store::PrimitiveInstanceKind; use std::ops; +use std::sync::Arc; use storage; +use util::PrimaryArc; /// A run of glyphs, with associated font information. #[cfg_attr(feature = "capture", derive(Serialize))] @@ -28,7 +30,7 @@ pub struct TextRunKey { pub common: PrimKeyCommonData, pub font: FontInstance, pub offset: VectorKey, - pub glyphs: Vec, + pub glyphs: PrimaryArc>, pub shadow: bool, } @@ -43,7 +45,7 @@ impl TextRunKey { ), font: text_run.font, offset: text_run.offset.into(), - glyphs: text_run.glyphs, + glyphs: PrimaryArc(text_run.glyphs), shadow: text_run.shadow, } } @@ -76,7 +78,8 @@ pub struct TextRunTemplate { pub common: PrimTemplateCommonData, pub font: FontInstance, pub offset: LayoutVector2D, - pub glyphs: Vec, + #[ignore_malloc_size_of = "Measured via PrimaryArc"] + pub glyphs: Arc>, } impl ops::Deref for TextRunTemplate { @@ -99,7 +102,7 @@ impl From for TextRunTemplate { common, font: item.font, offset: item.offset.into(), - glyphs: item.glyphs, + glyphs: item.glyphs.0, } } } @@ -171,7 +174,7 @@ pub type TextRunDataInterner = intern::Interner, + pub glyphs: Arc>, pub shadow: bool, } @@ -336,8 +339,8 @@ fn test_struct_sizes() { // test expectations and move on. // (b) You made a structure larger. This is not necessarily a problem, but should only // be done with care, and after checking if talos performance regresses badly. - assert_eq!(mem::size_of::(), 112, "TextRun size changed"); - assert_eq!(mem::size_of::(), 128, "TextRunTemplate size changed"); - assert_eq!(mem::size_of::(), 120, "TextRunKey size changed"); + assert_eq!(mem::size_of::(), 96, "TextRun size changed"); + assert_eq!(mem::size_of::(), 112, "TextRunTemplate size changed"); + assert_eq!(mem::size_of::(), 104, "TextRunKey size changed"); assert_eq!(mem::size_of::(), 88, "TextRunPrimitive size changed"); } diff --git a/gfx/wr/webrender/src/util.rs b/gfx/wr/webrender/src/util.rs index 9c6dd4e24fae..12b4b09b7ab6 100644 --- a/gfx/wr/webrender/src/util.rs +++ b/gfx/wr/webrender/src/util.rs @@ -6,10 +6,13 @@ use api::{BorderRadius, DeviceIntPoint, DeviceIntRect, DeviceIntSize, DevicePixe use api::{LayoutPixel, DeviceRect, WorldPixel, RasterRect}; use euclid::{Point2D, Rect, Size2D, TypedPoint2D, TypedRect, TypedSize2D, Vector2D}; use euclid::{TypedTransform2D, TypedTransform3D, TypedVector2D, TypedScale}; +use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps}; use num_traits::Zero; use plane_split::{Clipper, Polygon}; use std::{i32, f32, fmt, ptr}; use std::borrow::Cow; +use std::os::raw::c_void; +use std::sync::Arc; // Matches the definition of SK_ScalarNearlyZero in Skia. @@ -977,3 +980,48 @@ impl ComparableVec where T: PartialEq + Clone + fmt::Debug { self.is_same && self.prev_len == self.current_index } } + +/// Arc wrapper to support measurement via MallocSizeOf. +/// +/// Memory reporting for Arcs is tricky because of the risk of double-counting. +/// One way to measure them is to keep a table of pointers that have already been +/// traversed. The other way is to use knowledge of the program structure to +/// identify which Arc instances should be measured and which should be skipped to +/// avoid double-counting. +/// +/// This struct implements the second approach. It identifies the "main" pointer +/// to the Arc-ed resource, and measures the buffer as if it were an owned pointer. +/// The programmer should ensure that there is at most one PrimaryArc for a given +/// underlying ArcInner. +#[cfg_attr(feature = "capture", derive(Serialize))] +#[cfg_attr(feature = "replay", derive(Deserialize))] +#[derive(Clone, Debug, Hash, PartialEq, Eq)] +pub struct PrimaryArc(pub Arc); + +impl ::std::ops::Deref for PrimaryArc { + type Target = Arc; + + #[inline] + fn deref(&self) -> &Arc { + &self.0 + } +} + +impl MallocShallowSizeOf for PrimaryArc { + fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + unsafe { + // This is a bit sketchy, but std::sync::Arc doesn't expose the + // base pointer. + let raw_arc_ptr: *const Arc = &self.0; + let raw_ptr_ptr: *const *const c_void = raw_arc_ptr as _; + let raw_ptr = *raw_ptr_ptr; + (ops.size_of_op)(raw_ptr) + } + } +} + +impl MallocSizeOf for PrimaryArc { + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + self.shallow_size_of(ops) + (**self).size_of(ops) + } +} From 03b8afc312e849159a9e1d24794432352bc85e7f Mon Sep 17 00:00:00 2001 From: Robert Helmer Date: Mon, 14 Jan 2019 23:50:08 +0000 Subject: [PATCH 17/18] Bug 1518728 - update vendored libprio to 1.4 r=glandium Differential Revision: https://phabricator.services.mozilla.com/D16266 --HG-- extra : moz-landing-system : lando --- third_party/prio/README-mozilla | 2 +- third_party/prio/include/mprio.h | 90 +++++++---- third_party/prio/prio/config.c | 13 +- third_party/prio/prio/encrypt.c | 247 ++++++++++++++++++++++++++----- third_party/prio/prio/poly.c | 4 +- third_party/prio/prio/prg.c | 26 ++++ third_party/prio/prio/prg.h | 7 + third_party/prio/prio/server.c | 25 +++- third_party/prio/update.sh | 2 +- 9 files changed, 343 insertions(+), 73 deletions(-) diff --git a/third_party/prio/README-mozilla b/third_party/prio/README-mozilla index 6a122861a87d..287ad0c7206b 100644 --- a/third_party/prio/README-mozilla +++ b/third_party/prio/README-mozilla @@ -1,7 +1,7 @@ This directory contains the Prio source from the upstream repo: https://github.com/mozilla/libprio -Current version: 1.2 [commit 02a81fb652d385d0f4f10989d051317097ab55fb] +Current version: 1.4 [commit a95cfdd5eaf7104582709c54ef23395d24d7f7fd] UPDATING: diff --git a/third_party/prio/include/mprio.h b/third_party/prio/include/mprio.h index a7208dca929b..7a53703e1d93 100644 --- a/third_party/prio/include/mprio.h +++ b/third_party/prio/include/mprio.h @@ -77,6 +77,9 @@ void Prio_clear(); * (2) the modulus we use for modular arithmetic. * The default configuration uses an 87-bit modulus. * + * The value `nFields` must be in the range `0 < nFields <= max`, where `max` + * is the value returned by the function `PrioConfig_maxDataFields()` below. + * * The `batch_id` field specifies which "batch" of aggregate statistics we are * computing. For example, if the aggregate statistics are computed every 24 * hours, the `batch_id` might be set to an encoding of the date. The clients @@ -87,18 +90,23 @@ void Prio_clear(); * caller passes in, so you may free the `batch_id` string as soon as * `PrioConfig_new` returns. */ -PrioConfig PrioConfig_new(int n_fields, PublicKey server_a, PublicKey server_b, - const unsigned char* batch_id, - unsigned int batch_id_len); +PrioConfig PrioConfig_new(int nFields, PublicKey serverA, PublicKey serverB, + const unsigned char* batchId, + unsigned int batchIdLen); void PrioConfig_clear(PrioConfig cfg); int PrioConfig_numDataFields(const_PrioConfig cfg); +/* + * Return the maximum number of data fields that the implementation supports. + */ +int PrioConfig_maxDataFields(void); + /* * Create a PrioConfig object with no encryption keys. This routine is * useful for testing, but PrioClient_encode() will always fail when used with * this config. */ -PrioConfig PrioConfig_newTest(int n_fields); +PrioConfig PrioConfig_newTest(int nFields); /* * We use the PublicKey and PrivateKey objects for public-key encryption. Each @@ -108,34 +116,57 @@ PrioConfig PrioConfig_newTest(int n_fields); SECStatus Keypair_new(PrivateKey* pvtkey, PublicKey* pubkey); /* - * Import a new curve25519 public key from the raw bytes given. The key passed - * in - * as `data` should be of length `CURVE25519_KEY_LEN`. This function allocates - * a new PublicKey object, which the caller must free using `PublicKey_clear`. + * Import a new curve25519 public/private key from the raw bytes given. When + * importing a private key, you must pass in the corresponding public key as + * well. The byte arrays given as input should be of length + * `CURVE25519_KEY_LEN`. + * + * These functions will allocate a new `PublicKey`/`PrivateKey` object, which + * the caller must free using `PublicKey_clear`/`PrivateKey_clear`. */ SECStatus PublicKey_import(PublicKey* pk, const unsigned char* data, unsigned int dataLen); +SECStatus PrivateKey_import(PrivateKey* sk, const unsigned char* privData, + unsigned int privDataLen, + const unsigned char* pubData, + unsigned int pubDataLen); /* - * Import a new curve25519 public key from a hex string that contains only the - * characters 0-9a-fA-F. The hex string passed in as `hex_data` should be of - * length `CURVE25519_KEY_LEN_HEX`. This function allocates a new PublicKey - * object, which the caller must free using `PublicKey_clear`. + * Import a new curve25519 public/private key from a hex string that contains + * only the characters 0-9a-fA-F. + * + * The hex strings passed in must each be of length `CURVE25519_KEY_LEN_HEX`. + * These functions will allocate a new `PublicKey`/`PrivateKey` object, which + * the caller must free using `PublicKey_clear`/`PrivateKey_clear`. */ -SECStatus PublicKey_import_hex(PublicKey* pk, const unsigned char* hex_data, +SECStatus PublicKey_import_hex(PublicKey* pk, const unsigned char* hexData, unsigned int dataLen); +SECStatus PrivateKey_import_hex(PrivateKey* sk, + const unsigned char* privHexData, + unsigned int privDataLen, + const unsigned char* pubHexData, + unsigned int pubDataLen); /* - * Export a curve25519 public key as a raw byte-array. + * Export a curve25519 key as a raw byte-array. + * + * The output buffer `data` must have length exactly `CURVE25519_KEY_LEN`. */ -SECStatus PublicKey_export(const_PublicKey pk, - unsigned char data[CURVE25519_KEY_LEN]); +SECStatus PublicKey_export(const_PublicKey pk, unsigned char* data, + unsigned int dataLen); +SECStatus PrivateKey_export(PrivateKey sk, unsigned char* data, + unsigned int dataLen); /* - * Export a curve25519 public key as a NULL-terminated hex string. + * Export a curve25519 key as a NULL-terminated hex string. + * + * The output buffer `data` must have length exactly `CURVE25519_KEY_LEN_HEX + + * 1`. */ -SECStatus PublicKey_export_hex(const_PublicKey pk, - unsigned char data[CURVE25519_KEY_LEN_HEX + 1]); +SECStatus PublicKey_export_hex(const_PublicKey pk, unsigned char* data, + unsigned int dataLen); +SECStatus PrivateKey_export_hex(PrivateKey sk, unsigned char* data, + unsigned int dataLen); void PublicKey_clear(PublicKey pubkey); void PrivateKey_clear(PrivateKey pvtkey); @@ -152,8 +183,8 @@ void PrivateKey_clear(PrivateKey pvtkey); * `for_server_b` to avoid memory leaks. */ SECStatus PrioClient_encode(const_PrioConfig cfg, const bool* data_in, - unsigned char** for_server_a, unsigned int* aLen, - unsigned char** for_server_b, unsigned int* bLen); + unsigned char** forServerA, unsigned int* aLen, + unsigned char** forServerB, unsigned int* bLen); /* * Generate a new PRG seed using the NSS global randomness source. @@ -167,9 +198,9 @@ SECStatus PrioPRGSeed_randomize(PrioPRGSeed* seed); * Pass in the _same_ secret PRGSeed when initializing the two servers. * The PRGSeed must remain secret to the two servers. */ -PrioServer PrioServer_new(const_PrioConfig cfg, PrioServerId server_idx, - PrivateKey server_priv, - const PrioPRGSeed server_shared_secret); +PrioServer PrioServer_new(const_PrioConfig cfg, PrioServerId serverIdx, + PrivateKey serverPriv, + const PrioPRGSeed serverSharedSecret); void PrioServer_clear(PrioServer s); /* @@ -255,11 +286,14 @@ SECStatus PrioTotalShare_read(PrioTotalShare t, msgpack_unpacker* upk, /* * Read the output data into an array of unsigned longs. You should - * be sure that each data value can fit into a single long and that - * the pointer `output` points to a buffer large enough to store - * one long per data field. + * be sure that each data value can fit into a single `unsigned long` + * and that the pointer `output` points to a buffer large enough to + * store one long per data field. + * + * This function returns failure if some final data value is too + * long to fit in an `unsigned long`. */ -SECStatus PrioTotalShare_final(const_PrioConfig cfg, unsigned long* output, +SECStatus PrioTotalShare_final(const_PrioConfig cfg, unsigned long long* output, const_PrioTotalShare tA, const_PrioTotalShare tB); diff --git a/third_party/prio/prio/config.c b/third_party/prio/prio/config.c index 85930f23c987..e8d774df5fb6 100644 --- a/third_party/prio/prio/config.c +++ b/third_party/prio/prio/config.c @@ -51,6 +51,13 @@ initialize_roots(MPArray arr, const char values[], bool inverted) return SECSuccess; } +int +PrioConfig_maxDataFields(void) +{ + const int n_roots = 1 << Generator2Order; + return (n_roots >> 1) - 1; +} + PrioConfig PrioConfig_new(int n_fields, PublicKey server_a, PublicKey server_b, const unsigned char* batch_id, unsigned int batch_id_len) @@ -71,10 +78,8 @@ PrioConfig_new(int n_fields, PublicKey server_a, PublicKey server_b, cfg->roots = NULL; cfg->rootsInv = NULL; - if (cfg->num_data_fields >= cfg->n_roots) { - rv = SECFailure; - goto cleanup; - } + P_CHECKCB(cfg->n_roots > 1); + P_CHECKCB(cfg->num_data_fields <= PrioConfig_maxDataFields()); P_CHECKA(cfg->batch_id = malloc(batch_id_len)); strncpy((char*)cfg->batch_id, (char*)batch_id, batch_id_len); diff --git a/third_party/prio/prio/encrypt.c b/third_party/prio/prio/encrypt.c index 5804991e9181..4bb567994170 100644 --- a/third_party/prio/prio/encrypt.c +++ b/third_party/prio/prio/encrypt.c @@ -26,7 +26,10 @@ #define PRIO_TAG "PrioPacket" #define AAD_LEN (strlen(PRIO_TAG) + CURVE25519_KEY_LEN + GCM_IV_LEN_BYTES) -// The all-zeros curve25519 public key, as DER-encoded SKPI blob. +// For an example of NSS curve25519 import/export code, see: +// https://searchfox.org/nss/rev/cfd5fcba7efbfe116e2c08848075240ec3a92718/gtests/pk11_gtest/pk11_curve25519_unittest.cc#66 + +// The all-zeros curve25519 public key, as DER-encoded SPKI blob. static const uint8_t curve25519_spki_zeros[] = { 0x30, 0x39, 0x30, 0x14, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01, 0x06, 0x09, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xda, 0x47, 0x0f, 0x01, @@ -35,6 +38,35 @@ static const uint8_t curve25519_spki_zeros[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, }; +// The all-zeros curve25519 private key, as a PKCS#8 blob. +static const uint8_t curve25519_priv_zeros[] = { + 0x30, 0x67, 0x02, 0x01, 0x00, 0x30, 0x14, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, + 0x3d, 0x02, 0x01, 0x06, 0x09, 0x2b, 0x06, 0x01, 0x04, 0x01, 0xda, 0x47, 0x0f, + 0x01, 0x04, 0x4c, 0x30, 0x4a, 0x02, 0x01, 0x01, 0x04, 0x20, + + /* Byte index 36: 32 bytes of curve25519 private key. */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + + /* misc type fields */ + 0xa1, 0x23, 0x03, 0x21, + + /* Byte index 73: 32 bytes of curve25519 public key. */ + 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 +}; + +// Index into `curve25519_priv_zeros` at which the private key begins. +static const size_t curve25519_priv_sk_offset = 36; +// Index into `curve25519_priv_zeros` at which the public key begins. +static const size_t curve25519_priv_pk_offset = 73; + +static SECStatus key_from_hex( + unsigned char key_out[CURVE25519_KEY_LEN], + const unsigned char hex_in[CURVE25519_KEY_LEN_HEX]); + // Note that we do not use isxdigit because it is locale-dependent // See: https://github.com/mozilla/libprio/issues/20 static inline char @@ -106,6 +138,7 @@ PublicKey_import(PublicKey* pk, const unsigned char* data, unsigned int dataLen) const int spki_len = sizeof(curve25519_spki_zeros); P_CHECKA(spki_data = calloc(spki_len, sizeof(uint8_t))); + memcpy(spki_data, curve25519_spki_zeros, spki_len); SECItem spki_item = { siBuffer, spki_data, spki_len }; @@ -131,59 +164,205 @@ cleanup: } SECStatus -PublicKey_import_hex(PublicKey* pk, const unsigned char* hex_data, +PrivateKey_import(PrivateKey* sk, const unsigned char* sk_data, + unsigned int sk_data_len, const unsigned char* pk_data, + unsigned int pk_data_len) +{ + if (sk_data_len != CURVE25519_KEY_LEN || !sk_data) { + return SECFailure; + } + + if (pk_data_len != CURVE25519_KEY_LEN || !pk_data) { + return SECFailure; + } + + SECStatus rv = SECSuccess; + PK11SlotInfo* slot = NULL; + uint8_t* zero_priv_data = NULL; + *sk = NULL; + const int zero_priv_len = sizeof(curve25519_priv_zeros); + + P_CHECKA(slot = PK11_GetInternalSlot()); + + P_CHECKA(zero_priv_data = calloc(zero_priv_len, sizeof(uint8_t))); + SECItem zero_priv_item = { siBuffer, zero_priv_data, zero_priv_len }; + + // Copy the PKCS#8-encoded keypair into writable buffer. + memcpy(zero_priv_data, curve25519_priv_zeros, zero_priv_len); + // Copy private key into bytes beginning at index `curve25519_priv_sk_offset`. + memcpy(zero_priv_data + curve25519_priv_sk_offset, sk_data, sk_data_len); + // Copy private key into bytes beginning at index `curve25519_priv_pk_offset`. + memcpy(zero_priv_data + curve25519_priv_pk_offset, pk_data, pk_data_len); + + P_CHECKC(PK11_ImportDERPrivateKeyInfoAndReturnKey( + slot, &zero_priv_item, NULL, NULL, PR_FALSE, PR_FALSE, KU_ALL, sk, NULL)); + +cleanup: + if (slot) { + PK11_FreeSlot(slot); + } + if (zero_priv_data) { + free(zero_priv_data); + } + if (rv != SECSuccess) { + PrivateKey_clear(*sk); + } + return rv; +} + +SECStatus +PublicKey_import_hex(PublicKey* pk, const unsigned char* hexData, unsigned int dataLen) { unsigned char raw_bytes[CURVE25519_KEY_LEN]; - if (dataLen != CURVE25519_KEY_LEN_HEX) + if (dataLen != CURVE25519_KEY_LEN_HEX || !hexData) { return SECFailure; - - for (unsigned int i = 0; i < dataLen; i++) { - if (!is_hex_digit(hex_data[i])) - return SECFailure; } - const unsigned char* p = hex_data; - for (unsigned int i = 0; i < CURVE25519_KEY_LEN; i++) { - uint8_t d0 = hex_to_int(p[0]); - uint8_t d1 = hex_to_int(p[1]); - raw_bytes[i] = (d0 << 4) | d1; - p += 2; + if (key_from_hex(raw_bytes, hexData) != SECSuccess) { + return SECFailure; } return PublicKey_import(pk, raw_bytes, CURVE25519_KEY_LEN); } SECStatus -PublicKey_export(const_PublicKey pk, unsigned char data[CURVE25519_KEY_LEN]) +PrivateKey_import_hex(PrivateKey* sk, const unsigned char* privHexData, + unsigned int privDataLen, const unsigned char* pubHexData, + unsigned int pubDataLen) { - if (pk == NULL) - return SECFailure; + SECStatus rv = SECSuccess; + unsigned char raw_priv[CURVE25519_KEY_LEN]; + unsigned char raw_pub[CURVE25519_KEY_LEN]; - memcpy(data, pk->u.ec.publicValue.data, CURVE25519_KEY_LEN); + if (privDataLen != CURVE25519_KEY_LEN_HEX || + pubDataLen != CURVE25519_KEY_LEN_HEX) { + return SECFailure; + } + + if (!privHexData || !pubHexData) { + return SECFailure; + } + + P_CHECK(key_from_hex(raw_priv, privHexData)); + P_CHECK(key_from_hex(raw_pub, pubHexData)); + + return PrivateKey_import(sk, raw_priv, CURVE25519_KEY_LEN, raw_pub, + CURVE25519_KEY_LEN); +} + +SECStatus +PublicKey_export(const_PublicKey pk, unsigned char* data, unsigned int dataLen) +{ + if (pk == NULL || dataLen != CURVE25519_KEY_LEN) { + return SECFailure; + } + + const SECItem* key = &pk->u.ec.publicValue; + if (key->len != CURVE25519_KEY_LEN) { + return SECFailure; + } + + memcpy(data, key->data, key->len); + return SECSuccess; +} + +SECStatus +PrivateKey_export(PrivateKey sk, unsigned char* data, unsigned int dataLen) +{ + if (sk == NULL || dataLen != CURVE25519_KEY_LEN) { + return SECFailure; + } + + SECStatus rv = SECSuccess; + SECItem item = { siBuffer, NULL, 0 }; + + P_CHECKC(PK11_ReadRawAttribute(PK11_TypePrivKey, sk, CKA_VALUE, &item)); + + // If the leading bytes of the key are '\0', then this string can be + // shorter than `CURVE25519_KEY_LEN` bytes. + memset(data, 0, CURVE25519_KEY_LEN); + P_CHECKCB(item.len <= CURVE25519_KEY_LEN); + + // Copy into the low-order bytes of the output. + const size_t leading_zeros = CURVE25519_KEY_LEN - item.len; + memcpy(data + leading_zeros, item.data, item.len); + +cleanup: + if (item.data != NULL) { + SECITEM_ZfreeItem(&item, PR_FALSE); + } + + return rv; +} + +static void +key_to_hex(const unsigned char key_in[CURVE25519_KEY_LEN], + unsigned char hex_out[(2 * CURVE25519_KEY_LEN) + 1]) +{ + const unsigned char* p = key_in; + for (unsigned int i = 0; i < CURVE25519_KEY_LEN; i++) { + unsigned char bytel = p[0] & 0x0f; + unsigned char byteu = (p[0] & 0xf0) >> 4; + hex_out[2 * i] = int_to_hex(byteu); + hex_out[2 * i + 1] = int_to_hex(bytel); + p++; + } + + hex_out[2 * CURVE25519_KEY_LEN] = '\0'; +} + +static SECStatus +key_from_hex(unsigned char key_out[CURVE25519_KEY_LEN], + const unsigned char hex_in[CURVE25519_KEY_LEN_HEX]) +{ + for (unsigned int i = 0; i < CURVE25519_KEY_LEN_HEX; i++) { + if (!is_hex_digit(hex_in[i])) + return SECFailure; + } + + const unsigned char* p = hex_in; + for (unsigned int i = 0; i < CURVE25519_KEY_LEN; i++) { + uint8_t d0 = hex_to_int(p[0]); + uint8_t d1 = hex_to_int(p[1]); + key_out[i] = (d0 << 4) | d1; + p += 2; + } return SECSuccess; } SECStatus -PublicKey_export_hex(const_PublicKey pk, - unsigned char data[(2 * CURVE25519_KEY_LEN) + 1]) +PublicKey_export_hex(const_PublicKey pk, unsigned char* data, + unsigned int dataLen) { - unsigned char raw_data[CURVE25519_KEY_LEN]; - if (PublicKey_export(pk, raw_data) != SECSuccess) + if (dataLen != CURVE25519_KEY_LEN_HEX + 1) { return SECFailure; - - const unsigned char* p = raw_data; - for (unsigned int i = 0; i < CURVE25519_KEY_LEN; i++) { - unsigned char bytel = p[0] & 0x0f; - unsigned char byteu = (p[0] & 0xf0) >> 4; - data[2 * i] = int_to_hex(byteu); - data[2 * i + 1] = int_to_hex(bytel); - p++; } - data[2 * CURVE25519_KEY_LEN] = '\0'; + unsigned char raw_data[CURVE25519_KEY_LEN]; + if (PublicKey_export(pk, raw_data, sizeof(raw_data)) != SECSuccess) { + return SECFailure; + } + + key_to_hex(raw_data, data); + return SECSuccess; +} + +SECStatus +PrivateKey_export_hex(PrivateKey sk, unsigned char* data, unsigned int dataLen) +{ + if (dataLen != CURVE25519_KEY_LEN_HEX + 1) { + return SECFailure; + } + + unsigned char raw_data[CURVE25519_KEY_LEN]; + if (PrivateKey_export(sk, raw_data, sizeof(raw_data)) != SECSuccess) { + return SECFailure; + } + + key_to_hex(raw_data, data); return SECSuccess; } @@ -220,11 +399,13 @@ Keypair_new(PrivateKey* pvtkey, PublicKey* pubkey) P_CHECKA(*pvtkey = PK11_GenerateKeyPair(slot, CKM_EC_KEY_PAIR_GEN, &ecp, (SECKEYPublicKey**)pubkey, PR_FALSE, PR_FALSE, NULL)); - PK11_FreeSlot(slot); - cleanup: - if (ecp.data) + if (slot) { + PK11_FreeSlot(slot); + } + if (ecp.data) { free(ecp.data); + } if (rv != SECSuccess) { PublicKey_clear(*pubkey); PrivateKey_clear(*pvtkey); diff --git a/third_party/prio/prio/poly.c b/third_party/prio/prio/poly.c index 7ee57c8f9dbf..972d077af9ba 100644 --- a/third_party/prio/prio/poly.c +++ b/third_party/prio/prio/poly.c @@ -74,8 +74,8 @@ fft_interpolate_raw(mp_int* out, const mp_int* ys, int nPoints, mp_int n_inverse; MP_DIGITS(&n_inverse) = NULL; - MP_CHECK(fft_recurse(out, mod, nPoints, roots, ys, tmp->data, ySub->data, - rootsSub->data)); + MP_CHECKC(fft_recurse(out, mod, nPoints, roots, ys, tmp->data, ySub->data, + rootsSub->data)); if (invert) { MP_CHECKC(mp_init(&n_inverse)); diff --git a/third_party/prio/prio/prg.c b/third_party/prio/prio/prg.c index 03845f742cca..78851fd65b8a 100644 --- a/third_party/prio/prio/prg.c +++ b/third_party/prio/prio/prg.c @@ -119,6 +119,32 @@ PRG_get_int(PRG prg, mp_int* out, const mp_int* max) return rand_int_rng(out, max, &PRG_get_bytes_internal, (void*)prg); } +SECStatus +PRG_get_int_range(PRG prg, mp_int* out, const mp_int* lower, const mp_int* max) +{ + SECStatus rv; + mp_int width; + MP_DIGITS(&width) = NULL; + MP_CHECKC(mp_init(&width)); + + // Compute + // width = max - lower + MP_CHECKC(mp_sub(max, lower, &width)); + + // Get an integer x in the range [0, width) + P_CHECKC(PRG_get_int(prg, out, &width)); + + // Set + // out = lower + x + // which is in the range [lower, width+lower), + // which is [lower, max). + MP_CHECKC(mp_add(lower, out, out)); + +cleanup: + mp_clear(&width); + return rv; +} + SECStatus PRG_get_array(PRG prg, MPArray dst, const mp_int* mod) { diff --git a/third_party/prio/prio/prg.h b/third_party/prio/prio/prg.h index 5dc6520ceecf..f1a3b30b629e 100644 --- a/third_party/prio/prio/prg.h +++ b/third_party/prio/prio/prg.h @@ -35,6 +35,13 @@ SECStatus PRG_get_bytes(PRG prg, unsigned char* bytes, size_t len); */ SECStatus PRG_get_int(PRG prg, mp_int* out, const mp_int* max); +/* + * Use the PRG output to sample a big integer x in the range + * lower <= x < max. + */ +SECStatus PRG_get_int_range(PRG prg, mp_int* out, const mp_int* lower, + const mp_int* max); + /* * Use secret sharing to split the int src into two shares. * Use PRG to generate the value `shareB`. diff --git a/third_party/prio/prio/server.c b/third_party/prio/prio/server.c index a96e6f2fd064..f2eede18dadf 100644 --- a/third_party/prio/prio/server.c +++ b/third_party/prio/prio/server.c @@ -18,6 +18,13 @@ #include "server.h" #include "util.h" +/* In `PrioTotalShare_final`, we need to be able to store + * an `mp_digit` in an `unsigned long long`. + */ +#if (MP_DIGIT_MAX > ULLONG_MAX) +#error "Unsigned long long is not long enough to hold an MP digit" +#endif + PrioServer PrioServer_new(const_PrioConfig cfg, PrioServerId server_idx, PrivateKey server_priv, const PrioPRGSeed seed) @@ -112,7 +119,7 @@ PrioTotalShare_set_data(PrioTotalShare t, const_PrioServer s) } SECStatus -PrioTotalShare_final(const_PrioConfig cfg, unsigned long* output, +PrioTotalShare_final(const_PrioConfig cfg, unsigned long long* output, const_PrioTotalShare tA, const_PrioTotalShare tB) { if (tA->data_shares->len != cfg->num_data_fields) @@ -132,7 +139,10 @@ PrioTotalShare_final(const_PrioConfig cfg, unsigned long* output, MP_CHECKC(mp_addmod(&tA->data_shares->data[i], &tB->data_shares->data[i], &cfg->modulus, &tmp)); - output[i] = tmp.dp[0]; + if (MP_USED(&tmp) > 1) { + P_CHECKCB(false); + } + output[i] = MP_DIGIT(&tmp, 0); } cleanup: @@ -178,19 +188,25 @@ compute_shares(PrioVerifier v, const_PrioPacketClient p) const int n = v->s->cfg->num_data_fields + 1; const int N = next_power_of_two(n); mp_int eval_at; + mp_int lower; MP_DIGITS(&eval_at) = NULL; + MP_DIGITS(&lower) = NULL; MPArray points_f = NULL; MPArray points_g = NULL; MPArray points_h = NULL; MP_CHECKC(mp_init(&eval_at)); + MP_CHECKC(mp_init(&lower)); P_CHECKA(points_f = MPArray_new(N)); P_CHECKA(points_g = MPArray_new(N)); P_CHECKA(points_h = MPArray_new(2 * N)); - // Use PRG to generate random point - MP_CHECKC(PRG_get_int(v->s->prg, &eval_at, &v->s->cfg->modulus)); + // Use PRG to generate random point. Per Appendix D.2 of full version of + // Prio paper, this value must be in the range + // [n+1, modulus). + mp_set(&lower, n + 1); + P_CHECKC(PRG_get_int_range(v->s->prg, &eval_at, &lower, &v->s->cfg->modulus)); // Reduce value into the field we're using. This // doesn't yield exactly a uniformly random point, @@ -233,6 +249,7 @@ cleanup: MPArray_clear(points_g); MPArray_clear(points_h); mp_clear(&eval_at); + mp_clear(&lower); return rv; } diff --git a/third_party/prio/update.sh b/third_party/prio/update.sh index 50424af830e2..6ee1bf04ef5d 100644 --- a/third_party/prio/update.sh +++ b/third_party/prio/update.sh @@ -5,7 +5,7 @@ MY_TEMP_DIR=`mktemp -d -t libprio_update.XXXXXX` || exit 1 -COMMIT="02a81fb652d385d0f4f10989d051317097ab55fb" +COMMIT="a95cfdd5eaf7104582709c54ef23395d24d7f7fd" git clone -n https://github.com/mozilla/libprio ${MY_TEMP_DIR}/libprio git -C ${MY_TEMP_DIR}/libprio checkout ${COMMIT} From bee8cf15c901b9f4b0c074c9977da4bbebc506e3 Mon Sep 17 00:00:00 2001 From: Robert Helmer Date: Fri, 11 Jan 2019 08:25:42 +0000 Subject: [PATCH 18/18] Bug 1518728 - pass key length to libprio public key export function and use long long for output r=hsivonen Depends on D16266 Differential Revision: https://phabricator.services.mozilla.com/D16267 --HG-- extra : moz-landing-system : lando --- dom/prio/test/gtest/TestPrioEncoder.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/dom/prio/test/gtest/TestPrioEncoder.cpp b/dom/prio/test/gtest/TestPrioEncoder.cpp index afe74d8803d7..b72979b5fbc4 100644 --- a/dom/prio/test/gtest/TestPrioEncoder.cpp +++ b/dom/prio/test/gtest/TestPrioEncoder.cpp @@ -113,7 +113,7 @@ TEST(PrioEncoder, VerifyFull) { snprintf((char*)batchIDStr, sizeof batchIDStr, "%d", rand()); bool dataItems[ndata]; - unsigned long output[ndata]; + unsigned long long output[ndata]; // The client's data submission is an arbitrary boolean vector. for (int i = 0; i < ndata; i++) { @@ -133,12 +133,13 @@ TEST(PrioEncoder, VerifyFull) { ASSERT_TRUE(prioRv == SECSuccess); // Export public keys to hex and print to stdout - unsigned char pkHexA[CURVE25519_KEY_LEN_HEX + 1]; - unsigned char pkHexB[CURVE25519_KEY_LEN_HEX + 1]; - prioRv = PublicKey_export_hex(pkA, pkHexA); + const int keyLength = CURVE25519_KEY_LEN_HEX + 1; + unsigned char pkHexA[keyLength]; + unsigned char pkHexB[keyLength]; + prioRv = PublicKey_export_hex(pkA, pkHexA, keyLength); ASSERT_TRUE(prioRv == SECSuccess); - prioRv = PublicKey_export_hex(pkB, pkHexB); + prioRv = PublicKey_export_hex(pkB, pkHexB, keyLength); ASSERT_TRUE(prioRv == SECSuccess); // Use the default configuration parameters.