From b490bda019cc4dc8c8bdbf5ef45ab6efe49cf677 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Wed, 6 May 2020 10:28:50 -0400 Subject: [PATCH 01/15] Issue #21 - Remove TelemertyVFS This reverts m-c Bug 668378 and completely removes Telemetry SQLite IO. As a bonus this fixes a potential crash in newer SQLite versions without the need for updating this useless telemetry shim. --- storage/TelemetryVFS.cpp | 879 ------------------ storage/moz.build | 1 - storage/mozStorageConnection.cpp | 10 - storage/mozStorageService.cpp | 20 +- storage/mozStorageService.h | 3 - storage/test/unit/test_telemetry_vfs.js | 30 - storage/test/unit/xpcshell.ini | 1 - toolkit/components/telemetry/Histograms.json | 259 ------ .../telemetry/histogram-whitelists.json | 78 -- 9 files changed, 1 insertion(+), 1280 deletions(-) delete mode 100644 storage/TelemetryVFS.cpp delete mode 100644 storage/test/unit/test_telemetry_vfs.js diff --git a/storage/TelemetryVFS.cpp b/storage/TelemetryVFS.cpp deleted file mode 100644 index eb102a0461..0000000000 --- a/storage/TelemetryVFS.cpp +++ /dev/null @@ -1,879 +0,0 @@ -/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- - * vim: sw=2 ts=2 et lcs=trail\:.,tab\:>~ : - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include -#include "mozilla/Telemetry.h" -#include "mozilla/Preferences.h" -#include "sqlite3.h" -#include "nsThreadUtils.h" -#include "mozilla/dom/quota/PersistenceType.h" -#include "mozilla/dom/quota/QuotaManager.h" -#include "mozilla/dom/quota/QuotaObject.h" -#include "mozilla/IOInterposer.h" - -// The last VFS version for which this file has been updated. -#define LAST_KNOWN_VFS_VERSION 3 - -// The last io_methods version for which this file has been updated. -#define LAST_KNOWN_IOMETHODS_VERSION 3 - -/** - * This preference is a workaround to allow users/sysadmins to identify - * that the profile exists on an NFS share whose implementation - * is incompatible with SQLite's default locking implementation. - * Bug 433129 attempted to automatically identify such file-systems, - * but a reliable way was not found and it was determined that the fallback - * locking is slower than POSIX locking, so we do not want to do it by default. -*/ -#define PREF_NFS_FILESYSTEM "storage.nfs_filesystem" - -namespace { - -using namespace mozilla; -using namespace mozilla::dom::quota; - -struct Histograms { - const char *name; - const Telemetry::ID readB; - const Telemetry::ID writeB; - const Telemetry::ID readMS; - const Telemetry::ID writeMS; - const Telemetry::ID syncMS; -}; - -#define SQLITE_TELEMETRY(FILENAME, HGRAM) \ - { FILENAME, \ - Telemetry::MOZ_SQLITE_ ## HGRAM ## _READ_B, \ - Telemetry::MOZ_SQLITE_ ## HGRAM ## _WRITE_B, \ - Telemetry::MOZ_SQLITE_ ## HGRAM ## _READ_MS, \ - Telemetry::MOZ_SQLITE_ ## HGRAM ## _WRITE_MS, \ - Telemetry::MOZ_SQLITE_ ## HGRAM ## _SYNC_MS \ - } - -Histograms gHistograms[] = { - SQLITE_TELEMETRY("places.sqlite", PLACES), - SQLITE_TELEMETRY("cookies.sqlite", COOKIES), - SQLITE_TELEMETRY("webappsstore.sqlite", WEBAPPS), - SQLITE_TELEMETRY(nullptr, OTHER) -}; -#undef SQLITE_TELEMETRY - -/** RAII class for measuring how long io takes on/off main thread - */ -class IOThreadAutoTimer { -public: - /** - * IOThreadAutoTimer measures time spent in IO. Additionally it - * automatically determines whether IO is happening on the main - * thread and picks an appropriate histogram. - * - * @param id takes a telemetry histogram id. The id+1 must be an - * equivalent histogram for the main thread. Eg, MOZ_SQLITE_OPEN_MS - * is followed by MOZ_SQLITE_OPEN_MAIN_THREAD_MS. - * - * @param aOp optionally takes an IO operation to report through the - * IOInterposer. Filename will be reported as NULL, and reference will be - * either "sqlite-mainthread" or "sqlite-otherthread". - */ - explicit IOThreadAutoTimer(Telemetry::ID aId, - IOInterposeObserver::Operation aOp = IOInterposeObserver::OpNone) - : start(TimeStamp::Now()), - id(aId), - op(aOp) - { - } - - /** - * This constructor is for when we want to report an operation to - * IOInterposer but do not require a telemetry probe. - * - * @param aOp IO Operation to report through the IOInterposer. - */ - explicit IOThreadAutoTimer(IOInterposeObserver::Operation aOp) - : start(TimeStamp::Now()), - id(Telemetry::HistogramCount), - op(aOp) - { - } - - ~IOThreadAutoTimer() - { - // We don't report SQLite I/O on Windows because we have a comprehensive - // mechanism for intercepting I/O on that platform that captures a superset - // of the data captured here. - } - -private: - const TimeStamp start; - const Telemetry::ID id; - IOInterposeObserver::Operation op; -}; - -struct telemetry_file { - // Base class. Must be first - sqlite3_file base; - - // histograms pertaining to this file - Histograms *histograms; - - // quota object for this file - RefPtr quotaObject; - - // The chunk size for this file. See the documentation for - // sqlite3_file_control() and FCNTL_CHUNK_SIZE. - int fileChunkSize; - - // This contains the vfs that actually does work - sqlite3_file pReal[1]; -}; - -const char* -DatabasePathFromWALPath(const char *zWALName) -{ - /** - * Do some sketchy pointer arithmetic to find the parameter key. The WAL - * filename is in the middle of a big allocated block that contains: - * - * - Random Values - * - Main Database Path - * - \0 - * - Multiple URI components consisting of: - * - Key - * - \0 - * - Value - * - \0 - * - \0 - * - Journal Path - * - \0 - * - WAL Path (zWALName) - * - \0 - * - * Because the main database path is preceded by a random value we have to be - * careful when trying to figure out when we should terminate this loop. - */ - MOZ_ASSERT(zWALName); - - nsDependentCSubstring dbPath(zWALName, strlen(zWALName)); - - // Chop off the "-wal" suffix. - NS_NAMED_LITERAL_CSTRING(kWALSuffix, "-wal"); - MOZ_ASSERT(StringEndsWith(dbPath, kWALSuffix)); - - dbPath.Rebind(zWALName, dbPath.Length() - kWALSuffix.Length()); - MOZ_ASSERT(!dbPath.IsEmpty()); - - // We want to scan to the end of the key/value URI pairs. Skip the preceding - // null and go to the last char of the journal path. - const char* cursor = zWALName - 2; - - // Make sure we just skipped a null. - MOZ_ASSERT(!*(cursor + 1)); - - // Walk backwards over the journal path. - while (*cursor) { - cursor--; - } - - // There should be another null here. - cursor--; - MOZ_ASSERT(!*cursor); - - // Back up one more char to the last char of the previous string. It may be - // the database path or it may be a key/value URI pair. - cursor--; - -#ifdef DEBUG - { - // Verify that we just walked over the journal path. Account for the two - // nulls we just skipped. - const char *journalStart = cursor + 3; - - nsDependentCSubstring journalPath(journalStart, - strlen(journalStart)); - - // Chop off the "-journal" suffix. - NS_NAMED_LITERAL_CSTRING(kJournalSuffix, "-journal"); - MOZ_ASSERT(StringEndsWith(journalPath, kJournalSuffix)); - - journalPath.Rebind(journalStart, - journalPath.Length() - kJournalSuffix.Length()); - MOZ_ASSERT(!journalPath.IsEmpty()); - - // Make sure that the database name is a substring of the journal name. - MOZ_ASSERT(journalPath == dbPath); - } -#endif - - // Now we're either at the end of the key/value URI pairs or we're at the - // end of the database path. Carefully walk backwards one character at a - // time to do this safely without running past the beginning of the database - // path. - const char *const dbPathStart = dbPath.BeginReading(); - const char *dbPathCursor = dbPath.EndReading() - 1; - bool isDBPath = true; - - while (true) { - MOZ_ASSERT(*dbPathCursor, "dbPathCursor should never see a null char!"); - - if (isDBPath) { - isDBPath = dbPathStart <= dbPathCursor && - *dbPathCursor == *cursor && - *cursor; - } - - if (!isDBPath) { - // This isn't the database path so it must be a value. Scan past it and - // the key also. - for (size_t stringCount = 0; stringCount < 2; stringCount++) { - // Scan past the string to the preceding null character. - while (*cursor) { - cursor--; - } - - // Back up one more char to the last char of preceding string. - cursor--; - } - - // Reset and start again. - dbPathCursor = dbPath.EndReading() - 1; - isDBPath = true; - - continue; - } - - MOZ_ASSERT(isDBPath); - MOZ_ASSERT(*cursor); - - if (dbPathStart == dbPathCursor) { - // Found the full database path, we're all done. - MOZ_ASSERT(nsDependentCString(cursor) == dbPath); - return cursor; - } - - // Change the cursors and go through the loop again. - cursor--; - dbPathCursor--; - } - - MOZ_CRASH("Should never get here!"); -} - -already_AddRefed -GetQuotaObjectFromNameAndParameters(const char *zName, - const char *zURIParameterKey) -{ - MOZ_ASSERT(zName); - MOZ_ASSERT(zURIParameterKey); - - const char *persistenceType = - sqlite3_uri_parameter(zURIParameterKey, "persistenceType"); - if (!persistenceType) { - return nullptr; - } - - const char *group = sqlite3_uri_parameter(zURIParameterKey, "group"); - if (!group) { - NS_WARNING("SQLite URI had 'persistenceType' but not 'group'?!"); - return nullptr; - } - - const char *origin = sqlite3_uri_parameter(zURIParameterKey, "origin"); - if (!origin) { - NS_WARNING("SQLite URI had 'persistenceType' and 'group' but not " - "'origin'?!"); - return nullptr; - } - - QuotaManager *quotaManager = QuotaManager::Get(); - MOZ_ASSERT(quotaManager); - - return quotaManager->GetQuotaObject( - PersistenceTypeFromText(nsDependentCString(persistenceType)), - nsDependentCString(group), - nsDependentCString(origin), - NS_ConvertUTF8toUTF16(zName)); -} - -void -MaybeEstablishQuotaControl(const char *zName, - telemetry_file *pFile, - int flags) -{ - MOZ_ASSERT(pFile); - MOZ_ASSERT(!pFile->quotaObject); - - if (!(flags & (SQLITE_OPEN_URI | SQLITE_OPEN_WAL))) { - return; - } - - MOZ_ASSERT(zName); - - const char *zURIParameterKey = (flags & SQLITE_OPEN_WAL) ? - DatabasePathFromWALPath(zName) : - zName; - - MOZ_ASSERT(zURIParameterKey); - - pFile->quotaObject = - GetQuotaObjectFromNameAndParameters(zName, zURIParameterKey); -} - -/* -** Close a telemetry_file. -*/ -int -xClose(sqlite3_file *pFile) -{ - telemetry_file *p = (telemetry_file *)pFile; - int rc; - { // Scope for IOThreadAutoTimer - IOThreadAutoTimer ioTimer(IOInterposeObserver::OpClose); - rc = p->pReal->pMethods->xClose(p->pReal); - } - if( rc==SQLITE_OK ){ - delete p->base.pMethods; - p->base.pMethods = nullptr; - p->quotaObject = nullptr; -#ifdef DEBUG - p->fileChunkSize = 0; -#endif - } - return rc; -} - -/* -** Read data from a telemetry_file. -*/ -int -xRead(sqlite3_file *pFile, void *zBuf, int iAmt, sqlite_int64 iOfst) -{ - telemetry_file *p = (telemetry_file *)pFile; - IOThreadAutoTimer ioTimer(p->histograms->readMS, IOInterposeObserver::OpRead); - int rc; - rc = p->pReal->pMethods->xRead(p->pReal, zBuf, iAmt, iOfst); - // sqlite likes to read from empty files, this is normal, ignore it. - return rc; -} - -/* -** Return the current file-size of a telemetry_file. -*/ -int -xFileSize(sqlite3_file *pFile, sqlite_int64 *pSize) -{ - IOThreadAutoTimer ioTimer(IOInterposeObserver::OpStat); - telemetry_file *p = (telemetry_file *)pFile; - int rc; - rc = p->pReal->pMethods->xFileSize(p->pReal, pSize); - return rc; -} - -/* -** Write data to a telemetry_file. -*/ -int -xWrite(sqlite3_file *pFile, const void *zBuf, int iAmt, sqlite_int64 iOfst) -{ - telemetry_file *p = (telemetry_file *)pFile; - IOThreadAutoTimer ioTimer(p->histograms->writeMS, IOInterposeObserver::OpWrite); - int rc; - if (p->quotaObject) { - MOZ_ASSERT(INT64_MAX - iOfst >= iAmt); - if (!p->quotaObject->MaybeUpdateSize(iOfst + iAmt, /* aTruncate */ false)) { - return SQLITE_FULL; - } - } - rc = p->pReal->pMethods->xWrite(p->pReal, zBuf, iAmt, iOfst); - if (p->quotaObject && rc != SQLITE_OK) { - NS_WARNING("xWrite failed on a quota-controlled file, attempting to " - "update its current size..."); - sqlite_int64 currentSize; - if (xFileSize(pFile, ¤tSize) == SQLITE_OK) { - p->quotaObject->MaybeUpdateSize(currentSize, /* aTruncate */ true); - } - } - return rc; -} - -/* -** Truncate a telemetry_file. -*/ -int -xTruncate(sqlite3_file *pFile, sqlite_int64 size) -{ - IOThreadAutoTimer ioTimer(Telemetry::MOZ_SQLITE_TRUNCATE_MS); - telemetry_file *p = (telemetry_file *)pFile; - int rc; - if (p->quotaObject) { - if (p->fileChunkSize > 0) { - // Round up to the smallest multiple of the chunk size that will hold all - // the data. - size = - ((size + p->fileChunkSize - 1) / p->fileChunkSize) * p->fileChunkSize; - } - if (!p->quotaObject->MaybeUpdateSize(size, /* aTruncate */ true)) { - return SQLITE_FULL; - } - } - rc = p->pReal->pMethods->xTruncate(p->pReal, size); - if (p->quotaObject) { - if (rc == SQLITE_OK) { -#ifdef DEBUG - // Make sure xTruncate set the size exactly as we calculated above. - sqlite_int64 newSize; - MOZ_ASSERT(xFileSize(pFile, &newSize) == SQLITE_OK); - MOZ_ASSERT(newSize == size); -#endif - } else { - NS_WARNING("xTruncate failed on a quota-controlled file, attempting to " - "update its current size..."); - if (xFileSize(pFile, &size) == SQLITE_OK) { - p->quotaObject->MaybeUpdateSize(size, /* aTruncate */ true); - } - } - } - return rc; -} - -/* -** Sync a telemetry_file. -*/ -int -xSync(sqlite3_file *pFile, int flags) -{ - telemetry_file *p = (telemetry_file *)pFile; - IOThreadAutoTimer ioTimer(p->histograms->syncMS, IOInterposeObserver::OpFSync); - return p->pReal->pMethods->xSync(p->pReal, flags); -} - -/* -** Lock a telemetry_file. -*/ -int -xLock(sqlite3_file *pFile, int eLock) -{ - telemetry_file *p = (telemetry_file *)pFile; - int rc; - rc = p->pReal->pMethods->xLock(p->pReal, eLock); - return rc; -} - -/* -** Unlock a telemetry_file. -*/ -int -xUnlock(sqlite3_file *pFile, int eLock) -{ - telemetry_file *p = (telemetry_file *)pFile; - int rc; - rc = p->pReal->pMethods->xUnlock(p->pReal, eLock); - return rc; -} - -/* -** Check if another file-handle holds a RESERVED lock on a telemetry_file. -*/ -int -xCheckReservedLock(sqlite3_file *pFile, int *pResOut) -{ - telemetry_file *p = (telemetry_file *)pFile; - int rc = p->pReal->pMethods->xCheckReservedLock(p->pReal, pResOut); - return rc; -} - -/* -** File control method. For custom operations on a telemetry_file. -*/ -int -xFileControl(sqlite3_file *pFile, int op, void *pArg) -{ - telemetry_file *p = (telemetry_file *)pFile; - int rc; - // Hook SQLITE_FCNTL_SIZE_HINT for quota-controlled files and do the necessary - // work before passing to the SQLite VFS. - if (op == SQLITE_FCNTL_SIZE_HINT && p->quotaObject) { - sqlite3_int64 hintSize = *static_cast(pArg); - sqlite3_int64 currentSize; - rc = xFileSize(pFile, ¤tSize); - if (rc != SQLITE_OK) { - return rc; - } - if (hintSize > currentSize) { - rc = xTruncate(pFile, hintSize); - if (rc != SQLITE_OK) { - return rc; - } - } - } - rc = p->pReal->pMethods->xFileControl(p->pReal, op, pArg); - // Grab the file chunk size after the SQLite VFS has approved. - if (op == SQLITE_FCNTL_CHUNK_SIZE && rc == SQLITE_OK) { - p->fileChunkSize = *static_cast(pArg); - } -#ifdef DEBUG - if (op == SQLITE_FCNTL_SIZE_HINT && p->quotaObject && rc == SQLITE_OK) { - sqlite3_int64 hintSize = *static_cast(pArg); - if (p->fileChunkSize > 0) { - hintSize = - ((hintSize + p->fileChunkSize - 1) / p->fileChunkSize) * - p->fileChunkSize; - } - sqlite3_int64 currentSize; - MOZ_ASSERT(xFileSize(pFile, ¤tSize) == SQLITE_OK); - MOZ_ASSERT(currentSize >= hintSize); - } -#endif - return rc; -} - -/* -** Return the sector-size in bytes for a telemetry_file. -*/ -int -xSectorSize(sqlite3_file *pFile) -{ - telemetry_file *p = (telemetry_file *)pFile; - int rc; - rc = p->pReal->pMethods->xSectorSize(p->pReal); - return rc; -} - -/* -** Return the device characteristic flags supported by a telemetry_file. -*/ -int -xDeviceCharacteristics(sqlite3_file *pFile) -{ - telemetry_file *p = (telemetry_file *)pFile; - int rc; - rc = p->pReal->pMethods->xDeviceCharacteristics(p->pReal); - return rc; -} - -/* -** Shared-memory operations. -*/ -int -xShmLock(sqlite3_file *pFile, int ofst, int n, int flags) -{ - telemetry_file *p = (telemetry_file *)pFile; - return p->pReal->pMethods->xShmLock(p->pReal, ofst, n, flags); -} - -int -xShmMap(sqlite3_file *pFile, int iRegion, int szRegion, int isWrite, void volatile **pp) -{ - telemetry_file *p = (telemetry_file *)pFile; - int rc; - rc = p->pReal->pMethods->xShmMap(p->pReal, iRegion, szRegion, isWrite, pp); - return rc; -} - -void -xShmBarrier(sqlite3_file *pFile){ - telemetry_file *p = (telemetry_file *)pFile; - p->pReal->pMethods->xShmBarrier(p->pReal); -} - -int -xShmUnmap(sqlite3_file *pFile, int delFlag){ - telemetry_file *p = (telemetry_file *)pFile; - int rc; - rc = p->pReal->pMethods->xShmUnmap(p->pReal, delFlag); - return rc; -} - -int -xFetch(sqlite3_file *pFile, sqlite3_int64 iOff, int iAmt, void **pp) -{ - telemetry_file *p = (telemetry_file *)pFile; - MOZ_ASSERT(p->pReal->pMethods->iVersion >= 3); - return p->pReal->pMethods->xFetch(p->pReal, iOff, iAmt, pp); -} - -int -xUnfetch(sqlite3_file *pFile, sqlite3_int64 iOff, void *pResOut) -{ - telemetry_file *p = (telemetry_file *)pFile; - MOZ_ASSERT(p->pReal->pMethods->iVersion >= 3); - return p->pReal->pMethods->xUnfetch(p->pReal, iOff, pResOut); -} - -int -xOpen(sqlite3_vfs* vfs, const char *zName, sqlite3_file* pFile, - int flags, int *pOutFlags) -{ - IOThreadAutoTimer ioTimer(Telemetry::MOZ_SQLITE_OPEN_MS, - IOInterposeObserver::OpCreateOrOpen); - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - int rc; - telemetry_file *p = (telemetry_file *)pFile; - Histograms *h = nullptr; - // check if the filename is one we are probing for - for(size_t i = 0;i < sizeof(gHistograms)/sizeof(gHistograms[0]);i++) { - h = &gHistograms[i]; - // last probe is the fallback probe - if (!h->name) - break; - if (!zName) - continue; - const char *match = strstr(zName, h->name); - if (!match) - continue; - char c = match[strlen(h->name)]; - // include -wal/-journal too - if (!c || c == '-') - break; - } - p->histograms = h; - - MaybeEstablishQuotaControl(zName, p, flags); - - rc = orig_vfs->xOpen(orig_vfs, zName, p->pReal, flags, pOutFlags); - if( rc != SQLITE_OK ) - return rc; - if( p->pReal->pMethods ){ - sqlite3_io_methods *pNew = new sqlite3_io_methods; - const sqlite3_io_methods *pSub = p->pReal->pMethods; - memset(pNew, 0, sizeof(*pNew)); - // If the io_methods version is higher than the last known one, you should - // update this VFS adding appropriate IO methods for any methods added in - // the version change. - pNew->iVersion = pSub->iVersion; - MOZ_ASSERT(pNew->iVersion <= LAST_KNOWN_IOMETHODS_VERSION); - pNew->xClose = xClose; - pNew->xRead = xRead; - pNew->xWrite = xWrite; - pNew->xTruncate = xTruncate; - pNew->xSync = xSync; - pNew->xFileSize = xFileSize; - pNew->xLock = xLock; - pNew->xUnlock = xUnlock; - pNew->xCheckReservedLock = xCheckReservedLock; - pNew->xFileControl = xFileControl; - pNew->xSectorSize = xSectorSize; - pNew->xDeviceCharacteristics = xDeviceCharacteristics; - if (pNew->iVersion >= 2) { - // Methods added in version 2. - pNew->xShmMap = pSub->xShmMap ? xShmMap : 0; - pNew->xShmLock = pSub->xShmLock ? xShmLock : 0; - pNew->xShmBarrier = pSub->xShmBarrier ? xShmBarrier : 0; - pNew->xShmUnmap = pSub->xShmUnmap ? xShmUnmap : 0; - } - if (pNew->iVersion >= 3) { - // Methods added in version 3. - // SQLite 3.7.17 calls these methods without checking for nullptr first, - // so we always define them. Verify that we're not going to call - // nullptrs, though. - MOZ_ASSERT(pSub->xFetch); - pNew->xFetch = xFetch; - MOZ_ASSERT(pSub->xUnfetch); - pNew->xUnfetch = xUnfetch; - } - pFile->pMethods = pNew; - } - return rc; -} - -int -xDelete(sqlite3_vfs* vfs, const char *zName, int syncDir) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - int rc; - RefPtr quotaObject; - - if (StringEndsWith(nsDependentCString(zName), NS_LITERAL_CSTRING("-wal"))) { - const char *zURIParameterKey = DatabasePathFromWALPath(zName); - MOZ_ASSERT(zURIParameterKey); - - quotaObject = GetQuotaObjectFromNameAndParameters(zName, zURIParameterKey); - } - - rc = orig_vfs->xDelete(orig_vfs, zName, syncDir); - if (rc == SQLITE_OK && quotaObject) { - MOZ_ALWAYS_TRUE(quotaObject->MaybeUpdateSize(0, /* aTruncate */ true)); - } - - return rc; -} - -int -xAccess(sqlite3_vfs *vfs, const char *zName, int flags, int *pResOut) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xAccess(orig_vfs, zName, flags, pResOut); -} - -int -xFullPathname(sqlite3_vfs *vfs, const char *zName, int nOut, char *zOut) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xFullPathname(orig_vfs, zName, nOut, zOut); -} - -void* -xDlOpen(sqlite3_vfs *vfs, const char *zFilename) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xDlOpen(orig_vfs, zFilename); -} - -void -xDlError(sqlite3_vfs *vfs, int nByte, char *zErrMsg) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - orig_vfs->xDlError(orig_vfs, nByte, zErrMsg); -} - -void -(*xDlSym(sqlite3_vfs *vfs, void *pHdle, const char *zSym))(void){ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xDlSym(orig_vfs, pHdle, zSym); -} - -void -xDlClose(sqlite3_vfs *vfs, void *pHandle) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - orig_vfs->xDlClose(orig_vfs, pHandle); -} - -int -xRandomness(sqlite3_vfs *vfs, int nByte, char *zOut) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xRandomness(orig_vfs, nByte, zOut); -} - -int -xSleep(sqlite3_vfs *vfs, int microseconds) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xSleep(orig_vfs, microseconds); -} - -int -xCurrentTime(sqlite3_vfs *vfs, double *prNow) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xCurrentTime(orig_vfs, prNow); -} - -int -xGetLastError(sqlite3_vfs *vfs, int nBuf, char *zBuf) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xGetLastError(orig_vfs, nBuf, zBuf); -} - -int -xCurrentTimeInt64(sqlite3_vfs *vfs, sqlite3_int64 *piNow) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xCurrentTimeInt64(orig_vfs, piNow); -} - -static -int -xSetSystemCall(sqlite3_vfs *vfs, const char *zName, sqlite3_syscall_ptr pFunc) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xSetSystemCall(orig_vfs, zName, pFunc); -} - -static -sqlite3_syscall_ptr -xGetSystemCall(sqlite3_vfs *vfs, const char *zName) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xGetSystemCall(orig_vfs, zName); -} - -static -const char * -xNextSystemCall(sqlite3_vfs *vfs, const char *zName) -{ - sqlite3_vfs *orig_vfs = static_cast(vfs->pAppData); - return orig_vfs->xNextSystemCall(orig_vfs, zName); -} - -} // namespace - -namespace mozilla { -namespace storage { - -sqlite3_vfs* ConstructTelemetryVFS() -{ -#if defined(XP_WIN) -#define EXPECTED_VFS "win32" -#define EXPECTED_VFS_NFS "win32" -#else -#define EXPECTED_VFS "unix" -#define EXPECTED_VFS_NFS "unix-excl" -#endif - - bool expected_vfs; - sqlite3_vfs *vfs; - if (Preferences::GetBool(PREF_NFS_FILESYSTEM)) { - vfs = sqlite3_vfs_find(EXPECTED_VFS_NFS); - expected_vfs = (vfs != nullptr); - } - else { - vfs = sqlite3_vfs_find(nullptr); - expected_vfs = vfs->zName && !strcmp(vfs->zName, EXPECTED_VFS); - } - if (!expected_vfs) { - return nullptr; - } - - sqlite3_vfs *tvfs = new ::sqlite3_vfs; - memset(tvfs, 0, sizeof(::sqlite3_vfs)); - // If the VFS version is higher than the last known one, you should update - // this VFS adding appropriate methods for any methods added in the version - // change. - tvfs->iVersion = vfs->iVersion; - MOZ_ASSERT(vfs->iVersion <= LAST_KNOWN_VFS_VERSION); - tvfs->szOsFile = sizeof(telemetry_file) - sizeof(sqlite3_file) + vfs->szOsFile; - tvfs->mxPathname = vfs->mxPathname; - tvfs->zName = "telemetry-vfs"; - tvfs->pAppData = vfs; - tvfs->xOpen = xOpen; - tvfs->xDelete = xDelete; - tvfs->xAccess = xAccess; - tvfs->xFullPathname = xFullPathname; - tvfs->xDlOpen = xDlOpen; - tvfs->xDlError = xDlError; - tvfs->xDlSym = xDlSym; - tvfs->xDlClose = xDlClose; - tvfs->xRandomness = xRandomness; - tvfs->xSleep = xSleep; - tvfs->xCurrentTime = xCurrentTime; - tvfs->xGetLastError = xGetLastError; - if (tvfs->iVersion >= 2) { - // Methods added in version 2. - tvfs->xCurrentTimeInt64 = xCurrentTimeInt64; - } - if (tvfs->iVersion >= 3) { - // Methods added in version 3. - tvfs->xSetSystemCall = xSetSystemCall; - tvfs->xGetSystemCall = xGetSystemCall; - tvfs->xNextSystemCall = xNextSystemCall; - } - return tvfs; -} - -already_AddRefed -GetQuotaObjectForFile(sqlite3_file *pFile) -{ - MOZ_ASSERT(pFile); - - telemetry_file *p = (telemetry_file *)pFile; - RefPtr result = p->quotaObject; - return result.forget(); -} - -} // namespace storage -} // namespace mozilla diff --git a/storage/moz.build b/storage/moz.build index 2d30a85eca..f0e754bdcb 100644 --- a/storage/moz.build +++ b/storage/moz.build @@ -71,7 +71,6 @@ UNIFIED_SOURCES += [ 'mozStorageStatementRow.cpp', 'SQLCollations.cpp', 'StorageBaseStatementInternal.cpp', - 'TelemetryVFS.cpp', 'VacuumManager.cpp', ] diff --git a/storage/mozStorageConnection.cpp b/storage/mozStorageConnection.cpp index e6c3571859..40a71c28b5 100644 --- a/storage/mozStorageConnection.cpp +++ b/storage/mozStorageConnection.cpp @@ -1991,10 +1991,6 @@ Connection::EnableModule(const nsACString& aModuleName) return NS_ERROR_FAILURE; } -// Implemented in TelemetryVFS.cpp -already_AddRefed -GetQuotaObjectForFile(sqlite3_file *pFile); - NS_IMETHODIMP Connection::GetQuotaObjects(QuotaObject** aDatabaseQuotaObject, QuotaObject** aJournalQuotaObject) @@ -2015,8 +2011,6 @@ Connection::GetQuotaObjects(QuotaObject** aDatabaseQuotaObject, return convertResultCode(srv); } - RefPtr databaseQuotaObject = GetQuotaObjectForFile(file); - srv = ::sqlite3_file_control(mDBConn, nullptr, SQLITE_FCNTL_JOURNAL_POINTER, @@ -2025,10 +2019,6 @@ Connection::GetQuotaObjects(QuotaObject** aDatabaseQuotaObject, return convertResultCode(srv); } - RefPtr journalQuotaObject = GetQuotaObjectForFile(file); - - databaseQuotaObject.forget(aDatabaseQuotaObject); - journalQuotaObject.forget(aJournalQuotaObject); return NS_OK; } diff --git a/storage/mozStorageService.cpp b/storage/mozStorageService.cpp index 8c6f65232c..56c10a4d04 100644 --- a/storage/mozStorageService.cpp +++ b/storage/mozStorageService.cpp @@ -253,7 +253,6 @@ int32_t Service::sDefaultPageSize = PREF_TS_PAGESIZE_DEFAULT; Service::Service() : mMutex("Service::mMutex") -, mSqliteVFS(nullptr) , mRegistrationMutex("Service::mRegistrationMutex") , mConnections() { @@ -264,13 +263,9 @@ Service::~Service() mozilla::UnregisterWeakMemoryReporter(this); mozilla::UnregisterStorageSQLiteDistinguishedAmount(); - int rc = sqlite3_vfs_unregister(mSqliteVFS); - if (rc != SQLITE_OK) - NS_WARNING("Failed to unregister sqlite vfs wrapper."); - // Shutdown the sqlite3 API. Warn if shutdown did not turn out okay, but // there is nothing actionable we can do in that case. - rc = ::sqlite3_shutdown(); + int rc = ::sqlite3_shutdown(); if (rc != SQLITE_OK) NS_WARNING("sqlite3 did not shutdown cleanly."); @@ -278,8 +273,6 @@ Service::~Service() NS_ASSERTION(shutdownObserved, "Shutdown was not observed!"); gService = nullptr; - delete mSqliteVFS; - mSqliteVFS = nullptr; } void @@ -372,8 +365,6 @@ Service::shutdown() NS_IF_RELEASE(sXPConnect); } -sqlite3_vfs *ConstructTelemetryVFS(); - #ifdef MOZ_STORAGE_MEMORY namespace { @@ -481,15 +472,6 @@ Service::initialize() if (rc != SQLITE_OK) return convertResultCode(rc); - mSqliteVFS = ConstructTelemetryVFS(); - if (mSqliteVFS) { - rc = sqlite3_vfs_register(mSqliteVFS, 1); - if (rc != SQLITE_OK) - return convertResultCode(rc); - } else { - NS_WARNING("Failed to register telemetry VFS"); - } - // Register for xpcom-shutdown so we can cleanup after ourselves. The // observer service can only be used on the main thread. nsCOMPtr os = mozilla::services::GetObserverService(); diff --git a/storage/mozStorageService.h b/storage/mozStorageService.h index effd330b16..d3c1d74d8b 100644 --- a/storage/mozStorageService.h +++ b/storage/mozStorageService.h @@ -19,7 +19,6 @@ class nsIMemoryReporter; class nsIXPConnect; -struct sqlite3_vfs; namespace mozilla { namespace storage { @@ -136,8 +135,6 @@ private: * synchronizing access to mLocaleCollation. */ Mutex mMutex; - - sqlite3_vfs *mSqliteVFS; /** * Protects mConnections. diff --git a/storage/test/unit/test_telemetry_vfs.js b/storage/test/unit/test_telemetry_vfs.js deleted file mode 100644 index 0822fe3e7e..0000000000 --- a/storage/test/unit/test_telemetry_vfs.js +++ /dev/null @@ -1,30 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - * http://creativecommons.org/publicdomain/zero/1.0/ - */ - -// Make sure that there are telemetry entries created by sqlite io - -function run_sql(d, sql) { - var stmt = d.createStatement(sql); - stmt.execute(); - stmt.finalize(); -} - -function new_file(name) -{ - var file = dirSvc.get("ProfD", Ci.nsIFile); - file.append(name); - return file; -} -function run_test() -{ - const Telemetry = Cc["@mozilla.org/base/telemetry;1"].getService(Ci.nsITelemetry); - let read_hgram = Telemetry.getHistogramById("MOZ_SQLITE_OTHER_READ_B"); - let old_sum = read_hgram.snapshot().sum; - const file = new_file("telemetry.sqlite"); - var d = getDatabase(file); - run_sql(d, "CREATE TABLE bloat(data varchar)"); - run_sql(d, "DROP TABLE bloat"); - do_check_true(read_hgram.snapshot().sum > old_sum); -} - diff --git a/storage/test/unit/xpcshell.ini b/storage/test/unit/xpcshell.ini index e93c7d5b98..f9075a595d 100644 --- a/storage/test/unit/xpcshell.ini +++ b/storage/test/unit/xpcshell.ini @@ -41,6 +41,5 @@ fail-if = os == "android" [test_storage_value_array.js] [test_unicode.js] [test_vacuum.js] -[test_telemetry_vfs.js] # Bug 676981: test fails consistently on Android # fail-if = os == "android" diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index f61a67efc6..2d90166a6a 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -3704,265 +3704,6 @@ "n_values": 30, "description": "Flash object instances count on page" }, - "MOZ_SQLITE_OPEN_MS": { - "expires_in_version": "default", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite open() (ms)" - }, - "MOZ_SQLITE_OPEN_MAIN_THREAD_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite open() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_TRUNCATE_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite truncate() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_TRUNCATE_MAIN_THREAD_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite truncate() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_OTHER_READ_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite read() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_OTHER_READ_MAIN_THREAD_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite read() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_PLACES_READ_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite read() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_PLACES_READ_MAIN_THREAD_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite read() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_COOKIES_OPEN_READAHEAD_MS": { - "expires_in_version": "never", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on cookie DB open with readahead (ms)" - }, - "MOZ_SQLITE_COOKIES_READ_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite read() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_COOKIES_READ_MAIN_THREAD_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite read() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_WEBAPPS_READ_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite read() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_WEBAPPS_READ_MAIN_THREAD_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite read() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_OTHER_WRITE_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite write() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_OTHER_WRITE_MAIN_THREAD_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite write() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_PLACES_WRITE_MS": { - "expires_in_version": "default", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite write() (ms)" - }, - "MOZ_SQLITE_PLACES_WRITE_MAIN_THREAD_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite write() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_COOKIES_WRITE_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite write() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_COOKIES_WRITE_MAIN_THREAD_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite write() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_WEBAPPS_WRITE_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite write() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_WEBAPPS_WRITE_MAIN_THREAD_MS": { - "expires_in_version": "40", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite write() (ms) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_OTHER_SYNC_MS": { - "expires_in_version": "never", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite fsync() (ms)" - }, - "MOZ_SQLITE_OTHER_SYNC_MAIN_THREAD_MS": { - "expires_in_version": "never", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite fsync() (ms)" - }, - "MOZ_SQLITE_PLACES_SYNC_MS": { - "expires_in_version": "never", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite fsync() (ms)" - }, - "MOZ_SQLITE_PLACES_SYNC_MAIN_THREAD_MS": { - "expires_in_version": "never", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite fsync() (ms)" - }, - "MOZ_SQLITE_COOKIES_SYNC_MS": { - "expires_in_version": "never", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite fsync() (ms)" - }, - "MOZ_SQLITE_COOKIES_SYNC_MAIN_THREAD_MS": { - "expires_in_version": "never", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite fsync() (ms)" - }, - "MOZ_SQLITE_WEBAPPS_SYNC_MS": { - "expires_in_version": "never", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite fsync() (ms)" - }, - "MOZ_SQLITE_WEBAPPS_SYNC_MAIN_THREAD_MS": { - "expires_in_version": "never", - "kind": "exponential", - "high": 3000, - "n_buckets": 10, - "description": "Time spent on SQLite fsync() (ms)" - }, - "MOZ_SQLITE_OTHER_READ_B": { - "expires_in_version": "default", - "kind": "linear", - "high": 32768, - "n_buckets": 3, - "description": "SQLite read() (bytes)" - }, - "MOZ_SQLITE_PLACES_READ_B": { - "expires_in_version": "40", - "kind": "linear", - "high": 32768, - "n_buckets": 3, - "description": "SQLite read() (bytes) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_COOKIES_READ_B": { - "expires_in_version": "40", - "kind": "linear", - "high": 32768, - "n_buckets": 3, - "description": "SQLite read() (bytes) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_WEBAPPS_READ_B": { - "expires_in_version": "40", - "kind": "linear", - "high": 32768, - "n_buckets": 3, - "description": "SQLite read() (bytes) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_PLACES_WRITE_B": { - "expires_in_version": "40", - "kind": "linear", - "high": 32768, - "n_buckets": 3, - "description": "SQLite write (bytes) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_COOKIES_WRITE_B": { - "expires_in_version": "40", - "kind": "linear", - "high": 32768, - "n_buckets": 3, - "description": "SQLite write (bytes) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_WEBAPPS_WRITE_B": { - "expires_in_version": "40", - "kind": "linear", - "high": 32768, - "n_buckets": 3, - "description": "SQLite write (bytes) *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***" - }, - "MOZ_SQLITE_OTHER_WRITE_B": { - "expires_in_version": "default", - "kind": "linear", - "high": 32768, - "n_buckets": 3, - "description": "SQLite write (bytes)" - }, "MOZ_STORAGE_ASYNC_REQUESTS_MS": { "alert_emails": ["perf-telemetry-alerts@mozilla.com"], "expires_in_version": "40", diff --git a/toolkit/components/telemetry/histogram-whitelists.json b/toolkit/components/telemetry/histogram-whitelists.json index 529d9f4d57..5cd7c31452 100644 --- a/toolkit/components/telemetry/histogram-whitelists.json +++ b/toolkit/components/telemetry/histogram-whitelists.json @@ -392,43 +392,6 @@ "MEDIA_WMF_DECODE_ERROR", "MIXED_CONTENT_PAGE_LOAD", "MIXED_CONTENT_UNBLOCK_COUNTER", - "MOZ_SQLITE_COOKIES_OPEN_READAHEAD_MS", - "MOZ_SQLITE_COOKIES_READ_B", - "MOZ_SQLITE_COOKIES_READ_MAIN_THREAD_MS", - "MOZ_SQLITE_COOKIES_READ_MS", - "MOZ_SQLITE_COOKIES_SYNC_MAIN_THREAD_MS", - "MOZ_SQLITE_COOKIES_SYNC_MS", - "MOZ_SQLITE_COOKIES_WRITE_B", - "MOZ_SQLITE_COOKIES_WRITE_MAIN_THREAD_MS", - "MOZ_SQLITE_COOKIES_WRITE_MS", - "MOZ_SQLITE_OPEN_MAIN_THREAD_MS", - "MOZ_SQLITE_OPEN_MS", - "MOZ_SQLITE_OTHER_READ_B", - "MOZ_SQLITE_OTHER_READ_MAIN_THREAD_MS", - "MOZ_SQLITE_OTHER_READ_MS", - "MOZ_SQLITE_OTHER_SYNC_MAIN_THREAD_MS", - "MOZ_SQLITE_OTHER_SYNC_MS", - "MOZ_SQLITE_OTHER_WRITE_B", - "MOZ_SQLITE_OTHER_WRITE_MAIN_THREAD_MS", - "MOZ_SQLITE_OTHER_WRITE_MS", - "MOZ_SQLITE_PLACES_READ_B", - "MOZ_SQLITE_PLACES_READ_MAIN_THREAD_MS", - "MOZ_SQLITE_PLACES_READ_MS", - "MOZ_SQLITE_PLACES_SYNC_MAIN_THREAD_MS", - "MOZ_SQLITE_PLACES_SYNC_MS", - "MOZ_SQLITE_PLACES_WRITE_B", - "MOZ_SQLITE_PLACES_WRITE_MAIN_THREAD_MS", - "MOZ_SQLITE_PLACES_WRITE_MS", - "MOZ_SQLITE_TRUNCATE_MAIN_THREAD_MS", - "MOZ_SQLITE_TRUNCATE_MS", - "MOZ_SQLITE_WEBAPPS_READ_B", - "MOZ_SQLITE_WEBAPPS_READ_MAIN_THREAD_MS", - "MOZ_SQLITE_WEBAPPS_READ_MS", - "MOZ_SQLITE_WEBAPPS_SYNC_MAIN_THREAD_MS", - "MOZ_SQLITE_WEBAPPS_SYNC_MS", - "MOZ_SQLITE_WEBAPPS_WRITE_B", - "MOZ_SQLITE_WEBAPPS_WRITE_MAIN_THREAD_MS", - "MOZ_SQLITE_WEBAPPS_WRITE_MS", "NETWORK_CACHE_METADATA_FIRST_READ_SIZE", "NETWORK_CACHE_METADATA_FIRST_READ_TIME_MS", "NETWORK_CACHE_METADATA_SECOND_READ_TIME_MS", @@ -1232,43 +1195,6 @@ "MIXED_CONTENT_HSTS", "MIXED_CONTENT_PAGE_LOAD", "MIXED_CONTENT_UNBLOCK_COUNTER", - "MOZ_SQLITE_COOKIES_OPEN_READAHEAD_MS", - "MOZ_SQLITE_COOKIES_READ_B", - "MOZ_SQLITE_COOKIES_READ_MAIN_THREAD_MS", - "MOZ_SQLITE_COOKIES_READ_MS", - "MOZ_SQLITE_COOKIES_SYNC_MAIN_THREAD_MS", - "MOZ_SQLITE_COOKIES_SYNC_MS", - "MOZ_SQLITE_COOKIES_WRITE_B", - "MOZ_SQLITE_COOKIES_WRITE_MAIN_THREAD_MS", - "MOZ_SQLITE_COOKIES_WRITE_MS", - "MOZ_SQLITE_OPEN_MAIN_THREAD_MS", - "MOZ_SQLITE_OPEN_MS", - "MOZ_SQLITE_OTHER_READ_B", - "MOZ_SQLITE_OTHER_READ_MAIN_THREAD_MS", - "MOZ_SQLITE_OTHER_READ_MS", - "MOZ_SQLITE_OTHER_SYNC_MAIN_THREAD_MS", - "MOZ_SQLITE_OTHER_SYNC_MS", - "MOZ_SQLITE_OTHER_WRITE_B", - "MOZ_SQLITE_OTHER_WRITE_MAIN_THREAD_MS", - "MOZ_SQLITE_OTHER_WRITE_MS", - "MOZ_SQLITE_PLACES_READ_B", - "MOZ_SQLITE_PLACES_READ_MAIN_THREAD_MS", - "MOZ_SQLITE_PLACES_READ_MS", - "MOZ_SQLITE_PLACES_SYNC_MAIN_THREAD_MS", - "MOZ_SQLITE_PLACES_SYNC_MS", - "MOZ_SQLITE_PLACES_WRITE_B", - "MOZ_SQLITE_PLACES_WRITE_MAIN_THREAD_MS", - "MOZ_SQLITE_PLACES_WRITE_MS", - "MOZ_SQLITE_TRUNCATE_MAIN_THREAD_MS", - "MOZ_SQLITE_TRUNCATE_MS", - "MOZ_SQLITE_WEBAPPS_READ_B", - "MOZ_SQLITE_WEBAPPS_READ_MAIN_THREAD_MS", - "MOZ_SQLITE_WEBAPPS_READ_MS", - "MOZ_SQLITE_WEBAPPS_SYNC_MAIN_THREAD_MS", - "MOZ_SQLITE_WEBAPPS_SYNC_MS", - "MOZ_SQLITE_WEBAPPS_WRITE_B", - "MOZ_SQLITE_WEBAPPS_WRITE_MAIN_THREAD_MS", - "MOZ_SQLITE_WEBAPPS_WRITE_MS", "MOZ_STORAGE_ASYNC_REQUESTS_MS", "MOZ_STORAGE_ASYNC_REQUESTS_SUCCESS", "NETWORK_CACHE_METADATA_FIRST_READ_SIZE", @@ -1885,7 +1811,6 @@ "WEAVE_CONFIGURED", "NEWTAB_PAGE_ENABLED", "GRADIENT_DURATION", - "MOZ_SQLITE_OPEN_MS", "SHOULD_TRANSLATION_UI_APPEAR", "NEWTAB_PAGE_LIFE_SPAN", "FX_TOTAL_TOP_VISITS", @@ -1902,7 +1827,6 @@ "FX_NEW_WINDOW_MS", "PDF_VIEWER_TIME_TO_VIEW_MS", "SSL_OCSP_MAY_FETCH", - "MOZ_SQLITE_OTHER_READ_B", "CHECK_JAVA_ENABLED", "TRANSLATION_OPPORTUNITIES", "FX_SESSION_RESTORE_CONTENT_COLLECT_DATA_LONGEST_OP_MS", @@ -1928,11 +1852,9 @@ "FX_SESSION_RESTORE_DOM_STORAGE_SIZE_ESTIMATE_CHARS", "DATA_STORAGE_ENTRIES", "TRANSLATED_PAGES_BY_LANGUAGE", - "MOZ_SQLITE_OTHER_WRITE_B", "LOCALDOMSTORAGE_SHUTDOWN_DATABASE_MS", "SSL_CERT_VERIFICATION_ERRORS", "FX_SESSION_RESTORE_NUMBER_OF_WINDOWS_RESTORED", - "MOZ_SQLITE_PLACES_WRITE_MS", "FX_THUMBNAILS_BG_CAPTURE_CANVAS_DRAW_TIME_MS", "FX_SESSION_RESTORE_STARTUP_INIT_SESSION_MS", "FX_SESSION_RESTORE_WRITE_FILE_MS", From 84053aba97a8c26adc604f05c0d20f8581b04b21 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Sun, 10 May 2020 14:37:50 +0200 Subject: [PATCH 02/15] Issue #1589 - Ensure computed length and data is always available --- dom/media/webaudio/AudioBuffer.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dom/media/webaudio/AudioBuffer.cpp b/dom/media/webaudio/AudioBuffer.cpp index e4252dfcf8..39fe8c7353 100644 --- a/dom/media/webaudio/AudioBuffer.cpp +++ b/dom/media/webaudio/AudioBuffer.cpp @@ -254,6 +254,9 @@ void AudioBuffer::CopyFromChannel(const Float32Array& aDestination, uint32_t aChannelNumber, uint32_t aStartInChannel, ErrorResult& aRv) { + JS::AutoCheckCannotGC nogc; + aDestination.ComputeLengthAndData(); + uint32_t length = aDestination.Length(); CheckedInt end = aStartInChannel; end += length; @@ -263,8 +266,6 @@ AudioBuffer::CopyFromChannel(const Float32Array& aDestination, uint32_t aChannel return; } - JS::AutoCheckCannotGC nogc; - aDestination.ComputeLengthAndData(); JSObject* channelArray = mJSChannels[aChannelNumber]; const float* sourceData = nullptr; if (channelArray) { @@ -295,6 +296,9 @@ AudioBuffer::CopyToChannel(JSContext* aJSContext, const Float32Array& aSource, uint32_t aChannelNumber, uint32_t aStartInChannel, ErrorResult& aRv) { + JS::AutoCheckCannotGC nogc; + aSource.ComputeLengthAndData(); + uint32_t length = aSource.Length(); CheckedInt end = aStartInChannel; end += length; @@ -309,7 +313,6 @@ AudioBuffer::CopyToChannel(JSContext* aJSContext, const Float32Array& aSource, return; } - JS::AutoCheckCannotGC nogc; JSObject* channelArray = mJSChannels[aChannelNumber]; if (JS_GetTypedArrayLength(channelArray) != mLength) { // The array's buffer was detached. @@ -317,7 +320,6 @@ AudioBuffer::CopyToChannel(JSContext* aJSContext, const Float32Array& aSource, return; } - aSource.ComputeLengthAndData(); bool isShared = false; float* channelData = JS_GetFloat32ArrayData(channelArray, &isShared, nogc); // The channelData arrays should all have originated in From 7ae0c88c461943c5d997632b67a2bc35d09dd8ec Mon Sep 17 00:00:00 2001 From: athenian200 Date: Sun, 10 May 2020 06:45:59 -0500 Subject: [PATCH 03/15] Issue #1540 - Stop MP3 demuxer from choking on very small files. --- dom/media/mp3/MP3Demuxer.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dom/media/mp3/MP3Demuxer.cpp b/dom/media/mp3/MP3Demuxer.cpp index 5a98cabfed..ccc515afb4 100644 --- a/dom/media/mp3/MP3Demuxer.cpp +++ b/dom/media/mp3/MP3Demuxer.cpp @@ -396,7 +396,10 @@ MP3TrackDemuxer::Duration(int64_t aNumFrames) const { MediaByteRange MP3TrackDemuxer::FindFirstFrame() { - static const int MIN_SUCCESSIVE_FRAMES = 4; + // This check is meant to avoid invalid frames from broken streams, but + // small MP3 files and streams with odd header data can break this. Lowering + // the value to 3 seems to help significantly. + static const int MIN_SUCCESSIVE_FRAMES = 3; MediaByteRange candidateFrame = FindNextFrame(); int numSuccFrames = candidateFrame.Length() > 0; From 85b85bac2833a2cf79eb6b2704d8eed3e6fa80c7 Mon Sep 17 00:00:00 2001 From: win7-7 Date: Fri, 8 May 2020 15:20:01 +0300 Subject: [PATCH 04/15] Issue #1355 - Store the dirty rect on the display list builder rather than passing it as a parameter to BuildDisplayList Also fix build bustage for De-unified layout/xul in nsRootBoxFrame.cpp --- layout/base/nsDisplayList.cpp | 16 +++-- layout/base/nsDisplayList.h | 25 ++++---- layout/base/nsLayoutUtils.cpp | 6 +- layout/base/nsPresShell.cpp | 4 +- layout/forms/nsComboboxControlFrame.cpp | 11 +--- layout/forms/nsComboboxControlFrame.h | 1 - layout/forms/nsFieldSetFrame.cpp | 7 +-- layout/forms/nsFieldSetFrame.h | 1 - layout/forms/nsFileControlFrame.cpp | 3 +- layout/forms/nsFileControlFrame.h | 1 - layout/forms/nsFormControlFrame.h | 1 - layout/forms/nsGfxCheckboxControlFrame.cpp | 3 +- layout/forms/nsGfxCheckboxControlFrame.h | 1 - layout/forms/nsGfxRadioControlFrame.cpp | 3 +- layout/forms/nsGfxRadioControlFrame.h | 1 - layout/forms/nsHTMLButtonControlFrame.cpp | 3 +- layout/forms/nsHTMLButtonControlFrame.h | 1 - layout/forms/nsListControlFrame.cpp | 3 +- layout/forms/nsListControlFrame.h | 1 - layout/forms/nsProgressFrame.cpp | 3 +- layout/forms/nsProgressFrame.h | 1 - layout/forms/nsRangeFrame.cpp | 5 +- layout/forms/nsRangeFrame.h | 1 - layout/forms/nsSelectsAreaFrame.cpp | 8 +-- layout/forms/nsSelectsAreaFrame.h | 2 - layout/forms/nsTextControlFrame.cpp | 3 +- layout/forms/nsTextControlFrame.h | 1 - layout/generic/nsBackdropFrame.cpp | 1 - layout/generic/nsBackdropFrame.h | 1 - layout/generic/nsBlockFrame.cpp | 27 ++++---- layout/generic/nsBlockFrame.h | 1 - layout/generic/nsBulletFrame.cpp | 1 - layout/generic/nsBulletFrame.h | 1 - layout/generic/nsCanvasFrame.cpp | 7 +-- layout/generic/nsCanvasFrame.h | 1 - layout/generic/nsColumnSetFrame.cpp | 3 +- layout/generic/nsColumnSetFrame.h | 1 - layout/generic/nsContainerFrame.cpp | 9 +-- layout/generic/nsContainerFrame.h | 7 +-- layout/generic/nsFirstLetterFrame.cpp | 3 +- layout/generic/nsFirstLetterFrame.h | 1 - layout/generic/nsFlexContainerFrame.cpp | 3 +- layout/generic/nsFlexContainerFrame.h | 1 - layout/generic/nsFrame.cpp | 65 ++++++++++---------- layout/generic/nsFrameSetFrame.cpp | 7 +-- layout/generic/nsFrameSetFrame.h | 1 - layout/generic/nsGfxScrollFrame.cpp | 56 ++++++++++------- layout/generic/nsGfxScrollFrame.h | 8 +-- layout/generic/nsGridContainerFrame.cpp | 6 +- layout/generic/nsGridContainerFrame.h | 1 - layout/generic/nsHTMLCanvasFrame.cpp | 1 - layout/generic/nsHTMLCanvasFrame.h | 1 - layout/generic/nsIFrame.h | 13 +--- layout/generic/nsImageFrame.cpp | 1 - layout/generic/nsImageFrame.h | 1 - layout/generic/nsInlineFrame.cpp | 3 +- layout/generic/nsInlineFrame.h | 1 - layout/generic/nsLeafFrame.h | 1 - layout/generic/nsPageFrame.cpp | 18 ++++-- layout/generic/nsPageFrame.h | 1 - layout/generic/nsPlaceholderFrame.cpp | 1 - layout/generic/nsPlaceholderFrame.h | 1 - layout/generic/nsPluginFrame.cpp | 1 - layout/generic/nsPluginFrame.h | 1 - layout/generic/nsRubyTextFrame.cpp | 3 +- layout/generic/nsRubyTextFrame.h | 1 - layout/generic/nsSimplePageSequenceFrame.cpp | 10 +-- layout/generic/nsSimplePageSequenceFrame.h | 1 - layout/generic/nsSubDocumentFrame.cpp | 32 +++++----- layout/generic/nsSubDocumentFrame.h | 1 - layout/generic/nsTextFrame.cpp | 1 - layout/generic/nsTextFrame.h | 1 - layout/generic/nsVideoFrame.cpp | 18 +++--- layout/generic/nsVideoFrame.h | 1 - layout/generic/nsViewportFrame.cpp | 9 ++- layout/generic/nsViewportFrame.h | 1 - layout/ipc/RenderFrameParent.cpp | 1 - layout/ipc/RenderFrameParent.h | 1 - layout/mathml/nsMathMLContainerFrame.cpp | 4 +- layout/mathml/nsMathMLContainerFrame.h | 1 - layout/mathml/nsMathMLSelectedFrame.cpp | 5 +- layout/mathml/nsMathMLSelectedFrame.h | 1 - layout/mathml/nsMathMLmencloseFrame.cpp | 3 +- layout/mathml/nsMathMLmencloseFrame.h | 1 - layout/mathml/nsMathMLmfencedFrame.cpp | 3 +- layout/mathml/nsMathMLmfencedFrame.h | 1 - layout/mathml/nsMathMLmfracFrame.cpp | 3 +- layout/mathml/nsMathMLmfracFrame.h | 1 - layout/mathml/nsMathMLmoFrame.cpp | 3 +- layout/mathml/nsMathMLmoFrame.h | 1 - layout/mathml/nsMathMLmrootFrame.cpp | 3 +- layout/mathml/nsMathMLmrootFrame.h | 1 - layout/svg/SVGFEUnstyledLeafFrame.cpp | 1 - layout/svg/SVGTextFrame.cpp | 1 - layout/svg/SVGTextFrame.h | 1 - layout/svg/nsSVGClipPathFrame.h | 1 - layout/svg/nsSVGContainerFrame.cpp | 3 +- layout/svg/nsSVGContainerFrame.h | 2 - layout/svg/nsSVGFilterFrame.h | 1 - layout/svg/nsSVGForeignObjectFrame.cpp | 3 +- layout/svg/nsSVGForeignObjectFrame.h | 1 - layout/svg/nsSVGMarkerFrame.h | 1 - layout/svg/nsSVGMaskFrame.h | 1 - layout/svg/nsSVGOuterSVGFrame.cpp | 3 +- layout/svg/nsSVGOuterSVGFrame.h | 1 - layout/svg/nsSVGPaintServerFrame.h | 1 - layout/svg/nsSVGPathGeometryFrame.cpp | 1 - layout/svg/nsSVGPathGeometryFrame.h | 1 - layout/svg/nsSVGStopFrame.cpp | 1 - layout/svg/nsSVGSwitchFrame.cpp | 4 +- layout/tables/nsTableCellFrame.cpp | 3 +- layout/tables/nsTableCellFrame.h | 1 - layout/tables/nsTableColFrame.cpp | 3 +- layout/tables/nsTableColFrame.h | 1 - layout/tables/nsTableColGroupFrame.cpp | 3 +- layout/tables/nsTableColGroupFrame.h | 1 - layout/tables/nsTableFrame.cpp | 40 ++++++------ layout/tables/nsTableFrame.h | 6 +- layout/tables/nsTableRowFrame.cpp | 3 +- layout/tables/nsTableRowFrame.h | 1 - layout/tables/nsTableRowGroupFrame.cpp | 13 ++-- layout/tables/nsTableRowGroupFrame.h | 1 - layout/tables/nsTableWrapperFrame.cpp | 11 ++-- layout/tables/nsTableWrapperFrame.h | 2 - layout/xul/nsBoxFrame.cpp | 6 +- layout/xul/nsBoxFrame.h | 2 - layout/xul/nsButtonBoxFrame.cpp | 3 +- layout/xul/nsButtonBoxFrame.h | 1 - layout/xul/nsDeckFrame.cpp | 6 +- layout/xul/nsDeckFrame.h | 2 - layout/xul/nsGroupBoxFrame.cpp | 4 +- layout/xul/nsImageBoxFrame.cpp | 3 +- layout/xul/nsImageBoxFrame.h | 1 - layout/xul/nsLeafBoxFrame.cpp | 1 - layout/xul/nsLeafBoxFrame.h | 1 - layout/xul/nsListItemFrame.cpp | 3 +- layout/xul/nsListItemFrame.h | 1 - layout/xul/nsMenuFrame.cpp | 5 +- layout/xul/nsMenuFrame.h | 1 - layout/xul/nsRootBoxFrame.cpp | 8 +-- layout/xul/nsSliderFrame.cpp | 8 +-- layout/xul/nsSliderFrame.h | 2 - layout/xul/nsSplitterFrame.cpp | 3 +- layout/xul/nsSplitterFrame.h | 1 - layout/xul/nsStackFrame.cpp | 4 +- layout/xul/nsStackFrame.h | 1 - layout/xul/nsTextBoxFrame.cpp | 3 +- layout/xul/nsTextBoxFrame.h | 1 - layout/xul/nsTitleBarFrame.cpp | 3 +- layout/xul/nsTitleBarFrame.h | 1 - layout/xul/tree/nsTreeBodyFrame.cpp | 3 +- layout/xul/tree/nsTreeBodyFrame.h | 1 - layout/xul/tree/nsTreeColFrame.cpp | 5 +- layout/xul/tree/nsTreeColFrame.h | 1 - 154 files changed, 258 insertions(+), 429 deletions(-) diff --git a/layout/base/nsDisplayList.cpp b/layout/base/nsDisplayList.cpp index 8a34d108f1..b08fe4219a 100644 --- a/layout/base/nsDisplayList.cpp +++ b/layout/base/nsDisplayList.cpp @@ -864,10 +864,9 @@ nsDisplayListBuilder::FindAnimatedGeometryRootFor(nsDisplayItem* aItem) void nsDisplayListBuilder::MarkOutOfFlowFrameForDisplay(nsIFrame* aDirtyFrame, - nsIFrame* aFrame, - const nsRect& aDirtyRect) + nsIFrame* aFrame) { - nsRect dirtyRectRelativeToDirtyFrame = aDirtyRect; + nsRect dirtyRectRelativeToDirtyFrame = GetDirtyRect(); if (nsLayoutUtils::IsFixedPosFrameInDisplayPort(aFrame) && IsPaintingToWindow()) { NS_ASSERTION(aDirtyFrame == aFrame->GetParent(), "Dirty frame should be viewport frame"); @@ -882,7 +881,9 @@ void nsDisplayListBuilder::MarkOutOfFlowFrameForDisplay(nsIFrame* aDirtyFrame, dirtyRectRelativeToDirtyFrame.SizeTo(aDirtyFrame->GetSize()); } } - nsRect dirty = dirtyRectRelativeToDirtyFrame - aFrame->GetOffsetTo(aDirtyFrame); + + nsPoint offset = aFrame->GetOffsetTo(aDirtyFrame); + nsRect dirty = dirtyRectRelativeToDirtyFrame - offset; nsRect overflowRect = aFrame->GetVisualOverflowRect(); if (aFrame->IsTransformed() && @@ -1094,8 +1095,7 @@ nsDisplayListBuilder::ResetMarkedFramesForDisplayList() void nsDisplayListBuilder::MarkFramesForDisplayList(nsIFrame* aDirtyFrame, - const nsFrameList& aFrames, - const nsRect& aDirtyRect) { + const nsFrameList& aFrames) { for (nsIFrame* e : aFrames) { // Skip the AccessibleCaret frame when building no caret. if (!IsBuildingCaret()) { @@ -1107,9 +1107,8 @@ nsDisplayListBuilder::MarkFramesForDisplayList(nsIFrame* aDirtyFrame, } } } - mFramesMarkedForDisplay.AppendElement(e); - MarkOutOfFlowFrameForDisplay(aDirtyFrame, e, aDirtyRect); + MarkOutOfFlowFrameForDisplay(aDirtyFrame, e); } } @@ -2626,7 +2625,6 @@ SpecialCutoutRegionCase(nsDisplayListBuilder* aBuilder, return true; } - /*static*/ bool nsDisplayBackgroundImage::AppendBackgroundItemsToTop(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame, diff --git a/layout/base/nsDisplayList.h b/layout/base/nsDisplayList.h index 9cee7b517d..e4fb57e8ad 100644 --- a/layout/base/nsDisplayList.h +++ b/layout/base/nsDisplayList.h @@ -444,6 +444,10 @@ public: * BuildDisplayList on right now). */ const nsRect& GetDirtyRect() { return mDirtyRect; } + + void SetDirtyRect(const nsRect& aDirtyRect) { mDirtyRect = aDirtyRect; } + void IntersectDirtyRect(const nsRect& aDirtyRect) { mDirtyRect.IntersectRect(mDirtyRect, aDirtyRect); } + const nsIFrame* GetCurrentFrame() { return mCurrentFrame; } const nsIFrame* GetCurrentReferenceFrame() { return mCurrentReferenceFrame; } const nsPoint& GetCurrentFrameOffsetToReferenceFrame() { return mCurrentOffsetToReferenceFrame; } @@ -493,11 +497,10 @@ public: /** * Display the caret if needed. */ - void DisplayCaret(nsIFrame* aFrame, const nsRect& aDirtyRect, - nsDisplayList* aList) { + void DisplayCaret(nsIFrame* aFrame, nsDisplayList* aList) { nsIFrame* frame = GetCaretFrame(); if (aFrame == frame) { - frame->DisplayCaret(this, aDirtyRect, aList); + frame->DisplayCaret(this, aList); } } /** @@ -602,8 +605,7 @@ public: * destroyed. */ void MarkFramesForDisplayList(nsIFrame* aDirtyFrame, - const nsFrameList& aFrames, - const nsRect& aDirtyRect); + const nsFrameList& aFrames); /** * Mark all child frames that Preserve3D() as needing display. * Because these frames include transforms set on their parent, dirty rects @@ -700,8 +702,8 @@ public: friend class AutoBuildingDisplayList; class AutoBuildingDisplayList { public: - AutoBuildingDisplayList(nsDisplayListBuilder* aBuilder, - nsIFrame* aForChild, + + AutoBuildingDisplayList(nsDisplayListBuilder* aBuilder, nsIFrame* aForChild, const nsRect& aDirtyRect, bool aIsRoot) : mBuilder(aBuilder), mPrevFrame(aBuilder->mCurrentFrame), @@ -1120,11 +1122,11 @@ public: Preserves3DContext mSavedCtx; }; - const nsRect GetPreserves3DDirtyRect(const nsIFrame *aFrame) const { + const nsRect GetPreserves3DRects() const { return mPreserves3DCtx.mDirtyRect; } - void SetPreserves3DDirtyRect(const nsRect &aDirtyRect) { - mPreserves3DCtx.mDirtyRect = aDirtyRect; + void SavePreserves3DRects() { + mPreserves3DCtx.mDirtyRect = mDirtyRect; } bool IsBuildingInvisibleItems() const { return mBuildingInvisibleItems; } @@ -1133,8 +1135,7 @@ public: } private: - void MarkOutOfFlowFrameForDisplay(nsIFrame* aDirtyFrame, nsIFrame* aFrame, - const nsRect& aDirtyRect); + void MarkOutOfFlowFrameForDisplay(nsIFrame* aDirtyFrame, nsIFrame* aFrame); /** * Returns whether a frame acts as an animated geometry root, optionally diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index 710463a5ff..fdcdcac785 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -3111,7 +3111,8 @@ nsLayoutUtils::GetFramesForArea(nsIFrame* aFrame, const nsRect& aRect, } builder.EnterPresShell(aFrame); - aFrame->BuildDisplayListForStackingContext(&builder, aRect, &list); + builder.SetDirtyRect(aRect); + aFrame->BuildDisplayListForStackingContext(&builder, &list); builder.LeavePresShell(aFrame, nullptr); #ifdef MOZ_DUMP_PAINTING @@ -3460,7 +3461,8 @@ nsLayoutUtils::PaintFrame(nsRenderingContext* aRenderingContext, nsIFrame* aFram PROFILER_LABEL("nsLayoutUtils", "PaintFrame::BuildDisplayList", js::ProfileEntry::Category::GRAPHICS); - aFrame->BuildDisplayListForStackingContext(&builder, dirtyRect, &list); + builder.SetDirtyRect(dirtyRect); + aFrame->BuildDisplayListForStackingContext(&builder, &list); } nsIAtom* frameType = aFrame->GetType(); diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index 8b469185f2..e8670ff3bf 100644 --- a/layout/base/nsPresShell.cpp +++ b/layout/base/nsPresShell.cpp @@ -4868,8 +4868,8 @@ PresShell::CreateRangePaintInfo(nsIDOMRange* aRange, nsIFrame* frame = aNode->AsContent()->GetPrimaryFrame(); // XXX deal with frame being null due to display:contents for (; frame; frame = nsLayoutUtils::GetNextContinuationOrIBSplitSibling(frame)) { - frame->BuildDisplayListForStackingContext(&info->mBuilder, - frame->GetVisualOverflowRect(), &info->mList); + info->mBuilder.SetDirtyRect(frame->GetVisualOverflowRect()); + frame->BuildDisplayListForStackingContext(&info->mBuilder, &info->mList); } }; if (startParent->NodeType() == nsIDOMNode::TEXT_NODE) { diff --git a/layout/forms/nsComboboxControlFrame.cpp b/layout/forms/nsComboboxControlFrame.cpp index 27cfaca555..6a1d9c0a99 100644 --- a/layout/forms/nsComboboxControlFrame.cpp +++ b/layout/forms/nsComboboxControlFrame.cpp @@ -1297,7 +1297,6 @@ public: nsReflowStatus& aStatus) override; virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) override; protected: @@ -1338,11 +1337,10 @@ nsComboboxDisplayFrame::Reflow(nsPresContext* aPresContext, void nsComboboxDisplayFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { nsDisplayListCollection set; - nsBlockFrame::BuildDisplayList(aBuilder, aDirtyRect, set); + nsBlockFrame::BuildDisplayList(aBuilder, set); // remove background items if parent frame is themed if (mComboBox->IsThemed()) { @@ -1543,13 +1541,8 @@ void nsDisplayComboboxFocus::Paint(nsDisplayListBuilder* aBuilder, void nsComboboxControlFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { -#ifdef NOISY - printf("%p paint at (%d, %d, %d, %d)\n", this, - aDirtyRect.x, aDirtyRect.y, aDirtyRect.width, aDirtyRect.height); -#endif if (aBuilder->IsForEventDelivery()) { // Don't allow children to receive events. @@ -1558,7 +1551,7 @@ nsComboboxControlFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, } else { // REVIEW: Our in-flow child frames are inline-level so they will paint in our // content list, so we don't need to mess with layers. - nsBlockFrame::BuildDisplayList(aBuilder, aDirtyRect, aLists); + nsBlockFrame::BuildDisplayList(aBuilder, aLists); } // draw a focus indicator only when focus rings should be drawn diff --git a/layout/forms/nsComboboxControlFrame.h b/layout/forms/nsComboboxControlFrame.h index de713576f3..9f5005f3c3 100644 --- a/layout/forms/nsComboboxControlFrame.h +++ b/layout/forms/nsComboboxControlFrame.h @@ -92,7 +92,6 @@ public: nsEventStatus* aEventStatus) override; virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) override; void PaintFocus(DrawTarget& aDrawTarget, nsPoint aPt); diff --git a/layout/forms/nsFieldSetFrame.cpp b/layout/forms/nsFieldSetFrame.cpp index fc9f0571b7..087f44728a 100644 --- a/layout/forms/nsFieldSetFrame.cpp +++ b/layout/forms/nsFieldSetFrame.cpp @@ -153,7 +153,6 @@ nsDisplayFieldSetBorderBackground::ComputeInvalidationRegion(nsDisplayListBuilde void nsFieldSetFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { // Paint our background and border in a special way. // REVIEW: We don't really need to check frame emptiness here; if it's empty, @@ -180,7 +179,7 @@ nsFieldSetFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, } if (GetPrevInFlow()) { - DisplayOverflowContainers(aBuilder, aDirtyRect, aLists); + DisplayOverflowContainers(aBuilder, aLists); } nsDisplayListCollection contentDisplayItems; @@ -191,13 +190,13 @@ nsFieldSetFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, // legend. However, we want the inner frame's display items to be // after the legend's display items in z-order, so we need to save them // and append them later. - BuildDisplayListForChild(aBuilder, inner, aDirtyRect, contentDisplayItems); + BuildDisplayListForChild(aBuilder, inner, contentDisplayItems); } if (nsIFrame* legend = GetLegend()) { // The legend's background goes on our BlockBorderBackgrounds list because // it's a block child. nsDisplayListSet set(aLists, aLists.BlockBorderBackgrounds()); - BuildDisplayListForChild(aBuilder, legend, aDirtyRect, set); + BuildDisplayListForChild(aBuilder, legend, set); } // Put the inner frame's display items on the master list. Note that this // moves its border/background display items to our BorderBackground() list, diff --git a/layout/forms/nsFieldSetFrame.h b/layout/forms/nsFieldSetFrame.h index 5eb67c3209..7c162515ed 100644 --- a/layout/forms/nsFieldSetFrame.h +++ b/layout/forms/nsFieldSetFrame.h @@ -53,7 +53,6 @@ public: nscoord* aBaseline) const override; virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) override; DrawResult PaintBorder(nsDisplayListBuilder* aBuilder, diff --git a/layout/forms/nsFileControlFrame.cpp b/layout/forms/nsFileControlFrame.cpp index 6593716151..d60e4fb468 100644 --- a/layout/forms/nsFileControlFrame.cpp +++ b/layout/forms/nsFileControlFrame.cpp @@ -485,10 +485,9 @@ nsFileControlFrame::SetFormProperty(nsIAtom* aName, void nsFileControlFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { - BuildDisplayListForInline(aBuilder, aDirtyRect, aLists); + BuildDisplayListForInline(aBuilder, aLists); } #ifdef ACCESSIBILITY diff --git a/layout/forms/nsFileControlFrame.h b/layout/forms/nsFileControlFrame.h index 55c51d426c..4f975af299 100644 --- a/layout/forms/nsFileControlFrame.h +++ b/layout/forms/nsFileControlFrame.h @@ -33,7 +33,6 @@ public: nsIFrame* aPrevInFlow) override; virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) override; NS_DECL_QUERYFRAME diff --git a/layout/forms/nsFormControlFrame.h b/layout/forms/nsFormControlFrame.h index fd3e95d936..41bb1d9e8f 100644 --- a/layout/forms/nsFormControlFrame.h +++ b/layout/forms/nsFormControlFrame.h @@ -40,7 +40,6 @@ public: // nsIFrame replacements virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) override { DO_GLOBAL_REFLOW_COUNT_DSP("nsFormControlFrame"); DisplayBorderBackgroundOutline(aBuilder, aLists); diff --git a/layout/forms/nsGfxCheckboxControlFrame.cpp b/layout/forms/nsGfxCheckboxControlFrame.cpp index 061c92349f..80009eff2c 100644 --- a/layout/forms/nsGfxCheckboxControlFrame.cpp +++ b/layout/forms/nsGfxCheckboxControlFrame.cpp @@ -107,10 +107,9 @@ nsGfxCheckboxControlFrame::AccessibleType() //------------------------------------------------------------ void nsGfxCheckboxControlFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { - nsFormControlFrame::BuildDisplayList(aBuilder, aDirtyRect, aLists); + nsFormControlFrame::BuildDisplayList(aBuilder, aLists); // Get current checked state through content model. if ((!IsChecked() && !IsIndeterminate()) || !IsVisibleForPainting(aBuilder)) diff --git a/layout/forms/nsGfxCheckboxControlFrame.h b/layout/forms/nsGfxCheckboxControlFrame.h index 70b8d8d6a7..9234b50577 100644 --- a/layout/forms/nsGfxCheckboxControlFrame.h +++ b/layout/forms/nsGfxCheckboxControlFrame.h @@ -23,7 +23,6 @@ public: #endif virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) override; #ifdef ACCESSIBILITY diff --git a/layout/forms/nsGfxRadioControlFrame.cpp b/layout/forms/nsGfxRadioControlFrame.cpp index e4a35a998a..9c1ec070b4 100644 --- a/layout/forms/nsGfxRadioControlFrame.cpp +++ b/layout/forms/nsGfxRadioControlFrame.cpp @@ -70,10 +70,9 @@ PaintCheckedRadioButton(nsIFrame* aFrame, void nsGfxRadioControlFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { - nsFormControlFrame::BuildDisplayList(aBuilder, aDirtyRect, aLists); + nsFormControlFrame::BuildDisplayList(aBuilder, aLists); if (!IsVisibleForPainting(aBuilder)) return; diff --git a/layout/forms/nsGfxRadioControlFrame.h b/layout/forms/nsGfxRadioControlFrame.h index f91e6b94cc..dd268dec18 100644 --- a/layout/forms/nsGfxRadioControlFrame.h +++ b/layout/forms/nsGfxRadioControlFrame.h @@ -25,7 +25,6 @@ public: #endif virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) override; }; diff --git a/layout/forms/nsHTMLButtonControlFrame.cpp b/layout/forms/nsHTMLButtonControlFrame.cpp index c6d8e1c4f5..10f24e1e03 100644 --- a/layout/forms/nsHTMLButtonControlFrame.cpp +++ b/layout/forms/nsHTMLButtonControlFrame.cpp @@ -98,7 +98,6 @@ nsHTMLButtonControlFrame::ShouldClipPaintingToBorderBox() void nsHTMLButtonControlFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { // Clip to our border area for event hit testing. @@ -132,7 +131,7 @@ nsHTMLButtonControlFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, clipState.ClipContainingBlockDescendants(rect, hasRadii ? radii : nullptr); } - BuildDisplayListForChild(aBuilder, mFrames.FirstChild(), aDirtyRect, set, + BuildDisplayListForChild(aBuilder, mFrames.FirstChild(), set, DISPLAY_CHILD_FORCE_PSEUDO_STACKING_CONTEXT); // That should put the display items in set.Content() } diff --git a/layout/forms/nsHTMLButtonControlFrame.h b/layout/forms/nsHTMLButtonControlFrame.h index 432afa12c0..8837daf74b 100644 --- a/layout/forms/nsHTMLButtonControlFrame.h +++ b/layout/forms/nsHTMLButtonControlFrame.h @@ -27,7 +27,6 @@ public: NS_DECL_FRAMEARENA_HELPERS virtual void BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) override; virtual nscoord GetMinISize(nsRenderingContext *aRenderingContext) override; diff --git a/layout/forms/nsListControlFrame.cpp b/layout/forms/nsListControlFrame.cpp index cc5f37f9a5..7bd356a22b 100644 --- a/layout/forms/nsListControlFrame.cpp +++ b/layout/forms/nsListControlFrame.cpp @@ -155,7 +155,6 @@ nsListControlFrame::DestroyFrom(nsIFrame* aDestructRoot) void nsListControlFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, - const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { // We allow visibility:hidden diff --git a/image/test/crashtests/delaytest.html b/image/test/crashtests/delaytest.html index 00cd1ebd11..762aec7891 100644 --- a/image/test/crashtests/delaytest.html +++ b/image/test/crashtests/delaytest.html @@ -4,7 +4,8 @@ Delayed image reftest wrapper - + + - + + diff --git a/image/test/reftest/bmp/bmpsuite/b/wrapper.html b/image/test/reftest/bmp/bmpsuite/b/wrapper.html index 47e68959fd..22b74c8fc1 100644 --- a/image/test/reftest/bmp/bmpsuite/b/wrapper.html +++ b/image/test/reftest/bmp/bmpsuite/b/wrapper.html @@ -14,13 +14,13 @@ - + + diff --git a/image/test/reftest/bmp/bmpsuite/q/wrapper.html b/image/test/reftest/bmp/bmpsuite/q/wrapper.html index 47e68959fd..22b74c8fc1 100644 --- a/image/test/reftest/bmp/bmpsuite/q/wrapper.html +++ b/image/test/reftest/bmp/bmpsuite/q/wrapper.html @@ -14,13 +14,13 @@ - + + diff --git a/image/test/reftest/downscaling/downscale-16px.html b/image/test/reftest/downscaling/downscale-16px.html index b34adb93da..06d6db2bf6 100644 --- a/image/test/reftest/downscaling/downscale-16px.html +++ b/image/test/reftest/downscaling/downscale-16px.html @@ -14,13 +14,13 @@ - + + diff --git a/image/test/reftest/downscaling/downscale-8px.html b/image/test/reftest/downscaling/downscale-8px.html index 32cb1b211b..fdc632e53b 100644 --- a/image/test/reftest/downscaling/downscale-8px.html +++ b/image/test/reftest/downscaling/downscale-8px.html @@ -14,7 +14,8 @@ - + + - + + - + + - + + diff --git a/image/test/reftest/pngsuite-background/wrapper.html b/image/test/reftest/pngsuite-background/wrapper.html index 5bbe75e01c..45b5167754 100644 --- a/image/test/reftest/pngsuite-background/wrapper.html +++ b/image/test/reftest/pngsuite-background/wrapper.html @@ -14,7 +14,8 @@ - + + - + + diff --git a/image/test/reftest/pngsuite-transparency/wrapper.html b/image/test/reftest/pngsuite-transparency/wrapper.html index 5bbe75e01c..45b5167754 100644 --- a/image/test/reftest/pngsuite-transparency/wrapper.html +++ b/image/test/reftest/pngsuite-transparency/wrapper.html @@ -14,7 +14,8 @@ - + + + + + + + + + + + From 9e7397ac713198048430eda35821431ef2fe1c79 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 12 May 2020 09:08:51 +0000 Subject: [PATCH 11/15] Bump platform version. We've made some notable changes re: layout and rendering. --- config/milestone.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/milestone.txt b/config/milestone.txt index a99a9ac9f8..8642dc41d7 100644 --- a/config/milestone.txt +++ b/config/milestone.txt @@ -10,4 +10,4 @@ # hardcoded milestones in the tree from these two files. #-------------------------------------------------------- -4.5.9 \ No newline at end of file +4.6.0 \ No newline at end of file From 4ac82173d92758ac51badfff32d6fdb2fd636ac1 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 12 May 2020 18:28:25 +0000 Subject: [PATCH 12/15] Issue #1543 - Follow-up: avoid displaying the Alt text if an image is loading. This prevents the Alt text from briefly being shown before being replaced with the image. --- layout/generic/nsImageFrame.cpp | 26 +++++++++----------------- layout/style/res/html.css | 1 - 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/layout/generic/nsImageFrame.cpp b/layout/generic/nsImageFrame.cpp index aed50c4eaa..b96af66b54 100644 --- a/layout/generic/nsImageFrame.cpp +++ b/layout/generic/nsImageFrame.cpp @@ -430,15 +430,11 @@ nsImageFrame::SourceRectToDest(const nsIntRect& aRect) // that we'll construct image frames for them as needed if their display is // toggled from "none" (though we won't paint them, unless their visibility // is changed too). -#define BAD_STATES (NS_EVENT_STATE_BROKEN | NS_EVENT_STATE_USERDISABLED | \ - NS_EVENT_STATE_LOADING) +#define BAD_STATES (NS_EVENT_STATE_BROKEN | NS_EVENT_STATE_USERDISABLED) -// This is a macro so that we don't evaluate the boolean last arg -// unless we have to; it can be expensive -#define IMAGE_OK(_state, _loadingOK) \ - (!(_state).HasAtLeastOneOfStates(BAD_STATES) || \ - (!(_state).HasAtLeastOneOfStates(NS_EVENT_STATE_BROKEN | NS_EVENT_STATE_USERDISABLED) && \ - (_state).HasState(NS_EVENT_STATE_LOADING) && (_loadingOK))) +static bool ImageOk(EventStates aState) { + return !aState.HasAtLeastOneOfStates(BAD_STATES); +} static bool HasAltText(Element* aElement) { @@ -459,10 +455,8 @@ static bool HasAltText(Element* aElement) nsImageFrame::ShouldCreateImageFrameFor(Element* aElement, nsStyleContext* aStyleContext) { - EventStates state = aElement->State(); - if (IMAGE_OK(state, - HaveSpecifiedSize(aStyleContext->StylePosition()))) { - // Image is fine; do the image frame thing + if (ImageOk(aElement->State())) { + // Image is fine or loading; do the image frame thing return true; } @@ -1016,8 +1010,7 @@ nsImageFrame::Reflow(nsPresContext* aPresContext, } aMetrics.SetOverflowAreasToDesiredBounds(); - EventStates contentState = mContent->AsElement()->State(); - bool imageOK = IMAGE_OK(contentState, true); + bool imageOK = ImageOk(mContent->AsElement()->State()); // Determine if the size is available bool haveSize = false; @@ -1336,7 +1329,7 @@ nsImageFrame::DisplayAltFeedback(nsRenderingContext& aRenderingContext, MOZ_ASSERT(gIconLoad, "How did we succeed in Init then?"); // Whether we draw the broken or loading icon. - bool isLoading = IMAGE_OK(GetContent()->AsElement()->State(), true); + bool isLoading = ImageOk(mContent->AsElement()->State()); // Calculate the inner area nsRect inner = GetInnerArea() + aPt; @@ -1756,8 +1749,7 @@ nsImageFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, getter_AddRefs(currentRequest)); } - EventStates contentState = mContent->AsElement()->State(); - bool imageOK = IMAGE_OK(contentState, true); + bool imageOK = ImageOk(mContent->AsElement()->State()); // XXX(seth): The SizeIsAvailable check here should not be necessary - the // intention is that a non-null mImage means we have a size, but there is diff --git a/layout/style/res/html.css b/layout/style/res/html.css index ea8efbe24d..4f43f3134f 100644 --- a/layout/style/res/html.css +++ b/layout/style/res/html.css @@ -642,7 +642,6 @@ hr[size="1"] { img:-moz-broken::before, input:-moz-broken::before, img:-moz-user-disabled::before, input:-moz-user-disabled::before, -img:-moz-loading::before, input:-moz-loading::before, applet:-moz-empty-except-children-with-localname(param):-moz-broken::before, applet:-moz-empty-except-children-with-localname(param):-moz-user-disabled::before { content: -moz-alt-content !important; From b2a716b2f18c962ac903ccaac7ffffb95d14580e Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 13 May 2020 10:15:24 +0000 Subject: [PATCH 13/15] Issue #457 - Silence some superfluous compiler warnings in cairo --- gfx/cairo/cairo/src/moz.build | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gfx/cairo/cairo/src/moz.build b/gfx/cairo/cairo/src/moz.build index 14b602ac23..c78758a06b 100644 --- a/gfx/cairo/cairo/src/moz.build +++ b/gfx/cairo/cairo/src/moz.build @@ -214,6 +214,11 @@ if CONFIG['MOZ_TREE_FREETYPE']: DEFINES['FT_LCD_FILTER_H'] = '%s/modules/freetype2/include/freetype/ftlcdfil.h' % TOPSRCDIR # Suppress warnings in third-party code. +if CONFIG['_MSC_VER']: + CFLAGS += [ + '-wd4005', + '-wd4146', + ] if CONFIG['GNU_CC'] or CONFIG['CLANG_CL']: CFLAGS += [ '-Wno-enum-compare', From c399e1938f3b99049435411a88e0173d544bf9b6 Mon Sep 17 00:00:00 2001 From: win7-7 Date: Tue, 12 May 2020 19:23:47 +0300 Subject: [PATCH 14/15] Issue #1545 - Fix border-radius on table row groups, rows, column groups, or columns Before issue #146, border-radius on row groups, rows, column groups, or columns don't apply to the background of each cell, yet the border-radius on the cell itself does. After issue #146, the behaviors changed. In this patch, I tried to revert the behaviors of border-radius on table row groups, rows, column groups, or columns back to what happened before issue #146. Also: Don't override GetBorderRadii in nsBCTableCellFrame. --- layout/base/nsCSSRendering.cpp | 11 ++++- .../table-bordercollapse/bug1375518-2.html | 22 ++++++++++ .../table-bordercollapse/bug1375518-3.html | 22 ++++++++++ .../bug1375518-4-ref.html | 44 +++++++++++++++++++ .../table-bordercollapse/bug1375518-4.html | 44 +++++++++++++++++++ .../bug1375518-5-ref.html | 44 +++++++++++++++++++ .../table-bordercollapse/bug1375518-5.html | 44 +++++++++++++++++++ .../table-bordercollapse/bug1375518-ref.html | 17 +++++++ .../table-bordercollapse/bug1375518.html | 24 ++++++++++ .../table-bordercollapse/reftest.list | 5 +++ layout/tables/nsTableCellFrame.cpp | 12 ----- layout/tables/nsTableCellFrame.h | 4 -- 12 files changed, 275 insertions(+), 18 deletions(-) create mode 100644 layout/reftests/table-bordercollapse/bug1375518-2.html create mode 100644 layout/reftests/table-bordercollapse/bug1375518-3.html create mode 100644 layout/reftests/table-bordercollapse/bug1375518-4-ref.html create mode 100644 layout/reftests/table-bordercollapse/bug1375518-4.html create mode 100644 layout/reftests/table-bordercollapse/bug1375518-5-ref.html create mode 100644 layout/reftests/table-bordercollapse/bug1375518-5.html create mode 100644 layout/reftests/table-bordercollapse/bug1375518-ref.html create mode 100644 layout/reftests/table-bordercollapse/bug1375518.html diff --git a/layout/base/nsCSSRendering.cpp b/layout/base/nsCSSRendering.cpp index 119c6c8a2d..9a827546fb 100644 --- a/layout/base/nsCSSRendering.cpp +++ b/layout/base/nsCSSRendering.cpp @@ -1903,8 +1903,15 @@ nsCSSRendering::GetImageLayerClip(const nsStyleImageLayers::Layer& aLayer, nsRect clipBorderArea = ::BoxDecorationRectForBorder(aForFrame, aBorderArea, skipSides, &aBorder); - bool haveRoundedCorners = GetRadii(aForFrame, aBorder, aBorderArea, - clipBorderArea, aClipState->mRadii); + bool haveRoundedCorners = false; + nsIAtom* fType = aForFrame->GetType(); + if (fType != nsGkAtoms::tableColGroupFrame && + fType != nsGkAtoms::tableColFrame && + fType != nsGkAtoms::tableRowFrame && + fType != nsGkAtoms::tableRowGroupFrame) { + haveRoundedCorners = GetRadii(aForFrame, aBorder, aBorderArea, + clipBorderArea, aClipState->mRadii); + } bool isSolidBorder = aWillPaintBorder && IsOpaqueBorder(aBorder); diff --git a/layout/reftests/table-bordercollapse/bug1375518-2.html b/layout/reftests/table-bordercollapse/bug1375518-2.html new file mode 100644 index 0000000000..c367376226 --- /dev/null +++ b/layout/reftests/table-bordercollapse/bug1375518-2.html @@ -0,0 +1,22 @@ + + + +Table border collapse + + + +
+ + \ No newline at end of file diff --git a/layout/reftests/table-bordercollapse/bug1375518-3.html b/layout/reftests/table-bordercollapse/bug1375518-3.html new file mode 100644 index 0000000000..1d188e19fa --- /dev/null +++ b/layout/reftests/table-bordercollapse/bug1375518-3.html @@ -0,0 +1,22 @@ + + + +Separated border model table + + + +
+ + \ No newline at end of file diff --git a/layout/reftests/table-bordercollapse/bug1375518-4-ref.html b/layout/reftests/table-bordercollapse/bug1375518-4-ref.html new file mode 100644 index 0000000000..f9a8f07d7f --- /dev/null +++ b/layout/reftests/table-bordercollapse/bug1375518-4-ref.html @@ -0,0 +1,44 @@ + +border-radius and separated border model tables + + +

