From c67b1f18a30a3031996d694454afd7b1ee2e2a89 Mon Sep 17 00:00:00 2001 From: Michael Layzell Date: Thu, 7 Sep 2017 13:41:13 -0400 Subject: [PATCH 1/8] Bug 1340578 - Allow execCommand('paste') to be called from webextensions without a target, r=ehsan --- dom/html/nsHTMLDocument.cpp | 8 ++-- editor/libeditor/tests/mochitest.ini | 1 + .../tests/test_execCommandPaste_noTarget.html | 48 +++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 editor/libeditor/tests/test_execCommandPaste_noTarget.html diff --git a/dom/html/nsHTMLDocument.cpp b/dom/html/nsHTMLDocument.cpp index be33b94adc771..c6ac8fa4ab139 100644 --- a/dom/html/nsHTMLDocument.cpp +++ b/dom/html/nsHTMLDocument.cpp @@ -3242,9 +3242,10 @@ nsHTMLDocument::ExecCommand(const nsAString& commandID, bool isCutCopy = (commandID.LowerCaseEqualsLiteral("cut") || commandID.LowerCaseEqualsLiteral("copy")); + bool isPaste = commandID.LowerCaseEqualsLiteral("paste"); // if editing is not on, bail - if (!isCutCopy && !IsEditingOnAfterFlush()) { + if (!isCutCopy && !isPaste && !IsEditingOnAfterFlush()) { return false; } @@ -3285,9 +3286,8 @@ nsHTMLDocument::ExecCommand(const nsAString& commandID, return false; } - bool restricted = commandID.LowerCaseEqualsLiteral("paste"); - if (restricted && !nsContentUtils::PrincipalHasPermission(&aSubjectPrincipal, - nsGkAtoms::clipboardRead)) { + if (isPaste && !nsContentUtils::PrincipalHasPermission(&aSubjectPrincipal, + nsGkAtoms::clipboardRead)) { return false; } diff --git a/editor/libeditor/tests/mochitest.ini b/editor/libeditor/tests/mochitest.ini index 56dcd2da366cc..9fcc78fa9e421 100644 --- a/editor/libeditor/tests/mochitest.ini +++ b/editor/libeditor/tests/mochitest.ini @@ -277,3 +277,4 @@ skip-if = toolkit == 'android' # chrome urls not available due to packaging [test_selection_move_commands.html] [test_pasteImgTextarea.html] skip-if = toolkit == 'android' # bug 1299578 +[test_execCommandPaste_noTarget.html] diff --git a/editor/libeditor/tests/test_execCommandPaste_noTarget.html b/editor/libeditor/tests/test_execCommandPaste_noTarget.html new file mode 100644 index 0000000000000..fe465385ac46b --- /dev/null +++ b/editor/libeditor/tests/test_execCommandPaste_noTarget.html @@ -0,0 +1,48 @@ + + + + + + + + + + + + From 4b8dbea3cbccff6326ec22b1b9fc95b73cea8af6 Mon Sep 17 00:00:00 2001 From: Simon Lindholm Date: Sat, 9 Sep 2017 02:36:00 -0400 Subject: [PATCH 2/8] Bug 1393486 - Reduce subquery evaluations in awesomebar search SQL. r=mak --- toolkit/components/places/UnifiedComplete.js | 24 ++++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/toolkit/components/places/UnifiedComplete.js b/toolkit/components/places/UnifiedComplete.js index 0a4b2befd305e..0fb3f5efda00a 100644 --- a/toolkit/components/places/UnifiedComplete.js +++ b/toolkit/components/places/UnifiedComplete.js @@ -166,8 +166,8 @@ const SQL_BOOKMARK_TAGS_FRAGMENT = // TODO bug 412736: in case of a frecency tie, we might break it with h.typed // and h.visit_count. That is slower though, so not doing it yet... -// NB: as a slight performance optimization, we only evaluate the "btitle" -// and "tags" queries for bookmarked entries. +// NB: as a slight performance optimization, we only evaluate the "bookmarked" +// condition once, and avoid evaluating "btitle" and "tags" when it is false. function defaultQuery(conditions = "") { let query = `SELECT :query_type, h.url, h.title, ${SQL_BOOKMARK_TAGS_FRAGMENT}, @@ -177,16 +177,20 @@ function defaultQuery(conditions = "") { ON t.url = h.url AND t.userContextId = :userContextId WHERE h.frecency <> 0 - AND AUTOCOMPLETE_MATCH(:searchString, h.url, - CASE WHEN bookmarked THEN - IFNULL(btitle, h.title) - ELSE h.title END, - CASE WHEN bookmarked THEN - tags - ELSE '' END, + AND CASE WHEN bookmarked + THEN + AUTOCOMPLETE_MATCH(:searchString, h.url, + IFNULL(btitle, h.title), tags, h.visit_count, h.typed, - bookmarked, t.open_count, + 1, t.open_count, :matchBehavior, :searchBehavior) + ELSE + AUTOCOMPLETE_MATCH(:searchString, h.url, + h.title, '', + h.visit_count, h.typed, + 0, t.open_count, + :matchBehavior, :searchBehavior) + END ${conditions} ORDER BY h.frecency DESC, h.id DESC LIMIT :maxResults`; From 122408f0fff061d4b2c32288147927275ac25fa2 Mon Sep 17 00:00:00 2001 From: Anthony Miyaguchi Date: Tue, 5 Sep 2017 17:49:45 -0700 Subject: [PATCH 3/8] Bug 1390095 - Send a duplicate of a users first shutdown ping with pingsender. r=Dexter --- browser/app/profile/firefox.js | 2 + testing/profiles/prefs_general.js | 3 + .../components/telemetry/TelemetrySession.jsm | 19 ++- .../components/telemetry/TelemetryUtils.jsm | 1 + .../telemetry/docs/data/first-shutdown.rst | 10 ++ .../telemetry/docs/internals/preferences.rst | 4 + .../components/telemetry/tests/unit/head.js | 1 + .../tests/unit/test_TelemetrySession.js | 120 ++++++++++++++++++ 8 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 toolkit/components/telemetry/docs/data/first-shutdown.rst diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index b352e9111d64b..60e5c5be56131 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1481,6 +1481,8 @@ pref("toolkit.telemetry.archive.enabled", true); pref("toolkit.telemetry.shutdownPingSender.enabled", true); // Enables sending the shutdown ping using the pingsender from the first session. pref("toolkit.telemetry.shutdownPingSender.enabledFirstSession", false); +// Enables sending a duplicate of the first shutdown ping from the first session. +pref("toolkit.telemetry.firstShutdownPing.enabled", true); // Enables sending the 'new-profile' ping on new profiles. pref("toolkit.telemetry.newProfilePing.enabled", true); // Enables sending 'update' pings on Firefox updates. diff --git a/testing/profiles/prefs_general.js b/testing/profiles/prefs_general.js index ce5c65235747f..4a04b51f6d3e0 100644 --- a/testing/profiles/prefs_general.js +++ b/testing/profiles/prefs_general.js @@ -260,6 +260,9 @@ user_pref("toolkit.telemetry.bhrPing.enabled", false); // to suppress the leak. Running locally does not reproduce the issue, // so disable this until we rewrite the pingsender in Rust (bug 1339035). user_pref("toolkit.telemetry.shutdownPingSender.enabledFirstSession", false); +// Don't send the 'first-shutdown' during tests, otherwise tests expecting +// main and subsession pings will fail. +user_pref("toolkit.telemetry.firstShutdownPing.enabled", false); // A couple of preferences with default values to test that telemetry preference // watching is working. diff --git a/toolkit/components/telemetry/TelemetrySession.jsm b/toolkit/components/telemetry/TelemetrySession.jsm index f27dc8f44fad1..2d22d0477c77a 100644 --- a/toolkit/components/telemetry/TelemetrySession.jsm +++ b/toolkit/components/telemetry/TelemetrySession.jsm @@ -1699,7 +1699,24 @@ var Impl = { }; p.push(TelemetryController.submitExternalPing(getPingType(shutdownPayload), shutdownPayload, options) .catch(e => this._log.error("saveShutdownPings - failed to submit shutdown ping", e))); - } + + // Send a duplicate of first-shutdown pings as a new ping type, in order to properly + // evaluate first session profiles (see bug 1390095). + const sendFirstShutdownPing = + Services.prefs.getBoolPref(Utils.Preferences.ShutdownPingSender, false) && + Services.prefs.getBoolPref(Utils.Preferences.FirstShutdownPingEnabled, false) && + TelemetryReportingPolicy.isFirstRun(); + + if (sendFirstShutdownPing) { + let options = { + addClientId: true, + addEnvironment: true, + usePingSender: true, + }; + p.push(TelemetryController.submitExternalPing("first-shutdown", shutdownPayload, options) + .catch(e => this._log.error("saveShutdownPings - failed to submit first shutdown ping", e))); + } + } // As a temporary measure, we want to submit saved-session too if extended Telemetry is enabled // to keep existing performance analysis working. diff --git a/toolkit/components/telemetry/TelemetryUtils.jsm b/toolkit/components/telemetry/TelemetryUtils.jsm index ab8cc2d5227dc..1668d74c16e37 100644 --- a/toolkit/components/telemetry/TelemetryUtils.jsm +++ b/toolkit/components/telemetry/TelemetryUtils.jsm @@ -29,6 +29,7 @@ this.TelemetryUtils = { ArchiveEnabled: "toolkit.telemetry.archive.enabled", CachedClientId: "toolkit.telemetry.cachedClientID", FirstRun: "toolkit.telemetry.reportingpolicy.firstRun", + FirstShutdownPingEnabled: "toolkit.telemetry.firstShutdownPing.enabled", HealthPingEnabled: "toolkit.telemetry.healthping.enabled", OverrideOfficialCheck: "toolkit.telemetry.send.overrideOfficialCheck", Server: "toolkit.telemetry.server", diff --git a/toolkit/components/telemetry/docs/data/first-shutdown.rst b/toolkit/components/telemetry/docs/data/first-shutdown.rst new file mode 100644 index 0000000000000..7476a54fba7d9 --- /dev/null +++ b/toolkit/components/telemetry/docs/data/first-shutdown.rst @@ -0,0 +1,10 @@ + +"first-shutdown" ping +================== + +This ping is a duplicate of the main-ping sent on first shutdown. Enabling pingsender +for first sessions will impact existing engagment metrics. The ``first-shutdown`` ping enables a +more accurate view of users that churn after the first session. This ping exists as a +stopgap until existing metrics are re-evaluated for use first session ``main-pings``. + +See :doc:`main-ping` for details about this payload. diff --git a/toolkit/components/telemetry/docs/internals/preferences.rst b/toolkit/components/telemetry/docs/internals/preferences.rst index 36ebfd01f4943..c58ee7bb9838a 100644 --- a/toolkit/components/telemetry/docs/internals/preferences.rst +++ b/toolkit/components/telemetry/docs/internals/preferences.rst @@ -58,6 +58,10 @@ Preferences Allow the ``shutdown`` ping to be sent using the :doc:`ping sender ` from the first browsing session. +``toolkit.telemetry.firstShutdownPing.enabled`` + + Allow a duplicate of the shutdown ping from the first browsing session to be sent as a separate ``first-shutdown`` ping. + ``toolkit.telemetry.newProfilePing.enabled`` Enable the :doc:`../data/new-profile` ping on new profiles. diff --git a/toolkit/components/telemetry/tests/unit/head.js b/toolkit/components/telemetry/tests/unit/head.js index 80fdfd180010b..ac70efb05d288 100644 --- a/toolkit/components/telemetry/tests/unit/head.js +++ b/toolkit/components/telemetry/tests/unit/head.js @@ -320,6 +320,7 @@ if (runningInParent) { Services.prefs.setBoolPref(TelemetryUtils.Preferences.ShutdownPingSender, false); Services.prefs.setBoolPref(TelemetryUtils.Preferences.ShutdownPingSenderFirstSession, false); Services.prefs.setBoolPref("toolkit.telemetry.newProfilePing.enabled", false); + Services.prefs.setBoolPref(TelemetryUtils.Preferences.FirstShutdownPingEnabled, false); // Ensure browser experiments are also disabled, to avoid network activity // when toggling PREF_ENABLED. Services.prefs.setBoolPref("experiments.enabled", false); diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js index 510fe3fd61e00..f362d09b2689e 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySession.js @@ -1483,6 +1483,126 @@ add_task(async function test_sendShutdownPing() { PingServer.resetPingHandler(); }); +add_task(async function test_sendFirstShutdownPing() { + if (gIsAndroid || + (AppConstants.platform == "linux" && OS.Constants.Sys.bits == 32)) { + // We don't support the pingsender on Android, yet, see bug 1335917. + // We also don't suppor the pingsender testing on Treeherder for + // Linux 32 bit (due to missing libraries). So skip it there too. + // See bug 1310703 comment 78. + return; + } + + let storageContainsFirstShutdown = async function() { + let pendingPings = await TelemetryStorage.loadPendingPingList(); + let pings = await Promise.all( + pendingPings.map(async (p) => { + return TelemetryStorage.loadPendingPing(p.id) + }) + ); + return pings.find(p => p.type == "first-shutdown") + } + + let checkShutdownNotSent = async function() { + // The failure-mode of the ping-sender is used to check that a ping was + // *not* sent. This can be combined with the state of the storage to infer + // the appropriate behavior from the preference flags. + + // Assert failure if we recive a ping. + PingServer.registerPingHandler((req, res) => { + const receivedPing = decodeRequestPayload(req); + Assert.ok(false, `No ping should be received in this test (got ${receivedPing.id}).`); + }); + + // Assert that pings are sent on first run, forcing a forced application + // quit. This should be equivalent to the first test in this suite. + Preferences.set(TelemetryUtils.Preferences.FirstRun, true); + TelemetryReportingPolicy.testUpdateFirstRun(); + + await TelemetryController.testReset(); + Services.obs.notifyObservers(null, "quit-application-forced"); + await TelemetryController.testShutdown(); + Assert.ok(await storageContainsFirstShutdown(), + "The 'first-shutdown' ping must be saved to disk.") + + await TelemetryStorage.testClearPendingPings(); + + // Assert that it's not sent during subsequent runs + Preferences.set(TelemetryUtils.Preferences.FirstRun, false); + TelemetryReportingPolicy.testUpdateFirstRun(); + + await TelemetryController.testReset(); + Services.obs.notifyObservers(null, "quit-application-forced"); + await TelemetryController.testShutdown(); + Assert.ok(!(await storageContainsFirstShutdown()), + "The 'first-shutdown' ping should only be written during first run.") + + await TelemetryStorage.testClearPendingPings(); + + // Assert that the the ping is only sent if the flag is enabled. + Preferences.set(TelemetryUtils.Preferences.FirstRun, true); + Preferences.set(TelemetryUtils.Preferences.FirstShutdownPingEnabled, false); + TelemetryReportingPolicy.testUpdateFirstRun(); + + await TelemetryController.testReset(); + await TelemetryController.testShutdown(); + Assert.ok(!(await storageContainsFirstShutdown()), + "The 'first-shutdown' ping should only be written if enabled") + + await TelemetryStorage.testClearPendingPings(); + + // Assert that the the ping is not collected when the ping-sender is disabled. + // The information would be made irrelevant by the main-ping in the second session. + Preferences.set(TelemetryUtils.Preferences.FirstShutdownPingEnabled, true); + Preferences.set(TelemetryUtils.Preferences.ShutdownPingSender, false); + TelemetryReportingPolicy.testUpdateFirstRun(); + + await TelemetryController.testReset(); + await TelemetryController.testShutdown(); + Assert.ok(!(await storageContainsFirstShutdown()), + "The 'first-shutdown' ping should only be written if ping-sender is enabled") + + // Clear the state and prepare for the next test. + await TelemetryStorage.testClearPendingPings(); + PingServer.clearRequests(); + PingServer.resetPingHandler(); + } + + // Remove leftover pending pings from other tests + await TelemetryStorage.testClearPendingPings(); + PingServer.clearRequests(); + Telemetry.clearScalars(); + + // Set testing invariants for FirstShutdownPingEnabled + Preferences.set(TelemetryUtils.Preferences.ShutdownPingSender, true); + Preferences.set(TelemetryUtils.Preferences.ShutdownPingSenderFirstSession, false) + + // Set primary conditions of the 'first-shutdown' ping + Preferences.set(TelemetryUtils.Preferences.FirstShutdownPingEnabled, true); + Preferences.set(TelemetryUtils.Preferences.FirstRun, true); + TelemetryReportingPolicy.testUpdateFirstRun(); + + // Assert general 'first-shutdown' use-case. + await TelemetryController.testReset(); + await TelemetryController.testShutdown(); + let ping = await PingServer.promiseNextPing(); + checkPingFormat(ping, "first-shutdown", true, true); + Assert.equal(ping.payload.info.reason, REASON_SHUTDOWN); + Assert.equal(ping.clientId, gClientID); + + await TelemetryStorage.testClearPendingPings(); + + // Assert that the shutdown is not sent under various conditions + await checkShutdownNotSent(); + + // Reset the pref and restart Telemetry. + Preferences.set(TelemetryUtils.Preferences.ShutdownPingSender, false); + Preferences.set(TelemetryUtils.Preferences.ShutdownPingSenderFirstSession, false); + Preferences.set(TelemetryUtils.Preferences.FirstShutdownPingEnabled, false); + Preferences.reset(TelemetryUtils.Preferences.FirstRun); + PingServer.resetPingHandler(); +}); + add_task(async function test_savedSessionData() { // Create the directory which will contain the data file, if it doesn't already // exist. From aa59a13010be9e598acd1eb14f986e339617feaf Mon Sep 17 00:00:00 2001 From: Myk Melez Date: Mon, 11 Sep 2017 19:22:15 +0100 Subject: [PATCH 4/8] Bug 1395061 - patch 1 - Refactor gfxFontEntry::SupportsLangGroup and MatchesGenericFamily into gfxFontFamily. r=jfkthame --- gfx/thebes/gfxFcPlatformFontList.cpp | 81 ++++++++------ gfx/thebes/gfxFcPlatformFontList.h | 4 +- gfx/thebes/gfxFontEntry.h | 15 +-- gfx/thebes/gfxGDIFontList.cpp | 22 ++-- gfx/thebes/gfxGDIFontList.h | 153 ++++++++++++++------------- gfx/thebes/gfxPlatformFontList.cpp | 4 +- 6 files changed, 151 insertions(+), 128 deletions(-) diff --git a/gfx/thebes/gfxFcPlatformFontList.cpp b/gfx/thebes/gfxFcPlatformFontList.cpp index 4caae061b1a46..810514bdbc742 100644 --- a/gfx/thebes/gfxFcPlatformFontList.cpp +++ b/gfx/thebes/gfxFcPlatformFontList.cpp @@ -325,39 +325,6 @@ gfxFontconfigFontEntry::~gfxFontconfigFontEntry() { } -static bool -PatternHasLang(const FcPattern *aPattern, const FcChar8 *aLang) -{ - FcLangSet *langset; - - if (FcPatternGetLangSet(aPattern, FC_LANG, 0, &langset) != FcResultMatch) { - return false; - } - - if (FcLangSetHasLang(langset, aLang) != FcLangDifferentLang) { - return true; - } - return false; -} - -bool -gfxFontconfigFontEntry::SupportsLangGroup(nsIAtom *aLangGroup) const -{ - if (!aLangGroup || aLangGroup == nsGkAtoms::Unicode) { - return true; - } - - nsAutoCString fcLang; - gfxFcPlatformFontList* pfl = gfxFcPlatformFontList::PlatformFontList(); - pfl->GetSampleLangForGroup(aLangGroup, fcLang); - if (fcLang.IsEmpty()) { - return true; - } - - // is lang included in the underlying pattern? - return PatternHasLang(mFontPattern, ToFcChar8Ptr(fcLang.get())); -} - nsresult gfxFontconfigFontEntry::ReadCMAP(FontInfoData *aFontInfoData) { @@ -1189,6 +1156,54 @@ gfxFontconfigFontFamily::FindAllFontsForStyle(const gfxFontStyle& aFontStyle, } } +static bool +PatternHasLang(const FcPattern *aPattern, const FcChar8 *aLang) +{ + FcLangSet *langset; + + if (FcPatternGetLangSet(aPattern, FC_LANG, 0, &langset) != FcResultMatch) { + return false; + } + + if (FcLangSetHasLang(langset, aLang) != FcLangDifferentLang) { + return true; + } + return false; +} + +bool +gfxFontconfigFontFamily::SupportsLangGroup(nsIAtom *aLangGroup) const +{ + if (!aLangGroup || aLangGroup == nsGkAtoms::Unicode) { + return true; + } + + nsAutoCString fcLang; + gfxFcPlatformFontList* pfl = gfxFcPlatformFontList::PlatformFontList(); + pfl->GetSampleLangForGroup(aLangGroup, fcLang); + if (fcLang.IsEmpty()) { + return true; + } + + // Before FindStyleVariations has been called, mFontPatterns will contain + // the font patterns. Afterward, it'll be empty, but mAvailableFonts + // will contain the font entries, each of which holds a reference to its + // pattern. We only check the first pattern in each list, because support + // for langs is considered to be consistent across all faces in a family. + FcPattern* fontPattern; + if (mFontPatterns.Length()) { + fontPattern = mFontPatterns[0]; + } else if (mAvailableFonts.Length()) { + fontPattern = static_cast + (mAvailableFonts[0].get())->GetPattern(); + } else { + return true; + } + + // is lang included in the underlying pattern? + return PatternHasLang(fontPattern, ToFcChar8Ptr(fcLang.get())); +} + /* virtual */ gfxFontconfigFontFamily::~gfxFontconfigFontFamily() { diff --git a/gfx/thebes/gfxFcPlatformFontList.h b/gfx/thebes/gfxFcPlatformFontList.h index 490922767f3cd..21c4cfb78a449 100644 --- a/gfx/thebes/gfxFcPlatformFontList.h +++ b/gfx/thebes/gfxFcPlatformFontList.h @@ -99,8 +99,6 @@ class gfxFontconfigFontEntry : public gfxFontEntry { FcPattern* GetPattern() { return mFontPattern; } - bool SupportsLangGroup(nsIAtom *aLangGroup) const override; - nsresult ReadCMAP(FontInfoData *aFontInfoData = nullptr) override; bool TestCharacterMap(uint32_t aCh) override; @@ -201,6 +199,8 @@ class gfxFontconfigFontFamily : public gfxFontFamily { bool& aNeedsSyntheticBold, bool aIgnoreSizeTolerance) override; + bool SupportsLangGroup(nsIAtom *aLangGroup) const override; + protected: virtual ~gfxFontconfigFontFamily(); diff --git a/gfx/thebes/gfxFontEntry.h b/gfx/thebes/gfxFontEntry.h index bdb8a11778b27..5afbee174711c 100644 --- a/gfx/thebes/gfxFontEntry.h +++ b/gfx/thebes/gfxFontEntry.h @@ -207,13 +207,6 @@ class gfxFontEntry { nsTArray& layerGlyphs, nsTArray& layerColors); - virtual bool MatchesGenericFamily(const nsACString& aGeneric) const { - return true; - } - virtual bool SupportsLangGroup(nsIAtom *aLangGroup) const { - return true; - } - // Access to raw font table data (needed for Harfbuzz): // returns a pointer to data owned by the fontEntry or the OS, // which will remain valid until the blob is destroyed. @@ -764,6 +757,14 @@ class gfxFontFamily { mSkipDefaultFeatureSpaceCheck = aSkipCheck; } + virtual bool MatchesGenericFamily(const nsACString& aGeneric) const { + return true; + } + + virtual bool SupportsLangGroup(nsIAtom *aLangGroup) const { + return true; + } + protected: // Protected destructor, to discourage deletion outside of Release(): virtual ~gfxFontFamily(); diff --git a/gfx/thebes/gfxGDIFontList.cpp b/gfx/thebes/gfxGDIFontList.cpp index 3eda6f624da8a..b695fce25e260 100644 --- a/gfx/thebes/gfxGDIFontList.cpp +++ b/gfx/thebes/gfxGDIFontList.cpp @@ -120,11 +120,10 @@ GDIFontEntry::GDIFontEntry(const nsAString& aFaceName, gfxUserFontData *aUserFontData, bool aFamilyHasItalicFace) : gfxFontEntry(aFaceName), - mWindowsFamily(0), mWindowsPitch(0), mFontType(aFontType), mForceGDI(false), mFamilyHasItalicFace(aFamilyHasItalicFace), - mCharset(), mUnicodeRanges() + mUnicodeRanges() { mUserFontData.reset(aUserFontData); mStyle = aStyle; @@ -486,7 +485,9 @@ GDIFontFamily::FamilyAddStylesProc(const ENUMLOGFONTEXW *lpelfe, if (fe->mWeight == logFont.lfWeight && fe->IsItalic() == (logFont.lfItalic == 0xFF)) { // update the charset bit here since this could be different - fe->mCharset.set(metrics.tmCharSet); + // XXX Can we still do this now that we store mCharset + // on the font family rather than the font entry? + ff->mCharset.set(metrics.tmCharSet); return 1; } } @@ -505,12 +506,6 @@ GDIFontFamily::FamilyAddStylesProc(const ENUMLOGFONTEXW *lpelfe, ff->AddFontEntry(fe); - // mark the charset bit - fe->mCharset.set(metrics.tmCharSet); - - fe->mWindowsFamily = logFont.lfPitchAndFamily & 0xF0; - fe->mWindowsPitch = logFont.lfPitchAndFamily & 0x0F; - if (nmetrics->ntmFontSig.fsUsb[0] != 0x00000000 && nmetrics->ntmFontSig.fsUsb[1] != 0x00000000 && nmetrics->ntmFontSig.fsUsb[2] != 0x00000000 && @@ -713,6 +708,7 @@ gfxGDIFontList::EnumFontFamExProc(ENUMLOGFONTEXW *lpelfe, DWORD fontType, LPARAM lParam) { + const NEWTEXTMETRICW& metrics = lpntme->ntmTm; const LOGFONTW& lf = lpelfe->elfLogFont; if (lf.lfFaceName[0] == '@') { @@ -726,7 +722,7 @@ gfxGDIFontList::EnumFontFamExProc(ENUMLOGFONTEXW *lpelfe, if (!fontList->mFontFamilies.GetWeak(name)) { nsDependentString faceName(lf.lfFaceName); - RefPtr family = new GDIFontFamily(faceName); + RefPtr family = new GDIFontFamily(faceName); fontList->mFontFamilies.Put(name, family); // if locale is such that CJK font names are the default coming from @@ -739,6 +735,12 @@ gfxGDIFontList::EnumFontFamExProc(ENUMLOGFONTEXW *lpelfe, if (fontList->mBadUnderlineFamilyNames.Contains(name)) family->SetBadUnderlineFamily(); + + family->mWindowsFamily = lf.lfPitchAndFamily & 0xF0; + family->mWindowsPitch = lf.lfPitchAndFamily & 0x0F; + + // mark the charset bit + family->mCharset.set(metrics.tmCharSet); } return 1; diff --git a/gfx/thebes/gfxGDIFontList.h b/gfx/thebes/gfxGDIFontList.h index e9aa289a3f264..abd2c2b3d0366 100644 --- a/gfx/thebes/gfxGDIFontList.h +++ b/gfx/thebes/gfxGDIFontList.h @@ -147,6 +147,83 @@ class GDIFontEntry : public gfxFontEntry mFontType == GFX_FONT_TYPE_TT_OPENTYPE); } + virtual bool SupportsRange(uint8_t range) { + return mUnicodeRanges.test(range); + } + + virtual bool SkipDuringSystemFallback() { + return !HasCmapTable(); // explicitly skip non-SFNT fonts + } + + virtual bool TestCharacterMap(uint32_t aCh); + + virtual void AddSizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf, + FontListSizes* aSizes) const; + + gfxFontEntry* Clone() const override; + + // create a font entry for a font with a given name + static GDIFontEntry* CreateFontEntry(const nsAString& aName, + gfxWindowsFontType aFontType, + uint8_t aStyle, + uint16_t aWeight, int16_t aStretch, + gfxUserFontData* aUserFontData, + bool aFamilyHasItalicFace); + + // create a font entry for a font referenced by its fullname + static GDIFontEntry* LoadLocalFont(const nsAString& aFontName, + uint16_t aWeight, + int16_t aStretch, + uint8_t aStyle); + + gfxWindowsFontType mFontType; + bool mForceGDI : 1; + + // For src:local user-fonts, we keep track of whether the platform family + // contains an italic face, because in this case we can't safely ask GDI + // to create synthetic italics (oblique) via the LOGFONT. + // (For other types of font, this is just set to false.) + bool mFamilyHasItalicFace : 1; + + gfxSparseBitSet mUnicodeRanges; + +protected: + friend class gfxGDIFont; + + GDIFontEntry(const nsAString& aFaceName, gfxWindowsFontType aFontType, + uint8_t aStyle, uint16_t aWeight, int16_t aStretch, + gfxUserFontData *aUserFontData, bool aFamilyHasItalicFace); + + void InitLogFont(const nsAString& aName, gfxWindowsFontType aFontType); + + virtual gfxFont *CreateFontInstance(const gfxFontStyle *aFontStyle, bool aNeedsBold); + + virtual nsresult CopyFontTable(uint32_t aTableTag, + nsTArray& aBuffer) override; + + already_AddRefed LookupUnscaledFont(HFONT aFont); + + LOGFONTW mLogFont; + + mozilla::WeakPtr mUnscaledFont; +}; + +// a single font family, referencing one or more faces +class GDIFontFamily : public gfxFontFamily +{ +public: + explicit GDIFontFamily(const nsAString& aName) : + gfxFontFamily(aName), + mWindowsFamily(0), + mWindowsPitch(0), + mCharset() + {} + + virtual void FindStyleVariations(FontInfoData *aFontInfoData = nullptr); + + uint8_t mWindowsFamily; + uint8_t mWindowsPitch; + virtual bool MatchesGenericFamily(const nsACString& aGeneric) const { if (aGeneric.IsEmpty()) { return true; @@ -183,6 +260,8 @@ class GDIFontEntry : public gfxFontEntry return false; } + gfxSparseBitSet mCharset; + virtual bool SupportsLangGroup(nsIAtom* aLangGroup) const override { if (!aLangGroup || aLangGroup == nsGkAtoms::Unicode) { return true; @@ -220,80 +299,6 @@ class GDIFontEntry : public gfxFontEntry return false; } - virtual bool SupportsRange(uint8_t range) { - return mUnicodeRanges.test(range); - } - - virtual bool SkipDuringSystemFallback() { - return !HasCmapTable(); // explicitly skip non-SFNT fonts - } - - virtual bool TestCharacterMap(uint32_t aCh); - - virtual void AddSizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf, - FontListSizes* aSizes) const; - - gfxFontEntry* Clone() const override; - - // create a font entry for a font with a given name - static GDIFontEntry* CreateFontEntry(const nsAString& aName, - gfxWindowsFontType aFontType, - uint8_t aStyle, - uint16_t aWeight, int16_t aStretch, - gfxUserFontData* aUserFontData, - bool aFamilyHasItalicFace); - - // create a font entry for a font referenced by its fullname - static GDIFontEntry* LoadLocalFont(const nsAString& aFontName, - uint16_t aWeight, - int16_t aStretch, - uint8_t aStyle); - - uint8_t mWindowsFamily; - uint8_t mWindowsPitch; - - gfxWindowsFontType mFontType; - bool mForceGDI : 1; - - // For src:local user-fonts, we keep track of whether the platform family - // contains an italic face, because in this case we can't safely ask GDI - // to create synthetic italics (oblique) via the LOGFONT. - // (For other types of font, this is just set to false.) - bool mFamilyHasItalicFace : 1; - - gfxSparseBitSet mCharset; - gfxSparseBitSet mUnicodeRanges; - -protected: - friend class gfxGDIFont; - - GDIFontEntry(const nsAString& aFaceName, gfxWindowsFontType aFontType, - uint8_t aStyle, uint16_t aWeight, int16_t aStretch, - gfxUserFontData *aUserFontData, bool aFamilyHasItalicFace); - - void InitLogFont(const nsAString& aName, gfxWindowsFontType aFontType); - - virtual gfxFont *CreateFontInstance(const gfxFontStyle *aFontStyle, bool aNeedsBold); - - virtual nsresult CopyFontTable(uint32_t aTableTag, - nsTArray& aBuffer) override; - - already_AddRefed LookupUnscaledFont(HFONT aFont); - - LOGFONTW mLogFont; - - mozilla::WeakPtr mUnscaledFont; -}; - -// a single font family, referencing one or more faces -class GDIFontFamily : public gfxFontFamily -{ -public: - explicit GDIFontFamily(const nsAString& aName) : - gfxFontFamily(aName) {} - - virtual void FindStyleVariations(FontInfoData *aFontInfoData = nullptr); - private: static int CALLBACK FamilyAddStylesProc(const ENUMLOGFONTEXW *lpelfe, const NEWTEXTMETRICEXW *nmetrics, diff --git a/gfx/thebes/gfxPlatformFontList.cpp b/gfx/thebes/gfxPlatformFontList.cpp index 10f767d075413..4f86980ef468e 100644 --- a/gfx/thebes/gfxPlatformFontList.cpp +++ b/gfx/thebes/gfxPlatformFontList.cpp @@ -500,8 +500,8 @@ gfxPlatformFontList::GetFontList(nsIAtom *aLangGroup, continue; } - if (fontEntry->SupportsLangGroup(aLangGroup) && - fontEntry->MatchesGenericFamily(aGenericFamily)) { + if (family->SupportsLangGroup(aLangGroup) && + family->MatchesGenericFamily(aGenericFamily)) { nsAutoString localizedFamilyName; family->LocalizedName(localizedFamilyName); aListOfFonts.AppendElement(localizedFamilyName); From 379ddad5975b54331b88c991f6b933766e42e467 Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Mon, 11 Sep 2017 19:22:57 +0100 Subject: [PATCH 5/8] Bug 1395061 - patch 2 - Implement default gfxFontFamily::IsSymbolFontFamily, and provide override for GDI font backend. r=jrmuizel --- gfx/thebes/gfxDWriteFontList.cpp | 23 +++++++++++++---------- gfx/thebes/gfxDWriteFontList.h | 4 ++-- gfx/thebes/gfxFontEntry.cpp | 7 ------- gfx/thebes/gfxFontEntry.h | 7 ++++--- gfx/thebes/gfxGDIFontList.cpp | 10 ---------- gfx/thebes/gfxGDIFontList.h | 6 ++++-- gfx/thebes/gfxPlatformFontList.cpp | 20 ++------------------ 7 files changed, 25 insertions(+), 52 deletions(-) diff --git a/gfx/thebes/gfxDWriteFontList.cpp b/gfx/thebes/gfxDWriteFontList.cpp index f24c28bfca1b0..7a7c66ade2647 100644 --- a/gfx/thebes/gfxDWriteFontList.cpp +++ b/gfx/thebes/gfxDWriteFontList.cpp @@ -340,6 +340,19 @@ gfxDWriteFontFamily::LocalizedName(nsAString &aLocalizedName) aLocalizedName = nsDependentString(famName.Elements()); } +bool +gfxDWriteFontFamily::IsSymbolFontFamily() const +{ + // Just check the first font in the family + if (mDWFamily->GetFontCount() > 0) { + RefPtr font; + if (SUCCEEDED(mDWFamily->GetFont(0, getter_AddRefs(font)))) { + return font->IsSymbolFont(); + } + } + return false; +} + void gfxDWriteFontFamily::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf, FontListSizes* aSizes) const @@ -371,16 +384,6 @@ gfxDWriteFontEntry::~gfxDWriteFontEntry() { } -bool -gfxDWriteFontEntry::IsSymbolFont() -{ - if (mFont) { - return mFont->IsSymbolFont(); - } else { - return false; - } -} - static bool UsingArabicOrHebrewScriptSystemLocale() { diff --git a/gfx/thebes/gfxDWriteFontList.h b/gfx/thebes/gfxDWriteFontList.h index dcf23e3a96dde..b2bfcaa3266ba 100644 --- a/gfx/thebes/gfxDWriteFontList.h +++ b/gfx/thebes/gfxDWriteFontList.h @@ -55,6 +55,8 @@ class gfxDWriteFontFamily : public gfxFontFamily void SetForceGDIClassic(bool aForce) { mForceGDIClassic = aForce; } + bool IsSymbolFontFamily() const final; + void AddSizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf, FontListSizes* aSizes) const final; void AddSizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf, @@ -154,8 +156,6 @@ class gfxDWriteFontEntry : public gfxFontEntry virtual ~gfxDWriteFontEntry(); - virtual bool IsSymbolFont(); - virtual hb_blob_t* GetFontTable(uint32_t aTableTag) override; nsresult ReadCMAP(FontInfoData *aFontInfoData = nullptr); diff --git a/gfx/thebes/gfxFontEntry.cpp b/gfx/thebes/gfxFontEntry.cpp index b6ad830cdf61b..11c5ccbbf5117 100644 --- a/gfx/thebes/gfxFontEntry.cpp +++ b/gfx/thebes/gfxFontEntry.cpp @@ -65,7 +65,6 @@ gfxFontEntry::gfxFontEntry() : mIsDataUserFont(false), mIsLocalUserFont(false), mStandardFace(false), - mSymbolFont(false), mIgnoreGDEF(false), mIgnoreGSUB(false), mSVGInitialized(false), @@ -104,7 +103,6 @@ gfxFontEntry::gfxFontEntry(const nsAString& aName, bool aIsStandardFace) : mIsUserFontContainer(false), mIsDataUserFont(false), mIsLocalUserFont(false), mStandardFace(aIsStandardFace), - mSymbolFont(false), mIgnoreGDEF(false), mIgnoreGSUB(false), mSVGInitialized(false), @@ -168,11 +166,6 @@ gfxFontEntry::~gfxFontEntry() MOZ_ASSERT(!mGrFaceInitialized); } -bool gfxFontEntry::IsSymbolFont() -{ - return mSymbolFont; -} - bool gfxFontEntry::TestCharacterMap(uint32_t aCh) { if (!mCharacterMap) { diff --git a/gfx/thebes/gfxFontEntry.h b/gfx/thebes/gfxFontEntry.h index 5afbee174711c..55e04a29b5f08 100644 --- a/gfx/thebes/gfxFontEntry.h +++ b/gfx/thebes/gfxFontEntry.h @@ -152,8 +152,6 @@ class gfxFontEntry { const hb_set_t* InputsForOpenTypeFeature(Script aScript, uint32_t aFeatureTag); - virtual bool IsSymbolFont(); - virtual bool HasFontTable(uint32_t aTableTag); inline bool HasGraphiteTables() { @@ -344,7 +342,6 @@ class gfxFontEntry { bool mIsDataUserFont : 1; // platform font entry (data) bool mIsLocalUserFont : 1; // platform font entry (local) bool mStandardFace : 1; - bool mSymbolFont : 1; bool mIgnoreGDEF : 1; bool mIgnoreGSUB : 1; bool mSVGInitialized : 1; @@ -765,6 +762,10 @@ class gfxFontFamily { return true; } + virtual bool IsSymbolFontFamily() const { + return false; + } + protected: // Protected destructor, to discourage deletion outside of Release(): virtual ~gfxFontFamily(); diff --git a/gfx/thebes/gfxGDIFontList.cpp b/gfx/thebes/gfxGDIFontList.cpp index b695fce25e260..b4076e6fd6de8 100644 --- a/gfx/thebes/gfxGDIFontList.cpp +++ b/gfx/thebes/gfxGDIFontList.cpp @@ -171,7 +171,6 @@ GDIFontEntry::ReadCMAP(FontInfoData *aFontInfoData) if (aFontInfoData && (charmap = GetCMAPFromFontInfo(aFontInfoData, mUVSOffset, symbolFont))) { - mSymbolFont = symbolFont; rv = NS_OK; } else { uint32_t kCMAP = TRUETYPE_TAG('c','m','a','p'); @@ -184,7 +183,6 @@ GDIFontEntry::ReadCMAP(FontInfoData *aFontInfoData) *charmap, mUVSOffset, unicodeFont, symbolFont); } - mSymbolFont = symbolFont; } mHasCmapTable = NS_SUCCEEDED(rv); @@ -213,14 +211,6 @@ GDIFontEntry::ReadCMAP(FontInfoData *aFontInfoData) return rv; } -bool -GDIFontEntry::IsSymbolFont() -{ - // initialize cmap first - HasCmapTable(); - return mSymbolFont; -} - gfxFont * GDIFontEntry::CreateFontInstance(const gfxFontStyle* aFontStyle, bool aNeedsBold) { diff --git a/gfx/thebes/gfxGDIFontList.h b/gfx/thebes/gfxGDIFontList.h index abd2c2b3d0366..d78bb168c3b0c 100644 --- a/gfx/thebes/gfxGDIFontList.h +++ b/gfx/thebes/gfxGDIFontList.h @@ -111,8 +111,6 @@ class GDIFontEntry : public gfxFontEntry nsresult ReadCMAP(FontInfoData *aFontInfoData = nullptr); - virtual bool IsSymbolFont(); - void FillLogFont(LOGFONTW *aLogFont, uint16_t aWeight, gfxFloat aSize); static gfxWindowsFontType DetermineFontType(const NEWTEXTMETRICW& metrics, @@ -299,6 +297,10 @@ class GDIFontFamily : public gfxFontFamily return false; } + bool IsSymbolFontFamily() const final { + return mCharset.test(SYMBOL_CHARSET); + } + private: static int CALLBACK FamilyAddStylesProc(const ENUMLOGFONTEXW *lpelfe, const NEWTEXTMETRICEXW *nmetrics, diff --git a/gfx/thebes/gfxPlatformFontList.cpp b/gfx/thebes/gfxPlatformFontList.cpp index 4f86980ef468e..c4d16e782047a 100644 --- a/gfx/thebes/gfxPlatformFontList.cpp +++ b/gfx/thebes/gfxPlatformFontList.cpp @@ -483,24 +483,8 @@ gfxPlatformFontList::GetFontList(nsIAtom *aLangGroup, { for (auto iter = mFontFamilies.Iter(); !iter.Done(); iter.Next()) { RefPtr& family = iter.Data(); - // use the first variation for now. This data should be the same - // for all the variations and should probably be moved up to - // the Family - gfxFontStyle style; - style.language = aLangGroup; - bool needsBold; - RefPtr fontEntry = family->FindFontForStyle(style, needsBold); - NS_ASSERTION(fontEntry, "couldn't find any font entry in family"); - if (!fontEntry) { - continue; - } - - /* skip symbol fonts */ - if (fontEntry->IsSymbolFont()) { - continue; - } - - if (family->SupportsLangGroup(aLangGroup) && + if (!family->IsSymbolFontFamily() && + family->SupportsLangGroup(aLangGroup) && family->MatchesGenericFamily(aGenericFamily)) { nsAutoString localizedFamilyName; family->LocalizedName(localizedFamilyName); From 82bdaf03b2ffbc1071074433819a053c459e83fa Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Mon, 11 Sep 2017 19:23:30 +0100 Subject: [PATCH 6/8] Bug 1395061 - patch 3 - Clean up vestigial code that existed to support setting the gfxFontEntry::mSymbolFont flag. r=jrmuizel --- gfx/thebes/gfxDWriteFontList.cpp | 12 +++--------- gfx/thebes/gfxFT2FontList.cpp | 5 +---- gfx/thebes/gfxFcPlatformFontList.cpp | 8 ++------ gfx/thebes/gfxFontEntry.cpp | 5 ++--- gfx/thebes/gfxFontEntry.h | 3 +-- gfx/thebes/gfxFontInfoLoader.h | 8 ++------ gfx/thebes/gfxFontUtils.cpp | 26 ++++---------------------- gfx/thebes/gfxFontUtils.h | 6 ++---- gfx/thebes/gfxGDIFontList.cpp | 12 +++--------- gfx/thebes/gfxGDIFontList.h | 5 +++-- gfx/thebes/gfxHarfBuzzShaper.cpp | 4 +--- gfx/thebes/gfxMacPlatformFontList.mm | 14 +++----------- 12 files changed, 27 insertions(+), 81 deletions(-) diff --git a/gfx/thebes/gfxDWriteFontList.cpp b/gfx/thebes/gfxDWriteFontList.cpp index 7a7c66ade2647..bf5025feccc2b 100644 --- a/gfx/thebes/gfxDWriteFontList.cpp +++ b/gfx/thebes/gfxDWriteFontList.cpp @@ -535,11 +535,9 @@ gfxDWriteFontEntry::ReadCMAP(FontInfoData *aFontInfoData) RefPtr charmap; nsresult rv; - bool symbolFont; if (aFontInfoData && (charmap = GetCMAPFromFontInfo(aFontInfoData, - mUVSOffset, - symbolFont))) { + mUVSOffset))) { rv = NS_OK; } else { uint32_t kCMAP = TRUETYPE_TAG('c','m','a','p'); @@ -547,14 +545,12 @@ gfxDWriteFontEntry::ReadCMAP(FontInfoData *aFontInfoData) AutoTable cmapTable(this, kCMAP); if (cmapTable) { - bool unicodeFont = false, symbolFont = false; // currently ignored uint32_t cmapLen; const uint8_t* cmapData = reinterpret_cast(hb_blob_get_data(cmapTable, &cmapLen)); rv = gfxFontUtils::ReadCMAP(cmapData, cmapLen, - *charmap, mUVSOffset, - unicodeFont, symbolFont); + *charmap, mUVSOffset); } else { rv = NS_ERROR_NOT_AVAILABLE; } @@ -1676,7 +1672,6 @@ DirectWriteFontInfo::LoadFontFamilyData(const nsAString& aFamilyName) if (SUCCEEDED(hr)) { bool cmapLoaded = false; - bool unicodeFont = false, symbolFont = false; RefPtr charmap = new gfxCharacterMap(); uint32_t offset; @@ -1684,10 +1679,9 @@ DirectWriteFontInfo::LoadFontFamilyData(const nsAString& aFamilyName) cmapSize > 0 && NS_SUCCEEDED( gfxFontUtils::ReadCMAP(cmapData, cmapSize, *charmap, - offset, unicodeFont, symbolFont))) { + offset))) { fontData.mCharacterMap = charmap; fontData.mUVSOffset = offset; - fontData.mSymbolFont = symbolFont; cmapLoaded = true; mLoadStats.cmaps++; } diff --git a/gfx/thebes/gfxFT2FontList.cpp b/gfx/thebes/gfxFT2FontList.cpp index e31b9970ccb46..cc4cdeaad9d68 100644 --- a/gfx/thebes/gfxFT2FontList.cpp +++ b/gfx/thebes/gfxFT2FontList.cpp @@ -484,11 +484,8 @@ FT2FontEntry::ReadCMAP(FontInfoData *aFontInfoData) nsresult rv = CopyFontTable(TTAG_cmap, buffer); if (NS_SUCCEEDED(rv)) { - bool unicodeFont; - bool symbolFont; rv = gfxFontUtils::ReadCMAP(buffer.Elements(), buffer.Length(), - *charmap, mUVSOffset, - unicodeFont, symbolFont); + *charmap, mUVSOffset); } if (NS_SUCCEEDED(rv) && !HasGraphiteTables()) { diff --git a/gfx/thebes/gfxFcPlatformFontList.cpp b/gfx/thebes/gfxFcPlatformFontList.cpp index 810514bdbc742..969772b16547d 100644 --- a/gfx/thebes/gfxFcPlatformFontList.cpp +++ b/gfx/thebes/gfxFcPlatformFontList.cpp @@ -335,11 +335,9 @@ gfxFontconfigFontEntry::ReadCMAP(FontInfoData *aFontInfoData) RefPtr charmap; nsresult rv; - bool symbolFont = false; // currently ignored if (aFontInfoData && (charmap = GetCMAPFromFontInfo(aFontInfoData, - mUVSOffset, - symbolFont))) { + mUVSOffset))) { rv = NS_OK; } else { uint32_t kCMAP = TRUETYPE_TAG('c','m','a','p'); @@ -347,14 +345,12 @@ gfxFontconfigFontEntry::ReadCMAP(FontInfoData *aFontInfoData) AutoTable cmapTable(this, kCMAP); if (cmapTable) { - bool unicodeFont = false; // currently ignored uint32_t cmapLen; const uint8_t* cmapData = reinterpret_cast(hb_blob_get_data(cmapTable, &cmapLen)); rv = gfxFontUtils::ReadCMAP(cmapData, cmapLen, - *charmap, mUVSOffset, - unicodeFont, symbolFont); + *charmap, mUVSOffset); } else { rv = NS_ERROR_NOT_AVAILABLE; } diff --git a/gfx/thebes/gfxFontEntry.cpp b/gfx/thebes/gfxFontEntry.cpp index 11c5ccbbf5117..006b92c2fa9af 100644 --- a/gfx/thebes/gfxFontEntry.cpp +++ b/gfx/thebes/gfxFontEntry.cpp @@ -595,14 +595,13 @@ gfxFontEntry::ShareFontTableAndGetBlob(uint32_t aTag, already_AddRefed gfxFontEntry::GetCMAPFromFontInfo(FontInfoData *aFontInfoData, - uint32_t& aUVSOffset, - bool& aSymbolFont) + uint32_t& aUVSOffset) { if (!aFontInfoData || !aFontInfoData->mLoadCmaps) { return nullptr; } - return aFontInfoData->GetCMAP(mName, aUVSOffset, aSymbolFont); + return aFontInfoData->GetCMAP(mName, aUVSOffset); } hb_blob_t * diff --git a/gfx/thebes/gfxFontEntry.h b/gfx/thebes/gfxFontEntry.h index 55e04a29b5f08..8c492952fa8ee 100644 --- a/gfx/thebes/gfxFontEntry.h +++ b/gfx/thebes/gfxFontEntry.h @@ -413,8 +413,7 @@ class gfxFontEntry { // lookup the cmap in cached font data virtual already_AddRefed GetCMAPFromFontInfo(FontInfoData *aFontInfoData, - uint32_t& aUVSOffset, - bool& aSymbolFont); + uint32_t& aUVSOffset); // helper for HasCharacter(), which is what client code should call virtual bool TestCharacterMap(uint32_t aCh); diff --git a/gfx/thebes/gfxFontInfoLoader.h b/gfx/thebes/gfxFontInfoLoader.h index 40b655aa5b6fb..2e27a29412a03 100644 --- a/gfx/thebes/gfxFontInfoLoader.h +++ b/gfx/thebes/gfxFontInfoLoader.h @@ -21,21 +21,19 @@ // data retrieved for a given face struct FontFaceData { - FontFaceData() : mUVSOffset(0), mSymbolFont(false) {} + FontFaceData() : mUVSOffset(0) {} FontFaceData(const FontFaceData& aFontFaceData) { mFullName = aFontFaceData.mFullName; mPostscriptName = aFontFaceData.mPostscriptName; mCharacterMap = aFontFaceData.mCharacterMap; mUVSOffset = aFontFaceData.mUVSOffset; - mSymbolFont = aFontFaceData.mSymbolFont; } nsString mFullName; nsString mPostscriptName; RefPtr mCharacterMap; uint32_t mUVSOffset; - bool mSymbolFont; }; // base class used to contain cached system-wide font info. @@ -78,8 +76,7 @@ class FontInfoData { // fetches cmap data for a particular font from cached font data virtual already_AddRefed GetCMAP(const nsAString& aFontName, - uint32_t& aUVSOffset, - bool& aSymbolFont) + uint32_t& aUVSOffset) { FontFaceData faceData; if (!mFontFaceData.Get(aFontName, &faceData) || @@ -88,7 +85,6 @@ class FontInfoData { } aUVSOffset = faceData.mUVSOffset; - aSymbolFont = faceData.mSymbolFont; RefPtr cmap = faceData.mCharacterMap; return cmap.forget(); } diff --git a/gfx/thebes/gfxFontUtils.cpp b/gfx/thebes/gfxFontUtils.cpp index 27682a77816a8..25e308717a62b 100644 --- a/gfx/thebes/gfxFontUtils.cpp +++ b/gfx/thebes/gfxFontUtils.cpp @@ -413,8 +413,7 @@ gfxFontUtils::ReadCMAPTableFormat14(const uint8_t *aBuf, uint32_t aLength, uint32_t gfxFontUtils::FindPreferredSubtable(const uint8_t *aBuf, uint32_t aBufLength, uint32_t *aTableOffset, - uint32_t *aUVSTableOffset, - bool *aSymbolEncoding) + uint32_t *aUVSTableOffset) { enum { OffsetVersion = 0, @@ -474,17 +473,14 @@ gfxFontUtils::FindPreferredSubtable(const uint8_t *aBuf, uint32_t aBufLength, if (isSymbol(platformID, encodingID)) { keepFormat = format; *aTableOffset = offset; - *aSymbolEncoding = true; break; } else if (format == 4 && acceptableFormat4(platformID, encodingID, keepFormat)) { keepFormat = format; *aTableOffset = offset; - *aSymbolEncoding = false; } else if ((format == 10 || format == 12 || format == 13) && acceptableUCS4Encoding(platformID, encodingID, keepFormat)) { keepFormat = format; *aTableOffset = offset; - *aSymbolEncoding = false; if (platformID > PLATFORM_ID_UNICODE || !aUVSTableOffset || *aUVSTableOffset) { break; // we don't want to try anything else when this format is available. } @@ -502,36 +498,23 @@ gfxFontUtils::FindPreferredSubtable(const uint8_t *aBuf, uint32_t aBufLength, nsresult gfxFontUtils::ReadCMAP(const uint8_t *aBuf, uint32_t aBufLength, gfxSparseBitSet& aCharacterMap, - uint32_t& aUVSOffset, - bool& aUnicodeFont, bool& aSymbolFont) + uint32_t& aUVSOffset) { uint32_t offset; - bool symbol; uint32_t format = FindPreferredSubtable(aBuf, aBufLength, - &offset, &aUVSOffset, &symbol); + &offset, &aUVSOffset); switch (format) { case 4: - if (symbol) { - aUnicodeFont = false; - aSymbolFont = true; - } else { - aUnicodeFont = true; - aSymbolFont = false; - } return ReadCMAPTableFormat4(aBuf + offset, aBufLength - offset, aCharacterMap); case 10: - aUnicodeFont = true; - aSymbolFont = false; return ReadCMAPTableFormat10(aBuf + offset, aBufLength - offset, aCharacterMap); case 12: case 13: - aUnicodeFont = true; - aSymbolFont = false; return ReadCMAPTableFormat12or13(aBuf + offset, aBufLength - offset, aCharacterMap); @@ -769,9 +752,8 @@ gfxFontUtils::MapCharToGlyph(const uint8_t *aCmapBuf, uint32_t aBufLength, uint32_t aUnicode, uint32_t aVarSelector) { uint32_t offset, uvsOffset; - bool symbol; uint32_t format = FindPreferredSubtable(aCmapBuf, aBufLength, &offset, - &uvsOffset, &symbol); + &uvsOffset); uint32_t gid; switch (format) { diff --git a/gfx/thebes/gfxFontUtils.h b/gfx/thebes/gfxFontUtils.h index d5c7c3ae02dd6..59e94dbaccdbf 100644 --- a/gfx/thebes/gfxFontUtils.h +++ b/gfx/thebes/gfxFontUtils.h @@ -794,14 +794,12 @@ class gfxFontUtils { static uint32_t FindPreferredSubtable(const uint8_t *aBuf, uint32_t aBufLength, - uint32_t *aTableOffset, uint32_t *aUVSTableOffset, - bool *aSymbolEncoding); + uint32_t *aTableOffset, uint32_t *aUVSTableOffset); static nsresult ReadCMAP(const uint8_t *aBuf, uint32_t aBufLength, gfxSparseBitSet& aCharacterMap, - uint32_t& aUVSOffset, - bool& aUnicodeFont, bool& aSymbolFont); + uint32_t& aUVSOffset); static uint32_t MapCharToGlyphFormat4(const uint8_t *aBuf, char16_t aCh); diff --git a/gfx/thebes/gfxGDIFontList.cpp b/gfx/thebes/gfxGDIFontList.cpp index b4076e6fd6de8..23803cf90128c 100644 --- a/gfx/thebes/gfxGDIFontList.cpp +++ b/gfx/thebes/gfxGDIFontList.cpp @@ -166,11 +166,9 @@ GDIFontEntry::ReadCMAP(FontInfoData *aFontInfoData) RefPtr charmap; nsresult rv; - bool unicodeFont = false, symbolFont = false; if (aFontInfoData && (charmap = GetCMAPFromFontInfo(aFontInfoData, - mUVSOffset, - symbolFont))) { + mUVSOffset))) { rv = NS_OK; } else { uint32_t kCMAP = TRUETYPE_TAG('c','m','a','p'); @@ -180,8 +178,7 @@ GDIFontEntry::ReadCMAP(FontInfoData *aFontInfoData) if (NS_SUCCEEDED(rv)) { rv = gfxFontUtils::ReadCMAP(cmap.Elements(), cmap.Length(), - *charmap, mUVSOffset, - unicodeFont, symbolFont); + *charmap, mUVSOffset); } } @@ -1110,17 +1107,14 @@ int CALLBACK GDIFontInfo::EnumerateFontsForFamily( cmapData.SetLength(cmapSize, fallible)) { ::GetFontData(hdc, kCMAP, 0, cmapData.Elements(), cmapSize); bool cmapLoaded = false; - bool unicodeFont = false, symbolFont = false; RefPtr charmap = new gfxCharacterMap(); uint32_t offset; if (NS_SUCCEEDED(gfxFontUtils::ReadCMAP(cmapData.Elements(), cmapSize, *charmap, - offset, unicodeFont, - symbolFont))) { + offset))) { fontData.mCharacterMap = charmap; fontData.mUVSOffset = offset; - fontData.mSymbolFont = symbolFont; cmapLoaded = true; famData->mFontInfo.mLoadStats.cmaps++; } diff --git a/gfx/thebes/gfxGDIFontList.h b/gfx/thebes/gfxGDIFontList.h index d78bb168c3b0c..935f78a18bb19 100644 --- a/gfx/thebes/gfxGDIFontList.h +++ b/gfx/thebes/gfxGDIFontList.h @@ -109,7 +109,7 @@ class GDIFontEntry : public gfxFontEntry public: LPLOGFONTW GetLogFont() { return &mLogFont; } - nsresult ReadCMAP(FontInfoData *aFontInfoData = nullptr); + nsresult ReadCMAP(FontInfoData *aFontInfoData = nullptr) override; void FillLogFont(LOGFONTW *aLogFont, uint16_t aWeight, gfxFloat aSize); @@ -194,7 +194,8 @@ class GDIFontEntry : public gfxFontEntry void InitLogFont(const nsAString& aName, gfxWindowsFontType aFontType); - virtual gfxFont *CreateFontInstance(const gfxFontStyle *aFontStyle, bool aNeedsBold); + virtual gfxFont* CreateFontInstance(const gfxFontStyle *aFontStyle, + bool aNeedsBold) override; virtual nsresult CopyFontTable(uint32_t aTableTag, nsTArray& aBuffer) override; diff --git a/gfx/thebes/gfxHarfBuzzShaper.cpp b/gfx/thebes/gfxHarfBuzzShaper.cpp index 607c87c569ec3..5ea9db7c3915d 100644 --- a/gfx/thebes/gfxHarfBuzzShaper.cpp +++ b/gfx/thebes/gfxHarfBuzzShaper.cpp @@ -1275,11 +1275,9 @@ gfxHarfBuzzShaper::Initialize() } uint32_t len; const uint8_t* data = (const uint8_t*)hb_blob_get_data(mCmapTable, &len); - bool symbol; mCmapFormat = gfxFontUtils:: FindPreferredSubtable(data, len, - &mSubtableOffset, &mUVSTableOffset, - &symbol); + &mSubtableOffset, &mUVSTableOffset); if (mCmapFormat <= 0) { return false; } diff --git a/gfx/thebes/gfxMacPlatformFontList.mm b/gfx/thebes/gfxMacPlatformFontList.mm index f57b77f89f6f7..7b3fd9eebb528 100644 --- a/gfx/thebes/gfxMacPlatformFontList.mm +++ b/gfx/thebes/gfxMacPlatformFontList.mm @@ -152,11 +152,9 @@ static void GetStringForNSString(const NSString *aSrc, nsAString& aDist) RefPtr charmap; nsresult rv; - bool symbolFont = false; // currently ignored if (aFontInfoData && (charmap = GetCMAPFromFontInfo(aFontInfoData, - mUVSOffset, - symbolFont))) { + mUVSOffset))) { rv = NS_OK; } else { uint32_t kCMAP = TRUETYPE_TAG('c','m','a','p'); @@ -164,14 +162,12 @@ static void GetStringForNSString(const NSString *aSrc, nsAString& aDist) AutoTable cmapTable(this, kCMAP); if (cmapTable) { - bool unicodeFont = false; // currently ignored uint32_t cmapLen; const uint8_t* cmapData = reinterpret_cast(hb_blob_get_data(cmapTable, &cmapLen)); rv = gfxFontUtils::ReadCMAP(cmapData, cmapLen, - *charmap, mUVSOffset, - unicodeFont, symbolFont); + *charmap, mUVSOffset); } else { rv = NS_ERROR_NOT_AVAILABLE; } @@ -1479,16 +1475,12 @@ nsAutoString fontName(reinterpret_cast(buffer.Elements()), uint32_t cmapLen = CFDataGetLength(cmapTable); RefPtr charmap = new gfxCharacterMap(); uint32_t offset; - bool unicodeFont = false; // ignored - bool symbolFont = false; nsresult rv; - rv = gfxFontUtils::ReadCMAP(cmapData, cmapLen, *charmap, offset, - unicodeFont, symbolFont); + rv = gfxFontUtils::ReadCMAP(cmapData, cmapLen, *charmap, offset); if (NS_SUCCEEDED(rv)) { fontData.mCharacterMap = charmap; fontData.mUVSOffset = offset; - fontData.mSymbolFont = symbolFont; mLoadStats.cmaps++; } CFRelease(cmapTable); From 162dd2e4e13584c4ea76f7ef6d12179548b0c2b9 Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Mon, 11 Sep 2017 19:24:01 +0100 Subject: [PATCH 7/8] Bug 1395061 - patch 4 - Refactor checks in the gfxPlatformFontList::GetFontList loop to use a single virtual method call instead of three separate calls. r=jrmuizel --- gfx/thebes/gfxDWriteFontList.h | 10 ++++++++-- gfx/thebes/gfxFcPlatformFontList.h | 8 +++++++- gfx/thebes/gfxFontEntry.h | 17 ++++++++--------- gfx/thebes/gfxGDIFontList.h | 29 ++++++++++++++++++++--------- gfx/thebes/gfxPlatformFontList.cpp | 4 +--- 5 files changed, 44 insertions(+), 24 deletions(-) diff --git a/gfx/thebes/gfxDWriteFontList.h b/gfx/thebes/gfxDWriteFontList.h index b2bfcaa3266ba..35afad8c77ac2 100644 --- a/gfx/thebes/gfxDWriteFontList.h +++ b/gfx/thebes/gfxDWriteFontList.h @@ -55,14 +55,20 @@ class gfxDWriteFontFamily : public gfxFontFamily void SetForceGDIClassic(bool aForce) { mForceGDIClassic = aForce; } - bool IsSymbolFontFamily() const final; - void AddSizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf, FontListSizes* aSizes) const final; void AddSizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf, FontListSizes* aSizes) const final; + bool FilterForFontList(nsIAtom* aLangGroup, + const nsACString& aGeneric) const final { + return !IsSymbolFontFamily(); + } + protected: + // helper for FilterForFontList + bool IsSymbolFontFamily() const; + /** This font family's directwrite fontfamily object */ RefPtr mDWFamily; bool mForceGDIClassic; diff --git a/gfx/thebes/gfxFcPlatformFontList.h b/gfx/thebes/gfxFcPlatformFontList.h index 21c4cfb78a449..83690ea77b342 100644 --- a/gfx/thebes/gfxFcPlatformFontList.h +++ b/gfx/thebes/gfxFcPlatformFontList.h @@ -199,11 +199,17 @@ class gfxFontconfigFontFamily : public gfxFontFamily { bool& aNeedsSyntheticBold, bool aIgnoreSizeTolerance) override; - bool SupportsLangGroup(nsIAtom *aLangGroup) const override; + bool FilterForFontList(nsIAtom* aLangGroup, + const nsACString& aGeneric) const final { + return SupportsLangGroup(aLangGroup); + } protected: virtual ~gfxFontconfigFontFamily(); + // helper for FilterForFontList + bool SupportsLangGroup(nsIAtom *aLangGroup) const; + nsTArray > mFontPatterns; bool mContainsAppFonts; diff --git a/gfx/thebes/gfxFontEntry.h b/gfx/thebes/gfxFontEntry.h index 8c492952fa8ee..7bd28f0808e33 100644 --- a/gfx/thebes/gfxFontEntry.h +++ b/gfx/thebes/gfxFontEntry.h @@ -753,18 +753,17 @@ class gfxFontFamily { mSkipDefaultFeatureSpaceCheck = aSkipCheck; } - virtual bool MatchesGenericFamily(const nsACString& aGeneric) const { + // Check whether this family is appropriate to include in the Preferences + // font list for the given langGroup and CSS generic, if the platform lets + // us determine this. + // Return true if the family should be included in the list, false to omit. + // Default implementation returns true for everything, so no filtering + // will occur; individual platforms may override. + virtual bool FilterForFontList(nsIAtom* aLangGroup, + const nsACString& aGeneric) const { return true; } - virtual bool SupportsLangGroup(nsIAtom *aLangGroup) const { - return true; - } - - virtual bool IsSymbolFontFamily() const { - return false; - } - protected: // Protected destructor, to discourage deletion outside of Release(): virtual ~gfxFontFamily(); diff --git a/gfx/thebes/gfxGDIFontList.h b/gfx/thebes/gfxGDIFontList.h index 935f78a18bb19..8ae67df7c9ded 100644 --- a/gfx/thebes/gfxGDIFontList.h +++ b/gfx/thebes/gfxGDIFontList.h @@ -220,10 +220,22 @@ class GDIFontFamily : public gfxFontFamily virtual void FindStyleVariations(FontInfoData *aFontInfoData = nullptr); - uint8_t mWindowsFamily; - uint8_t mWindowsPitch; + bool FilterForFontList(nsIAtom* aLangGroup, + const nsACString& aGeneric) const final { + return !IsSymbolFontFamily() && + SupportsLangGroup(aLangGroup) && + MatchesGenericFamily(aGeneric); + } + +protected: + friend class gfxGDIFontList; + + // helpers for FilterForFontList + bool IsSymbolFontFamily() const { + return mCharset.test(SYMBOL_CHARSET); + } - virtual bool MatchesGenericFamily(const nsACString& aGeneric) const { + bool MatchesGenericFamily(const nsACString& aGeneric) const { if (aGeneric.IsEmpty()) { return true; } @@ -259,9 +271,7 @@ class GDIFontFamily : public gfxFontFamily return false; } - gfxSparseBitSet mCharset; - - virtual bool SupportsLangGroup(nsIAtom* aLangGroup) const override { + bool SupportsLangGroup(nsIAtom* aLangGroup) const { if (!aLangGroup || aLangGroup == nsGkAtoms::Unicode) { return true; } @@ -298,9 +308,10 @@ class GDIFontFamily : public gfxFontFamily return false; } - bool IsSymbolFontFamily() const final { - return mCharset.test(SYMBOL_CHARSET); - } + uint8_t mWindowsFamily; + uint8_t mWindowsPitch; + + gfxSparseBitSet mCharset; private: static int CALLBACK FamilyAddStylesProc(const ENUMLOGFONTEXW *lpelfe, diff --git a/gfx/thebes/gfxPlatformFontList.cpp b/gfx/thebes/gfxPlatformFontList.cpp index c4d16e782047a..c147ca62012e3 100644 --- a/gfx/thebes/gfxPlatformFontList.cpp +++ b/gfx/thebes/gfxPlatformFontList.cpp @@ -483,9 +483,7 @@ gfxPlatformFontList::GetFontList(nsIAtom *aLangGroup, { for (auto iter = mFontFamilies.Iter(); !iter.Done(); iter.Next()) { RefPtr& family = iter.Data(); - if (!family->IsSymbolFontFamily() && - family->SupportsLangGroup(aLangGroup) && - family->MatchesGenericFamily(aGenericFamily)) { + if (family->FilterForFontList(aLangGroup, aGenericFamily)) { nsAutoString localizedFamilyName; family->LocalizedName(localizedFamilyName); aListOfFonts.AppendElement(localizedFamilyName); From f2b7e3f5a3083989a5a7f1c9cd64e473f4f3d695 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 10 Sep 2017 16:26:28 +0200 Subject: [PATCH 8/8] Bug 1398448: Always insert async when reconstructing ancestors to avoid pathological frame construction cases. r=bz MozReview-Commit-ID: 5ARTWW9dt7X --- layout/base/PresShell.cpp | 1 - layout/base/nsCSSFrameConstructor.cpp | 106 ++++++++++++-------------- layout/base/nsCSSFrameConstructor.h | 28 +++---- 3 files changed, 64 insertions(+), 71 deletions(-) diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index f6e9335be93ff..c7b3c201d3ba4 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -4493,7 +4493,6 @@ PresShell::ContentRemoved(nsIDocument *aDocument, } mFrameConstructor->ContentRemoved(aMaybeContainer, aChild, oldNextSibling, - nsCSSFrameConstructor::InsertionKind::Async, nsCSSFrameConstructor::REMOVE_CONTENT); if (aChild->NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE) { diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index bd3d406f664f7..03286573cb577 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -7526,8 +7526,7 @@ nsCSSFrameConstructor::GetRangeInsertionPoint(nsIContent* aContainer, bool nsCSSFrameConstructor::MaybeRecreateForFrameset(nsIFrame* aParentFrame, nsIContent* aStartChild, - nsIContent* aEndChild, - InsertionKind aInsertionKind) + nsIContent* aEndChild) { if (aParentFrame->IsFrameSetFrame()) { // Check whether we have any kids we care about. @@ -7536,7 +7535,8 @@ nsCSSFrameConstructor::MaybeRecreateForFrameset(nsIFrame* aParentFrame, cur = cur->GetNextSibling()) { if (IsSpecialFramesetChild(cur)) { // Just reframe the parent, since framesets are weird like that. - RecreateFramesForContent(aParentFrame->GetContent(), aInsertionKind); + RecreateFramesForContent(aParentFrame->GetContent(), + InsertionKind::Async); return true; } } @@ -7592,7 +7592,8 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, { MOZ_ASSERT(!aProvidedTreeMatchContext || aInsertionKind == InsertionKind::Sync); - MOZ_ASSERT(aInsertionKind == InsertionKind::Sync || !RestyleManager()->IsInStyleRefresh()); + MOZ_ASSERT(aInsertionKind == InsertionKind::Sync || + !RestyleManager()->IsInStyleRefresh()); AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC); NS_PRECONDITION(mUpdateCount != 0, @@ -7695,7 +7696,7 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, } LAYOUT_PHASE_TEMP_EXIT(); - if (MaybeRecreateForFrameset(parentFrame, aFirstNewContent, nullptr, aInsertionKind)) { + if (MaybeRecreateForFrameset(parentFrame, aFirstNewContent, nullptr)) { LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -7829,7 +7830,7 @@ nsCSSFrameConstructor::ContentAppended(nsIContent* aContainer, // appending, so let WipeContainingBlock know that. LAYOUT_PHASE_TEMP_EXIT(); if (WipeContainingBlock(state, containingBlock, parentFrame, items, - true, prevSibling, aInsertionKind)) { + true, prevSibling)) { LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -8013,7 +8014,8 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, { MOZ_ASSERT(!aProvidedTreeMatchContext || aInsertionKind == InsertionKind::Sync); - MOZ_ASSERT(aInsertionKind == InsertionKind::Sync || !RestyleManager()->IsInStyleRefresh()); + MOZ_ASSERT(aInsertionKind == InsertionKind::Sync || + !RestyleManager()->IsInStyleRefresh()); AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC); NS_PRECONDITION(mUpdateCount != 0, @@ -8212,7 +8214,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, LayoutFrameType frameType = insertion.mParentFrame->Type(); LAYOUT_PHASE_TEMP_EXIT(); - if (MaybeRecreateForFrameset(insertion.mParentFrame, aStartChild, aEndChild, aInsertionKind)) { + if (MaybeRecreateForFrameset(insertion.mParentFrame, aStartChild, aEndChild)) { LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -8410,7 +8412,7 @@ nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aContainer, // appending, so let WipeContainingBlock know that. LAYOUT_PHASE_TEMP_EXIT(); if (WipeContainingBlock(state, containingBlock, insertion.mParentFrame, items, - isAppend, prevSibling, aInsertionKind)) { + isAppend, prevSibling)) { LAYOUT_PHASE_TEMP_REENTER(); return; } @@ -8603,18 +8605,12 @@ bool nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, nsIContent* aChild, nsIContent* aOldNextSibling, - InsertionKind aInsertionKind, RemoveFlags aFlags) { MOZ_ASSERT(aChild); AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC); NS_PRECONDITION(mUpdateCount != 0, "Should be in an update while destroying frames"); - // We only want to recreate sync if we're already in frame construction, that - // is, recreate async for XBL DOM changes, and normal content removals. - MOZ_ASSERT(aFlags == REMOVE_FOR_RECONSTRUCTION || - aInsertionKind == InsertionKind::Async); - nsPresContext* presContext = mPresShell->GetPresContext(); MOZ_ASSERT(presContext, "Our presShell should have a valid presContext"); @@ -8664,7 +8660,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, // XXXmats Can we recreate frames only for the ::after/::before content? // XXX Perhaps even only those that belong to the aChild sub-tree? LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(ancestor, aInsertionKind); + RecreateFramesForContent(ancestor, InsertionKind::Async); LAYOUT_PHASE_TEMP_REENTER(); return true; } @@ -8674,8 +8670,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, if (c->GetPrimaryFrame() || GetDisplayContentsStyleFor(c)) { LAYOUT_PHASE_TEMP_EXIT(); bool didReconstruct = - ContentRemoved(aChild, c, nullptr, aInsertionKind, - REMOVE_FOR_RECONSTRUCTION); + ContentRemoved(aChild, c, nullptr, REMOVE_FOR_RECONSTRUCTION); LAYOUT_PHASE_TEMP_REENTER(); if (didReconstruct) { return true; @@ -8725,7 +8720,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, // XXXsmaug This is super unefficient! nsIContent* bindingParent = aContainer->GetBindingParent(); LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(bindingParent, aInsertionKind); + RecreateFramesForContent(bindingParent, InsertionKind::Async); LAYOUT_PHASE_TEMP_REENTER(); return true; } @@ -8735,7 +8730,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, // See whether we need to remove more than just childFrame LAYOUT_PHASE_TEMP_EXIT(); - if (MaybeRecreateContainerForFrameRemoval(childFrame, aInsertionKind)) { + if (MaybeRecreateContainerForFrameRemoval(childFrame)) { LAYOUT_PHASE_TEMP_REENTER(); return true; } @@ -8749,7 +8744,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, IsSpecialFramesetChild(aChild)) { // Just reframe the parent, since framesets are weird like that. LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(parentFrame->GetContent(), aInsertionKind); + RecreateFramesForContent(parentFrame->GetContent(), InsertionKind::Async); LAYOUT_PHASE_TEMP_REENTER(); return true; } @@ -8762,7 +8757,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, : parentFrame; if (possibleMathMLAncestor->IsFrameOfType(nsIFrame::eMathML)) { LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(parentFrame->GetContent(), aInsertionKind); + RecreateFramesForContent(parentFrame->GetContent(), InsertionKind::Async); LAYOUT_PHASE_TEMP_REENTER(); return true; } @@ -8777,7 +8772,8 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, aChild == AnyKidsNeedBlockParent(parentFrame->PrincipalChildList().FirstChild()) && !AnyKidsNeedBlockParent(childFrame->GetNextSibling())) { LAYOUT_PHASE_TEMP_EXIT(); - RecreateFramesForContent(grandparentFrame->GetContent(), aInsertionKind); + RecreateFramesForContent(grandparentFrame->GetContent(), + InsertionKind::Async); LAYOUT_PHASE_TEMP_REENTER(); return true; } @@ -9709,9 +9705,7 @@ FindPreviousNonWhitespaceSibling(nsIFrame* aFrame) } bool -nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( - nsIFrame* aFrame, - InsertionKind aInsertionKind) +nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame) { NS_PRECONDITION(aFrame, "Must have a frame"); NS_PRECONDITION(aFrame->GetParent(), "Frame shouldn't be root"); @@ -9730,14 +9724,15 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( } #endif - ReframeContainingBlock(aFrame, aInsertionKind); + ReframeContainingBlock(aFrame); return true; } nsContainerFrame* insertionFrame = aFrame->GetContentInsertionFrame(); if (insertionFrame && insertionFrame->IsLegendFrame() && aFrame->GetParent()->IsFieldSetFrame()) { - RecreateFramesForContent(aFrame->GetParent()->GetContent(), aInsertionKind); + RecreateFramesForContent(aFrame->GetParent()->GetContent(), + InsertionKind::Async); return true; } @@ -9761,7 +9756,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( // When removing a summary, we should reframe the parent details frame to // ensure that another summary is used or the default summary is // generated. - RecreateFramesForContent(parent->GetContent(), aInsertionKind); + RecreateFramesForContent(parent->GetContent(), InsertionKind::Async); return true; } } @@ -9784,7 +9779,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( // Similar if we're a table-caption. (inFlowFrame->IsTableCaption() && parent->GetChildList(nsIFrame::kCaptionList).FirstChild() == inFlowFrame)) { - RecreateFramesForContent(parent->GetContent(), aInsertionKind); + RecreateFramesForContent(parent->GetContent(), InsertionKind::Async); return true; } } @@ -9813,7 +9808,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( #endif // Good enough to recreate frames for aFrame's parent's content; even if // aFrame's parent is a pseudo, that'll be the right content node. - RecreateFramesForContent(parent->GetContent(), aInsertionKind); + RecreateFramesForContent(parent->GetContent(), InsertionKind::Async); return true; } } @@ -9829,7 +9824,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( // frames may be constructed or destroyed accordingly. // 2. The type of the first child of a ruby frame determines // whether a pseudo ruby base container should exist. - RecreateFramesForContent(parent->GetContent(), aInsertionKind); + RecreateFramesForContent(parent->GetContent(), InsertionKind::Async); return true; } @@ -9852,7 +9847,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( } #endif // DEBUG // Recreate frames for the flex container (the removed frame's parent) - RecreateFramesForContent(parent->GetContent(), aInsertionKind); + RecreateFramesForContent(parent->GetContent(), InsertionKind::Async); return true; } @@ -9870,7 +9865,8 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( } #endif // DEBUG // Recreate frames for the flex container (the removed frame's grandparent) - RecreateFramesForContent(parent->GetParent()->GetContent(), aInsertionKind); + RecreateFramesForContent(parent->GetParent()->GetContent(), + InsertionKind::Async); return true; } @@ -9878,7 +9874,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( if (aFrame->IsPopupSetFrame()) { nsIRootBox* rootBox = nsIRootBox::GetRootBox(mPresShell); if (rootBox && rootBox->GetPopupSetFrame() == aFrame) { - ReconstructDocElementHierarchy(aInsertionKind); + ReconstructDocElementHierarchy(InsertionKind::Async); return true; } } @@ -9890,7 +9886,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( !inFlowFrame->GetNextSibling() && ((parent->GetPrevContinuation() && !parent->GetPrevInFlow()) || (parent->GetNextContinuation() && !parent->GetNextInFlow()))) { - RecreateFramesForContent(parent->GetContent(), aInsertionKind); + RecreateFramesForContent(parent->GetContent(), InsertionKind::Async); return true; } @@ -9926,7 +9922,7 @@ nsCSSFrameConstructor::MaybeRecreateContainerForFrameRemoval( } #endif - ReframeContainingBlock(parent, aInsertionKind); + ReframeContainingBlock(parent); return true; } @@ -9985,7 +9981,7 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, nsIFrame* nonGeneratedAncestor = nsLayoutUtils::GetNonGeneratedAncestor(frame); if (nonGeneratedAncestor->GetContent() != aContent) { return RecreateFramesForContent(nonGeneratedAncestor->GetContent(), - aInsertionKind); + InsertionKind::Async); } if (frame->GetStateBits() & NS_FRAME_ANONYMOUSCONTENTCREATOR_CONTENT) { @@ -10005,7 +10001,8 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, if (!ancestor->IsSVGUseFrame()) { NS_ASSERTION(aContent->IsInNativeAnonymousSubtree(), "Why is NS_FRAME_ANONYMOUSCONTENTCREATOR_CONTENT set?"); - return RecreateFramesForContent(ancestor->GetContent(), aInsertionKind); + return RecreateFramesForContent(ancestor->GetContent(), + InsertionKind::Async); } } @@ -10016,11 +10013,11 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, // with native anonymous content from the editor. if (parent && parent->IsLeaf() && parentContent && parentContent != aContent) { - return RecreateFramesForContent(parentContent, aInsertionKind); + return RecreateFramesForContent(parentContent, InsertionKind::Async); } } - if (frame && MaybeRecreateContainerForFrameRemoval(frame, aInsertionKind)) { + if (frame && MaybeRecreateContainerForFrameRemoval(frame)) { return; } @@ -10039,7 +10036,7 @@ nsCSSFrameConstructor::RecreateFramesForContent(nsIContent* aContent, nsIContent* nextSibling = aContent->IsRootOfAnonymousSubtree() ? nullptr : aContent->GetNextSibling(); bool didReconstruct = - ContentRemoved(container, aContent, nextSibling, aInsertionKind, + ContentRemoved(container, aContent, nextSibling, REMOVE_FOR_RECONSTRUCTION); if (!didReconstruct) { @@ -10079,7 +10076,6 @@ nsCSSFrameConstructor::DestroyFramesFor(Element* aElement) return ContentRemoved(aElement->GetParent(), aElement, nextSibling, - InsertionKind::Async, REMOVE_FOR_RECONSTRUCTION); } @@ -12759,8 +12755,7 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, nsIFrame* aFrame, FrameConstructionItemList& aItems, bool aIsAppend, - nsIFrame* aPrevSibling, - InsertionKind aInsertionKind) + nsIFrame* aPrevSibling) { if (aItems.IsEmpty()) { return false; @@ -12774,7 +12769,7 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, if (aFrame->IsXULBoxFrame() && !(aFrame->GetStateBits() & NS_STATE_BOX_WRAPS_KIDS_IN_BLOCK) && aItems.AnyItemsNeedBlockParent()) { - RecreateFramesForContent(aFrame->GetContent(), aInsertionKind); + RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async); return true; } @@ -12793,7 +12788,7 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, const bool isWebkitBox = IsFlexContainerForLegacyBox(aFrame); if (aPrevSibling && IsAnonymousFlexOrGridItem(aPrevSibling) && iter.item().NeedsAnonFlexOrGridItem(aState, isWebkitBox)) { - RecreateFramesForContent(aFrame->GetContent(), aInsertionKind); + RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async); return true; } @@ -12804,7 +12799,7 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, iter.SetToEnd(); iter.Prev(); if (iter.item().NeedsAnonFlexOrGridItem(aState, isWebkitBox)) { - RecreateFramesForContent(aFrame->GetContent(), aInsertionKind); + RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async); return true; } } @@ -12832,7 +12827,7 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, if (!iter.SkipItemsThatNeedAnonFlexOrGridItem(aState, isWebkitBox)) { // We hit something that _doesn't_ need an anonymous flex item! // Rebuild the flex container to bust it out. - RecreateFramesForContent(containerFrame->GetContent(), aInsertionKind); + RecreateFramesForContent(containerFrame->GetContent(), InsertionKind::Async); return true; } @@ -12855,7 +12850,7 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, // We want to optimize it better, and avoid reframing as much as // possible. But given the cases above, and the fact that a ruby // usually won't be very large, it should be fine to reframe it. - RecreateFramesForContent(aFrame->GetContent(), aInsertionKind); + RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async); return true; } @@ -13021,7 +13016,7 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, if (!aItems.AllWantParentType(parentType)) { // Reframing aFrame->GetContent() is good enough, since the content of // table pseudo-frames is the ancestor content. - RecreateFramesForContent(aFrame->GetContent(), aInsertionKind); + RecreateFramesForContent(aFrame->GetContent(), InsertionKind::Async); return true; } } @@ -13109,13 +13104,12 @@ nsCSSFrameConstructor::WipeContainingBlock(nsFrameConstructorState& aState, static_cast(blockContent)); } #endif - RecreateFramesForContent(blockContent, aInsertionKind); + RecreateFramesForContent(blockContent, InsertionKind::Async); return true; } void -nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame* aFrame, - InsertionKind aInsertionKind) +nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame* aFrame) { #ifdef DEBUG @@ -13157,14 +13151,14 @@ nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame* aFrame, printf(" ==> blockContent=%p\n", static_cast(blockContent)); } #endif - RecreateFramesForContent(blockContent->AsElement(), aInsertionKind); + RecreateFramesForContent(blockContent->AsElement(), InsertionKind::Async); return; } } // If we get here, we're screwed! RecreateFramesForContent(mPresShell->GetDocument()->GetRootElement(), - aInsertionKind); + InsertionKind::Async); } void diff --git a/layout/base/nsCSSFrameConstructor.h b/layout/base/nsCSSFrameConstructor.h index be6402c57c059..84d436eced1d1 100644 --- a/layout/base/nsCSSFrameConstructor.h +++ b/layout/base/nsCSSFrameConstructor.h @@ -79,9 +79,14 @@ class nsCSSFrameConstructor final : public nsFrameManager /** * Whether insertion should be done synchronously or asynchronously. * - * Generally, insertion is synchronous if we're reconstructing something from - * frame construction/reconstruction, and async if we're removing stuff, like - * from ContentRemoved. + * Generally, insertion is synchronous if we're entering frame construction + * from restyle processing, and async if we're removing stuff, or need to + * reconstruct some ancestor. + * + * Note that constructing async from frame construction will post a restyle + * event, but won't need another whole refresh driver tick to go in. Instead + * change hint processing will keep going as long as there are changes in the + * queue. */ enum class InsertionKind { @@ -177,8 +182,7 @@ class nsCSSFrameConstructor final : public nsFrameManager // Returns true if parent was recreated due to frameset child, false otherwise. bool MaybeRecreateForFrameset(nsIFrame* aParentFrame, nsIContent* aStartChild, - nsIContent* aEndChild, - InsertionKind); + nsIContent* aEndChild); /** * For each child in the aStartChild/aEndChild range, calls @@ -300,14 +304,12 @@ class nsCSSFrameConstructor final : public nsFrameManager * * The return value indicates whether this "reconstruct an ancestor" action * took place. If true is returned, that means that the frame subtree rooted - * at some ancestor of aChild's frame was destroyed and either has been - * reconstructed or will be reconstructed async, depending on the value of - * aInsertionKind. + * at some ancestor of aChild's frame was destroyed and will be reconstructed + * async. */ bool ContentRemoved(nsIContent* aContainer, nsIContent* aChild, nsIContent* aOldNextSibling, - InsertionKind aInsertionKind, RemoveFlags aFlags); void CharacterDataChanged(nsIContent* aContent, @@ -1875,8 +1877,7 @@ class nsCSSFrameConstructor final : public nsFrameManager // indicates whether this happened. aFrame must be the result of a // GetPrimaryFrame() call on a content node (which means its parent is also // not null). - bool MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame, - InsertionKind aInsertionKind); + bool MaybeRecreateContainerForFrameRemoval(nsIFrame* aFrame); nsIFrame* CreateContinuingOuterTableFrame(nsIPresShell* aPresShell, nsPresContext* aPresContext, @@ -1998,10 +1999,9 @@ class nsCSSFrameConstructor final : public nsFrameManager nsIFrame* aFrame, FrameConstructionItemList& aItems, bool aIsAppend, - nsIFrame* aPrevSibling, - InsertionKind); + nsIFrame* aPrevSibling); - void ReframeContainingBlock(nsIFrame* aFrame, InsertionKind aInsertionKind); + void ReframeContainingBlock(nsIFrame* aFrame); //----------------------------------------