border-radius and separated border model tables

+ + + + + + + + +
xxxxxx +
xxxxxx +
xxxxxx +
xxxxxx +
+ + +
xxxxxx +
xxxxxx +
+ + + +
xxxxxx +
xxxxxx +
+ + + +
xxxxxx +
xxxxxx +
\ No newline at end of file diff --git a/layout/reftests/table-bordercollapse/bug1375518-4.html b/layout/reftests/table-bordercollapse/bug1375518-4.html new file mode 100644 index 0000000000..97aebd456c --- /dev/null +++ b/layout/reftests/table-bordercollapse/bug1375518-4.html @@ -0,0 +1,44 @@ + +border-radius and separated border model tables + + +

border-radius and separated border model tables

+ + + + + + + + +
xxxxxx +
xxxxxx +
xxxxxx +
xxxxxx +
+ + +
xxxxxx +
xxxxxx +
+ + + +
xxxxxx +
xxxxxx +
+ + + +
xxxxxx +
xxxxxx +
\ No newline at end of file diff --git a/layout/reftests/table-bordercollapse/bug1375518-5-ref.html b/layout/reftests/table-bordercollapse/bug1375518-5-ref.html new file mode 100644 index 0000000000..eaf1710bce --- /dev/null +++ b/layout/reftests/table-bordercollapse/bug1375518-5-ref.html @@ -0,0 +1,44 @@ + +border-radius and border-collapse tables + + +

border-radius and border-collapse tables

+ + + + + + + + +
xxxxxx +
xxxxxx +
xxxxxx +
xxxxxx +
+ + +
xxxxxx +
xxxxxx +
+ + + +
xxxxxx +
xxxxxx +
+ + + +
xxxxxx +
xxxxxx +
diff --git a/layout/reftests/table-bordercollapse/bug1375518-5.html b/layout/reftests/table-bordercollapse/bug1375518-5.html new file mode 100644 index 0000000000..7f123cd42a --- /dev/null +++ b/layout/reftests/table-bordercollapse/bug1375518-5.html @@ -0,0 +1,44 @@ + +border-radius and border-collapse tables + + +

border-radius and border-collapse tables

+ + + + + + + + +
xxxxxx +
xxxxxx +
xxxxxx +
xxxxxx +
+ + +
xxxxxx +
xxxxxx +
+ + + +
xxxxxx +
xxxxxx +
+ + + +
xxxxxx +
xxxxxx +
diff --git a/layout/reftests/table-bordercollapse/bug1375518-ref.html b/layout/reftests/table-bordercollapse/bug1375518-ref.html new file mode 100644 index 0000000000..5d58d68392 --- /dev/null +++ b/layout/reftests/table-bordercollapse/bug1375518-ref.html @@ -0,0 +1,17 @@ + + + +Table border collapse + + + +
+ + \ No newline at end of file diff --git a/layout/reftests/table-bordercollapse/bug1375518.html b/layout/reftests/table-bordercollapse/bug1375518.html new file mode 100644 index 0000000000..101d925486 --- /dev/null +++ b/layout/reftests/table-bordercollapse/bug1375518.html @@ -0,0 +1,24 @@ + + + +Table border collapse + + + + + + + +
+ + \ No newline at end of file diff --git a/layout/reftests/table-bordercollapse/reftest.list b/layout/reftests/table-bordercollapse/reftest.list index 5ca6f305ab..aac4934d69 100644 --- a/layout/reftests/table-bordercollapse/reftest.list +++ b/layout/reftests/table-bordercollapse/reftest.list @@ -1,3 +1,8 @@ +== bug1375518.html bug1375518-ref.html +== bug1375518-2.html bug1375518-ref.html +== bug1375518-3.html bug1375518-ref.html +== bug1375518-4.html bug1375518-4-ref.html +== bug1375518-5.html bug1375518-5-ref.html == bc_dyn_cell1.html bc_dyn_cell1_ref.html == bc_dyn_cell2.html bc_dyn_cell2_ref.html == bc_dyn_cell3.html bc_dyn_cell3_ref.html diff --git a/layout/tables/nsTableCellFrame.cpp b/layout/tables/nsTableCellFrame.cpp index 9c715d999b..ee05565a9c 100644 --- a/layout/tables/nsTableCellFrame.cpp +++ b/layout/tables/nsTableCellFrame.cpp @@ -1108,18 +1108,6 @@ nsBCTableCellFrame::GetUsedBorder() const return GetBorderWidth(wm).GetPhysicalMargin(wm); } -/* virtual */ bool -nsBCTableCellFrame::GetBorderRadii(const nsSize& aFrameSize, - const nsSize& aBorderArea, - Sides aSkipSides, - nscoord aRadii[8]) const -{ - NS_FOR_CSS_HALF_CORNERS(corner) { - aRadii[corner] = 0; - } - return false; -} - #ifdef DEBUG_FRAME_DUMP nsresult nsBCTableCellFrame::GetFrameName(nsAString& aResult) const diff --git a/layout/tables/nsTableCellFrame.h b/layout/tables/nsTableCellFrame.h index a822e309da..2acd59667b 100644 --- a/layout/tables/nsTableCellFrame.h +++ b/layout/tables/nsTableCellFrame.h @@ -340,10 +340,6 @@ public: virtual nsIAtom* GetType() const override; virtual nsMargin GetUsedBorder() const override; - virtual bool GetBorderRadii(const nsSize& aFrameSize, - const nsSize& aBorderArea, - Sides aSkipSides, - nscoord aRadii[8]) const override; // Get the *inner half of the border only*, in twips. virtual LogicalMargin GetBorderWidth(WritingMode aWM) const override; From edaea616b3d06b9e874e83d9d72e395d36ffd72d Mon Sep 17 00:00:00 2001 From: win7-7 Date: Wed, 13 May 2020 19:57:18 +0300 Subject: [PATCH 15/15] issue #1547 - Correct z-ordering for some table parts and add reftests --- .../bug1394226-notref.html | 63 +++++++++++++++++++ .../table-bordercollapse/bug1394226-ref.html | 47 ++++++++++++++ .../table-bordercollapse/bug1394226.html | 47 ++++++++++++++ .../table-bordercollapse/reftest.list | 2 + layout/tables/nsTableWrapperFrame.cpp | 7 ++- 5 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 layout/reftests/table-bordercollapse/bug1394226-notref.html create mode 100644 layout/reftests/table-bordercollapse/bug1394226-ref.html create mode 100644 layout/reftests/table-bordercollapse/bug1394226.html diff --git a/layout/reftests/table-bordercollapse/bug1394226-notref.html b/layout/reftests/table-bordercollapse/bug1394226-notref.html new file mode 100644 index 0000000000..7c4b694a99 --- /dev/null +++ b/layout/reftests/table-bordercollapse/bug1394226-notref.html @@ -0,0 +1,63 @@ + + + +Table border collapse + + + +
+ + + + + + + + + + +
Cell 1-1Cell 1-2
Cell 2-1Cell 2-2 + + + + + + + + + +
Cell 2-2/1-1Cell 2-2/1-2
Cell 2-2/2-1Cell 2-2/2-2
+
+ + \ No newline at end of file diff --git a/layout/reftests/table-bordercollapse/bug1394226-ref.html b/layout/reftests/table-bordercollapse/bug1394226-ref.html new file mode 100644 index 0000000000..11c72d4bb5 --- /dev/null +++ b/layout/reftests/table-bordercollapse/bug1394226-ref.html @@ -0,0 +1,47 @@ + + + +Table border collapse + + + +
+ + + + + + + + + +
Cell 1-1Cell 1-2
Cell 2-1Cell 2-2 + + + + + + + + + +
Cell 2-2/1-1Cell 2-2/1-2
Cell 2-2/2-1Cell 2-2/2-2
+
+ + \ No newline at end of file diff --git a/layout/reftests/table-bordercollapse/bug1394226.html b/layout/reftests/table-bordercollapse/bug1394226.html new file mode 100644 index 0000000000..04c8ab1734 --- /dev/null +++ b/layout/reftests/table-bordercollapse/bug1394226.html @@ -0,0 +1,47 @@ + + + +Table border collapse + + + + + + + + + + + + + +
Cell 1-1Cell 1-2
Cell 2-1Cell 2-2 + + + + + + + + + +
Cell 2-2/1-1Cell 2-2/1-2
Cell 2-2/2-1Cell 2-2/2-2
+
+ + \ No newline at end of file diff --git a/layout/reftests/table-bordercollapse/reftest.list b/layout/reftests/table-bordercollapse/reftest.list index aac4934d69..2610d202d8 100644 --- a/layout/reftests/table-bordercollapse/reftest.list +++ b/layout/reftests/table-bordercollapse/reftest.list @@ -3,6 +3,8 @@ == bug1375518-3.html bug1375518-ref.html == bug1375518-4.html bug1375518-4-ref.html == bug1375518-5.html bug1375518-5-ref.html +== bug1394226.html bug1394226-ref.html +!= bug1394226.html bug1394226-notref.html == bc_dyn_cell1.html bc_dyn_cell1_ref.html == bc_dyn_cell2.html bc_dyn_cell2_ref.html == bc_dyn_cell3.html bc_dyn_cell3_ref.html diff --git a/layout/tables/nsTableWrapperFrame.cpp b/layout/tables/nsTableWrapperFrame.cpp index 476024e96d..86f032218e 100644 --- a/layout/tables/nsTableWrapperFrame.cpp +++ b/layout/tables/nsTableWrapperFrame.cpp @@ -187,8 +187,11 @@ nsTableWrapperFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, BuildDisplayListForChild(aBuilder, mCaptionFrames.FirstChild(), captionSet); // Now we have to sort everything by content order, since the caption - // may be somewhere inside the table - set.BlockBorderBackgrounds()->SortByContentOrder(GetContent()); + // may be somewhere inside the table. + // We don't sort BlockBorderBackgrounds and BorderBackgrounds because the + // display items in those lists should stay out of content order in order to + // follow the rules in https://www.w3.org/TR/CSS21/zindex.html#painting-order + // and paint the caption background after all of the rest. set.Floats()->SortByContentOrder(GetContent()); set.Content()->SortByContentOrder(GetContent()); set.PositionedDescendants()->SortByContentOrder(GetContent());