From 8f9744fe25e8e2ef9ac785c881da4fbdaba03f9b Mon Sep 17 00:00:00 2001 From: Kyle Machulis Date: Mon, 23 Jul 2018 10:11:52 +0000 Subject: [PATCH 01/26] Bug 1475407 - Add ReferrerPolicy header to nsDocShellLoadInfo r=baku Differential Revision: https://phabricator.services.mozilla.com/D2277 --HG-- extra : moz-landing-system : lando --- docshell/base/nsDocShellLoadInfo.h | 1 + 1 file changed, 1 insertion(+) diff --git a/docshell/base/nsDocShellLoadInfo.h b/docshell/base/nsDocShellLoadInfo.h index fd7cb22ca897..720b4aba5c40 100644 --- a/docshell/base/nsDocShellLoadInfo.h +++ b/docshell/base/nsDocShellLoadInfo.h @@ -11,6 +11,7 @@ #include "nsCOMPtr.h" #include "nsString.h" #include "nsDocShellLoadTypes.h" +#include "mozilla/net/ReferrerPolicy.h" class nsIInputStream; class nsISHEntry; From 229b0a95d4b2197009c529d354a0e119d24d9cc3 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Wed, 18 Jul 2018 15:23:12 -0500 Subject: [PATCH 02/26] Bug 1465473: Disable RTCP bundle filtering, because webrtc.org can do that for us. r=mjf MozReview-Commit-ID: EptqgMeMcoT --HG-- extra : rebase_source : 3a765a0f0cfb01a83bff0e8e1d8ae5b3262e80af --- .../gtest/mediapipeline_unittest.cpp | 107 +----------------- .../src/mediapipeline/MediaPipeline.cpp | 12 +- .../src/mediapipeline/MediaPipelineFilter.cpp | 28 ----- .../src/mediapipeline/MediaPipelineFilter.h | 7 -- 4 files changed, 10 insertions(+), 144 deletions(-) diff --git a/media/webrtc/signaling/gtest/mediapipeline_unittest.cpp b/media/webrtc/signaling/gtest/mediapipeline_unittest.cpp index b99e35674d52..27dbeff68c74 100644 --- a/media/webrtc/signaling/gtest/mediapipeline_unittest.cpp +++ b/media/webrtc/signaling/gtest/mediapipeline_unittest.cpp @@ -515,13 +515,13 @@ class MediaPipelineTest : public ::testing::Test { ASSERT_GE(p1_.GetAudioRtpCountSent(), 40); ASSERT_EQ(p1_.GetAudioRtpCountReceived(), p2_.GetAudioRtpCountSent()); ASSERT_EQ(p1_.GetAudioRtpCountSent(), p2_.GetAudioRtpCountReceived()); - - // Calling ShutdownMedia_m on both pipelines does not stop the flow of - // RTCP. So, we might be off by one here. - ASSERT_LE(p2_.GetAudioRtcpCountReceived(), p1_.GetAudioRtcpCountSent()); - ASSERT_GE(p2_.GetAudioRtcpCountReceived() + 1, p1_.GetAudioRtcpCountSent()); } + // No RTCP packets should have been dropped, because we do not filter them. + // Calling ShutdownMedia_m on both pipelines does not stop the flow of + // RTCP. So, we might be off by one here. + ASSERT_LE(p2_.GetAudioRtcpCountReceived(), p1_.GetAudioRtcpCountSent()); + ASSERT_GE(p2_.GetAudioRtcpCountReceived() + 1, p1_.GetAudioRtcpCountSent()); } void TestAudioReceiverBundle(bool bundle_accepted, @@ -588,89 +588,6 @@ TEST_F(MediaPipelineFilterTest, TestSSRCFilter) { #define RTCP_TYPEINFO(num_rrs, type, size) \ 0x80 + num_rrs, type, 0, size -const unsigned char rtcp_sr_s16[] = { - // zero rrs, size 6 words - RTCP_TYPEINFO(0, MediaPipelineFilter::SENDER_REPORT_T, 6), - REPORT_FRAGMENT(16) -}; - -const unsigned char rtcp_sr_s16_r17[] = { - // one rr, size 12 words - RTCP_TYPEINFO(1, MediaPipelineFilter::SENDER_REPORT_T, 12), - REPORT_FRAGMENT(16), - REPORT_FRAGMENT(17) -}; - -const unsigned char unknown_type[] = { - RTCP_TYPEINFO(1, 222, 0) -}; - -TEST_F(MediaPipelineFilterTest, TestEmptyFilterReport0) { - MediaPipelineFilter filter; - ASSERT_FALSE(filter.FilterSenderReport(rtcp_sr_s16, sizeof(rtcp_sr_s16))); -} - -TEST_F(MediaPipelineFilterTest, TestFilterReport0) { - MediaPipelineFilter filter; - filter.AddRemoteSSRC(16); - ASSERT_TRUE(filter.FilterSenderReport(rtcp_sr_s16, sizeof(rtcp_sr_s16))); -} - -TEST_F(MediaPipelineFilterTest, TestFilterReport0PTTruncated) { - MediaPipelineFilter filter; - filter.AddRemoteSSRC(16); - const unsigned char data[] = {0x80}; - ASSERT_FALSE(filter.FilterSenderReport(data, sizeof(data))); -} - -TEST_F(MediaPipelineFilterTest, TestFilterReport0CountTruncated) { - MediaPipelineFilter filter; - filter.AddRemoteSSRC(16); - const unsigned char* data = {}; - ASSERT_FALSE(filter.FilterSenderReport(data, sizeof(data))); -} - -TEST_F(MediaPipelineFilterTest, TestFilterReport1SSRCTruncated) { - MediaPipelineFilter filter; - filter.AddRemoteSSRC(16); - const unsigned char sr[] = { - RTCP_TYPEINFO(1, MediaPipelineFilter::SENDER_REPORT_T, 12), - REPORT_FRAGMENT(16), - 0,0,0 - }; - ASSERT_TRUE(filter.FilterSenderReport(sr, sizeof(sr))); -} - -TEST_F(MediaPipelineFilterTest, TestFilterReport1BigSSRC) { - MediaPipelineFilter filter; - filter.AddRemoteSSRC(0x01020304); - const unsigned char sr[] = { - RTCP_TYPEINFO(1, MediaPipelineFilter::SENDER_REPORT_T, 12), - SSRC(0x01020304), - REPORT_FRAGMENT(0x11121314) - }; - ASSERT_TRUE(filter.FilterSenderReport(sr, sizeof(sr))); -} - -TEST_F(MediaPipelineFilterTest, TestFilterReportMatch) { - MediaPipelineFilter filter; - filter.AddRemoteSSRC(16); - ASSERT_TRUE(filter.FilterSenderReport(rtcp_sr_s16_r17, - sizeof(rtcp_sr_s16_r17))); -} - -TEST_F(MediaPipelineFilterTest, TestFilterReportNoMatch) { - MediaPipelineFilter filter; - filter.AddRemoteSSRC(17); - ASSERT_FALSE(filter.FilterSenderReport(rtcp_sr_s16_r17, - sizeof(rtcp_sr_s16_r17))); -} - -TEST_F(MediaPipelineFilterTest, TestFilterUnknownRTCPType) { - MediaPipelineFilter filter; - ASSERT_FALSE(filter.FilterSenderReport(unknown_type, sizeof(unknown_type))); -} - TEST_F(MediaPipelineFilterTest, TestCorrelatorFilter) { MediaPipelineFilter filter; filter.SetCorrelator(7777); @@ -679,9 +596,6 @@ TEST_F(MediaPipelineFilterTest, TestCorrelatorFilter) { // This should also have resulted in the SSRC 16 being added to the filter ASSERT_TRUE(Filter(filter, 0, 16, 110)); ASSERT_FALSE(Filter(filter, 0, 17, 110)); - - // rtcp_sr_s16 has 16 as an SSRC - ASSERT_TRUE(filter.FilterSenderReport(rtcp_sr_s16, sizeof(rtcp_sr_s16))); } TEST_F(MediaPipelineFilterTest, TestPayloadTypeFilter) { @@ -691,15 +605,6 @@ TEST_F(MediaPipelineFilterTest, TestPayloadTypeFilter) { ASSERT_FALSE(Filter(filter, 0, 556, 111)); } -TEST_F(MediaPipelineFilterTest, TestPayloadTypeFilterSSRCUpdate) { - MediaPipelineFilter filter; - filter.AddUniquePT(110); - ASSERT_TRUE(Filter(filter, 0, 16, 110)); - - // rtcp_sr_s16 has 16 as an SSRC - ASSERT_TRUE(filter.FilterSenderReport(rtcp_sr_s16, sizeof(rtcp_sr_s16))); -} - TEST_F(MediaPipelineFilterTest, TestSSRCMovedWithCorrelator) { MediaPipelineFilter filter; filter.SetCorrelator(7777); @@ -752,8 +657,6 @@ TEST_F(MediaPipelineTest, TestAudioSendBundle) { ASSERT_GT(p1_.GetAudioRtpCountSent(), p2_.GetAudioRtpCountReceived()); ASSERT_GT(p2_.GetAudioRtpCountReceived(), 40); ASSERT_GT(p1_.GetAudioRtcpCountSent(), 1); - ASSERT_GT(p1_.GetAudioRtcpCountSent(), p2_.GetAudioRtcpCountReceived()); - ASSERT_GT(p2_.GetAudioRtcpCountReceived(), 0); } TEST_F(MediaPipelineTest, TestAudioSendEmptyBundleFilter) { diff --git a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp index 1036cb44d269..69edb8cf0826 100644 --- a/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp +++ b/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp @@ -1169,13 +1169,11 @@ MediaPipeline::RtcpPacketReceived(TransportLayer* aLayer, MediaPacket& packet) return; } - // We do not filter receiver reports, since the webrtc.org code for - // senders already has logic to ignore RRs that do not apply. - // TODO bug 1279153: remove SR check for reduced size RTCP - if (mFilter && !mFilter->FilterSenderReport(packet.data(), packet.len())) { - CSFLogWarn(LOGTAG, "Dropping incoming RTCP packet; filtered out"); - return; - } + // We do not filter RTCP. This is because a compound RTCP packet can contain + // any collection of RTCP packets, and webrtc.org already knows how to filter + // out what it is interested in, and what it is not. Maybe someday we should + // have a TransportLayer that breaks up compound RTCP so we can filter them + // individually, but I doubt that will matter much. CSFLogDebug(LOGTAG, "%s received RTCP packet.", mDescription.c_str()); IncrementRtcpPacketsReceived(); diff --git a/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp b/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp index 8df8d8ab4353..84c12db5811e 100644 --- a/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp +++ b/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.cpp @@ -94,33 +94,5 @@ void MediaPipelineFilter::Update(const MediaPipelineFilter& filter_update) { correlator_ = filter_update.correlator_; } -bool -MediaPipelineFilter::FilterSenderReport(const unsigned char* data, - size_t len) const { - - if (!data) { - return false; - } - - if (len < FIRST_SSRC_OFFSET + 4) { - return false; - } - - uint8_t payload_type = data[PT_OFFSET]; - - if (payload_type != SENDER_REPORT_T) { - // Not a sender report, let it through - return true; - } - - uint32_t ssrc = 0; - ssrc += (uint32_t)data[FIRST_SSRC_OFFSET] << 24; - ssrc += (uint32_t)data[FIRST_SSRC_OFFSET + 1] << 16; - ssrc += (uint32_t)data[FIRST_SSRC_OFFSET + 2] << 8; - ssrc += (uint32_t)data[FIRST_SSRC_OFFSET + 3]; - - return !!remote_ssrc_set_.count(ssrc); -} - } // end namespace mozilla diff --git a/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.h b/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.h index 8a584e9a3af8..380d42447833 100644 --- a/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.h +++ b/media/webrtc/signaling/src/mediapipeline/MediaPipelineFilter.h @@ -53,10 +53,6 @@ class MediaPipelineFilter { // the filter about ssrcs) bool Filter(const webrtc::RTPHeader& header, uint32_t correlator = 0); - // RTCP doesn't have things like the RTP correlator, and uses its own - // payload types too. - bool FilterSenderReport(const unsigned char* data, size_t len) const; - void AddRemoteSSRC(uint32_t ssrc); void AddRemoteRtpStreamId(const std::string& rtp_strm_id); @@ -66,9 +62,6 @@ class MediaPipelineFilter { void Update(const MediaPipelineFilter& filter_update); - // Some payload types - static const uint8_t SENDER_REPORT_T = 200; - private: // Payload type is always in the second byte static const size_t PT_OFFSET = 1; From cb6d2f84edb8f7f65665a997998197f6be66f35b Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Mon, 23 Jul 2018 19:20:30 +0100 Subject: [PATCH 03/26] Bug 1477775 - Remove extensions.webextensions.themes.enabled preference. r=jaws MozReview-Commit-ID: 2dnFhMGRLbG --HG-- extra : rebase_source : 72e92fbcce1d15f7fd800241b7db9bc7224f1d31 --- browser/app/profile/firefox.js | 1 - .../browser_ext_browserAction_theme_icons.js | 6 ----- .../test/browser/browser_ext_themes_icons.js | 3 +-- .../browser/browser_ext_themes_validation.js | 6 ----- modules/libpref/init/all.js | 3 +-- .../components/extensions/parent/ext-theme.js | 24 ------------------- .../browser/browser_ext_management_themes.js | 6 ----- .../browser_ext_themes_chromeparity.js | 6 ----- .../browser_ext_themes_dynamic_updates.js | 6 ----- .../browser/browser_ext_themes_findbar.js | 6 ----- .../browser/browser_ext_themes_lwtsupport.js | 6 ----- ...browser_ext_themes_multiple_backgrounds.js | 6 ----- .../browser/browser_ext_themes_persistence.js | 6 ----- .../browser/browser_ext_themes_tab_text.js | 6 ----- .../browser_ext_themes_toolbar_fields.js | 4 ---- .../browser/browser_ext_themes_toolbars.js | 6 ----- 16 files changed, 2 insertions(+), 99 deletions(-) diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 113579281084..498f9e1164a2 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -179,7 +179,6 @@ pref("extensions.update.background.url", "https://versioncheck-bg.addons.mozilla pref("extensions.update.interval", 86400); // Check for updates to Extensions and // Themes every day -pref("extensions.webextensions.themes.enabled", true); pref("extensions.webextensions.themes.icons.buttons", "back,forward,reload,stop,bookmark_star,bookmark_menu,downloads,home,app_menu,cut,copy,paste,new_window,new_private_window,save_page,print,history,full_screen,find,options,addons,developer,synced_tabs,open_file,sidebars,share_page,subscribe,text_encoding,email_link,forget,pocket"); pref("lightweightThemes.update.enabled", true); diff --git a/browser/components/extensions/test/browser/browser_ext_browserAction_theme_icons.js b/browser/components/extensions/test/browser/browser_ext_browserAction_theme_icons.js index a41aa8890d0a..73cb5745342b 100644 --- a/browser/components/extensions/test/browser/browser_ext_browserAction_theme_icons.js +++ b/browser/components/extensions/test/browser/browser_ext_browserAction_theme_icons.js @@ -15,12 +15,6 @@ const DARK_THEME_COLORS = { "textcolor": "#FFF", }; -add_task(async function setup() { - await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true]], - }); -}); - async function testBrowserAction(extension, expectedIcon) { let browserActionWidget = getBrowserActionWidget(extension); await promiseAnimationFrame(); diff --git a/browser/components/extensions/test/browser/browser_ext_themes_icons.js b/browser/components/extensions/test/browser/browser_ext_themes_icons.js index fd18966d39e3..59480080230e 100644 --- a/browser/components/extensions/test/browser/browser_ext_themes_icons.js +++ b/browser/components/extensions/test/browser/browser_ext_themes_icons.js @@ -162,8 +162,7 @@ async function runTestWithIcons(icons) { add_task(async function setup() { await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true], - ["extensions.webextensions.themes.icons.enabled", true]], + set: [["extensions.webextensions.themes.icons.enabled", true]], }); }); diff --git a/browser/components/extensions/test/browser/browser_ext_themes_validation.js b/browser/components/extensions/test/browser/browser_ext_themes_validation.js index 1c23f2484e68..269f2a746149 100644 --- a/browser/components/extensions/test/browser/browser_ext_themes_validation.js +++ b/browser/components/extensions/test/browser/browser_ext_themes_validation.js @@ -1,11 +1,5 @@ "use strict"; -add_task(async function setup() { - await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true]], - }); -}); - /** * Helper function for testing a theme with invalid properties. * @param {object} invalidProps The invalid properties to load the theme with. diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index ccc1f71c5c52..506b742130bf 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -4947,8 +4947,7 @@ pref("extensions.webextensions.keepUuidOnUninstall", false); // Redirect basedomain used by identity api pref("extensions.webextensions.identity.redirectDomain", "extensions.allizom.org"); pref("extensions.webextensions.restrictedDomains", "accounts-static.cdn.mozilla.net,accounts.firefox.com,addons.cdn.mozilla.net,addons.mozilla.org,api.accounts.firefox.com,content.cdn.mozilla.net,content.cdn.mozilla.net,discovery.addons.mozilla.org,input.mozilla.org,install.mozilla.org,oauth.accounts.firefox.com,profile.accounts.firefox.com,support.mozilla.org,sync.services.mozilla.com,testpilot.firefox.com"); -// Whether or not webextension themes are supported. -pref("extensions.webextensions.themes.enabled", false); +// Whether or not webextension icon theming is supported. pref("extensions.webextensions.themes.icons.enabled", false); pref("extensions.webextensions.remote", false); // Whether or not the moz-extension resource loads are remoted. For debugging diff --git a/toolkit/components/extensions/parent/ext-theme.js b/toolkit/components/extensions/parent/ext-theme.js index 5c22828798c8..def350c72ec2 100644 --- a/toolkit/components/extensions/parent/ext-theme.js +++ b/toolkit/components/extensions/parent/ext-theme.js @@ -7,10 +7,6 @@ ChromeUtils.import("resource://gre/modules/Services.jsm"); ChromeUtils.defineModuleGetter(this, "LightweightThemeManager", "resource://gre/modules/LightweightThemeManager.jsm"); -XPCOMUtils.defineLazyGetter(this, "gThemesEnabled", () => { - return Services.prefs.getBoolPref("extensions.webextensions.themes.enabled"); -}); - var { getWinUtils, } = ExtensionUtils; @@ -312,19 +308,9 @@ class Theme { this.theme = class extends ExtensionAPI { onManifestEntry(entryName) { - if (!gThemesEnabled) { - // Return early if themes are disabled. - return; - } - let {extension} = this; let {manifest} = extension; - if (!gThemesEnabled) { - // Return early if themes are disabled. - return; - } - defaultTheme = new Theme(extension); defaultTheme.load(manifest.theme); } @@ -363,11 +349,6 @@ this.theme = class extends ExtensionAPI { return Promise.resolve(defaultTheme.details); }, update: (windowId, details) => { - if (!gThemesEnabled) { - // Return early if themes are disabled. - return; - } - if (windowId) { const browserWindow = windowTracker.getWindow(windowId, context); if (!browserWindow) { @@ -379,11 +360,6 @@ this.theme = class extends ExtensionAPI { theme.load(details); }, reset: (windowId) => { - if (!gThemesEnabled) { - // Return early if themes are disabled. - return; - } - if (windowId) { const browserWindow = windowTracker.getWindow(windowId, context); if (!browserWindow) { diff --git a/toolkit/components/extensions/test/browser/browser_ext_management_themes.js b/toolkit/components/extensions/test/browser/browser_ext_management_themes.js index 62c0006e5d64..4d81dc89eab7 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_management_themes.js +++ b/toolkit/components/extensions/test/browser/browser_ext_management_themes.js @@ -7,12 +7,6 @@ const {LightweightThemeManager} = ChromeUtils.import("resource://gre/modules/Lig const {PromiseTestUtils} = ChromeUtils.import("resource://testing-common/PromiseTestUtils.jsm", null); PromiseTestUtils.whitelistRejectionsGlobally(/Message manager disconnected/); -add_task(async function setup() { - await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true]], - }); -}); - add_task(async function test_management_themes() { const TEST_ID = "test_management_themes@tests.mozilla.com"; diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js b/toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js index d660780fa90e..1866ccb34afe 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_chromeparity.js @@ -1,11 +1,5 @@ "use strict"; -add_task(async function setup() { - await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true]], - }); -}); - add_task(async function test_support_theme_frame() { const FRAME_COLOR = [71, 105, 91]; const TAB_TEXT_COLOR = [207, 221, 192]; diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js b/toolkit/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js index 84614aa93fca..70b512cf4f8c 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_dynamic_updates.js @@ -36,12 +36,6 @@ function validateTheme(backgroundImage, accentColor, textColor, isLWT) { Assert.equal(style.color, textColor, "Expected correct text color"); } -add_task(async function setup() { - await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true]], - }); -}); - add_task(async function test_dynamic_theme_updates() { let extension = ExtensionTestUtils.loadExtension({ manifest: { diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_findbar.js b/toolkit/components/extensions/test/browser/browser_ext_themes_findbar.js index 0e4d243115ec..893860120906 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_findbar.js +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_findbar.js @@ -3,12 +3,6 @@ // This test checks whether applied WebExtension themes that attempt to change // the toolbar and toolbar_field properties also theme the findbar. -add_task(async function setup() { - await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true]], - }); -}); - add_task(async function test_support_toolbar_properties_on_findbar() { const TOOLBAR_COLOR = "#ff00ff"; const TOOLBAR_TEXT_COLOR = "#9400ff"; diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_lwtsupport.js b/toolkit/components/extensions/test/browser/browser_ext_themes_lwtsupport.js index 64282924a556..60d579ddd020 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_lwtsupport.js +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_lwtsupport.js @@ -1,11 +1,5 @@ "use strict"; -add_task(async function setup() { - await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true]], - }); -}); - add_task(async function test_support_LWT_properties() { let extension = ExtensionTestUtils.loadExtension({ manifest: { diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js b/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js index db824b13d849..4b2e82ad018f 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js @@ -1,11 +1,5 @@ "use strict"; -add_task(async function setup() { - await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true]], - }); -}); - add_task(async function test_support_backgrounds_position() { let extension = ExtensionTestUtils.loadExtension({ manifest: { diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_persistence.js b/toolkit/components/extensions/test/browser/browser_ext_themes_persistence.js index e3a66aa24d15..3ea5e4539395 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_persistence.js +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_persistence.js @@ -3,12 +3,6 @@ // This test checks whether applied WebExtension themes are persisted and applied // on newly opened windows. -add_task(async function setup() { - await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true]], - }); -}); - add_task(async function test_multiple_windows() { let extension = ExtensionTestUtils.loadExtension({ manifest: { diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_tab_text.js b/toolkit/components/extensions/test/browser/browser_ext_themes_tab_text.js index f41b90f5de09..bd1591775861 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_tab_text.js +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_tab_text.js @@ -3,12 +3,6 @@ // This test checks whether applied WebExtension themes that attempt to change // the text color of the selected tab are applied properly. -add_task(async function setup() { - await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true]], - }); -}); - add_task(async function test_support_tab_text_property_css_color() { const TAB_TEXT_COLOR = "#9400ff"; let extension = ExtensionTestUtils.loadExtension({ diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_toolbar_fields.js b/toolkit/components/extensions/test/browser/browser_ext_themes_toolbar_fields.js index dc201652af30..8b957677381a 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_toolbar_fields.js +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_toolbar_fields.js @@ -7,10 +7,6 @@ ChromeUtils.import("resource://testing-common/CustomizableUITestUtils.jsm", this let gCUITestUtils = new CustomizableUITestUtils(window); add_task(async function setup() { - await SpecialPowers.pushPrefEnv({set: [ - ["extensions.webextensions.themes.enabled", true], - ]}); - await gCUITestUtils.addSearchBar(); registerCleanupFunction(() => { gCUITestUtils.removeSearchBar(); diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_toolbars.js b/toolkit/components/extensions/test/browser/browser_ext_themes_toolbars.js index 802a959ef19b..e307c5dcadda 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_toolbars.js +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_toolbars.js @@ -3,12 +3,6 @@ // This test checks whether applied WebExtension themes that attempt to change // the background color of toolbars are applied properly. -add_task(async function setup() { - await SpecialPowers.pushPrefEnv({ - set: [["extensions.webextensions.themes.enabled", true]], - }); -}); - add_task(async function test_support_toolbar_property() { const TOOLBAR_COLOR = "#ff00ff"; const TOOLBAR_TEXT_COLOR = "#9400ff"; From c4a680096a68b654cf26e0d304fb4ea2e6e8082e Mon Sep 17 00:00:00 2001 From: Kartikaya Gupta Date: Mon, 23 Jul 2018 14:49:04 -0400 Subject: [PATCH 04/26] Bug 1477792 - Save the rust-analysis on macOS searchfox builds. r=emilio This is the equivalent of bug 1432475 but for macOS. MozReview-Commit-ID: DT8V9Vd9eLS --HG-- extra : rebase_source : 982e2fcb8aed0640acd80025319fdcac24634c47 --- browser/config/mozconfigs/macosx64/debug-searchfox | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/browser/config/mozconfigs/macosx64/debug-searchfox b/browser/config/mozconfigs/macosx64/debug-searchfox index 526fd941f330..f475b91e1199 100644 --- a/browser/config/mozconfigs/macosx64/debug-searchfox +++ b/browser/config/mozconfigs/macosx64/debug-searchfox @@ -7,6 +7,10 @@ MOZ_AUTOMATION_L10N_CHECK=0 ac_add_options --enable-debug ac_add_options --enable-dmd +# Save rust analysis (this requires unlocking the unstable features) +export RUSTC_BOOTSTRAP=1 +export RUSTFLAGS="-Zsave-analysis" + ac_add_options --enable-clang-plugin ac_add_options --enable-mozsearch-plugin From 1985a59a89db3df908aaa1e15bdc36a1c501df29 Mon Sep 17 00:00:00 2001 From: Kartikaya Gupta Date: Mon, 23 Jul 2018 14:49:31 -0400 Subject: [PATCH 05/26] Bug 1477792 - Run daily macOS searchfox builds. r=emilio MozReview-Commit-ID: 5D05AovVXrf --HG-- extra : rebase_source : 70581c6b010df95121601564e5b5d5b56f5af7d0 --- taskcluster/taskgraph/target_tasks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/taskcluster/taskgraph/target_tasks.py b/taskcluster/taskgraph/target_tasks.py index 753f1d9c34c0..b71910f221e0 100644 --- a/taskcluster/taskgraph/target_tasks.py +++ b/taskcluster/taskgraph/target_tasks.py @@ -566,9 +566,10 @@ def target_tasks_dmd(full_task_graph, parameters, graph_config): @_target_task('searchfox_index') def target_tasks_searchfox(full_task_graph, parameters, graph_config): """Select tasks required for indexing Firefox for Searchfox web site each day""" - # For now we only do Linux debug builds. Windows and Mac builds + # For now we only do Linux and Mac debug builds. Windows builds # are currently broken (bug 1418415). - return ['searchfox-linux64-searchfox/debug'] + return ['searchfox-linux64-searchfox/debug', + 'searchfox-macosx64-searchfox/debug'] @_target_task('file_update') From 187dbcb6e059aa6281d05726747f401963c8c964 Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Mon, 23 Jul 2018 14:22:06 +0100 Subject: [PATCH 06/26] Bug 1477708 - Make theme loading part of the Theme constructor in ext-theme.js. r=jaws MozReview-Commit-ID: 2rZu7Iqtk7D --HG-- extra : rebase_source : a7f590444c9faecce37f08993c7bd781cd4c2138 --- .../components/extensions/parent/ext-theme.js | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/toolkit/components/extensions/parent/ext-theme.js b/toolkit/components/extensions/parent/ext-theme.js index def350c72ec2..dba576973fae 100644 --- a/toolkit/components/extensions/parent/ext-theme.js +++ b/toolkit/components/extensions/parent/ext-theme.js @@ -36,17 +36,16 @@ class Theme { * @param {string} extension Extension that created the theme. * @param {Integer} windowId The windowId where the theme is applied. */ - constructor(extension, windowId) { - // The base URI of the extension, used to resolve relative filepaths. - this.baseURI = extension.baseURI; - // Logger that will be used to show manifest warnings to the theme author. - this.logger = extension.logger; - + constructor({extension, details, windowId}) { this.extension = extension; + this.details = details; this.windowId = windowId; + this.lwtStyles = { icons: {}, }; + + this.load(); } /** @@ -56,8 +55,8 @@ class Theme { * @param {Object} details Theme part of the manifest. Supported * properties can be found in the schema under ThemeType. */ - load(details) { - this.details = details; + load() { + const {details} = this; if (details.colors) { this.loadColors(details.colors); @@ -174,6 +173,8 @@ class Theme { * @param {Object} images Dictionary mapping image properties to values. */ loadImages(images) { + const {baseURI} = this.extension; + for (let image of Object.keys(images)) { let val = images[image]; @@ -183,13 +184,13 @@ class Theme { switch (image) { case "additional_backgrounds": { - let backgroundImages = val.map(img => this.baseURI.resolve(img)); + let backgroundImages = val.map(img => baseURI.resolve(img)); this.lwtStyles.additionalBackgrounds = backgroundImages; break; } case "headerURL": case "theme_frame": { - let resolvedURL = this.baseURI.resolve(val); + let resolvedURL = baseURI.resolve(val); this.lwtStyles.headerURL = resolvedURL; break; } @@ -203,6 +204,8 @@ class Theme { * @param {Object} icons Dictionary mapping icon properties to extension URLs. */ loadIcons(icons) { + const {baseURI} = this.extension; + if (!Services.prefs.getBoolPref("extensions.webextensions.themes.icons.enabled")) { // Return early if icons are disabled. return; @@ -213,11 +216,11 @@ class Theme { // We also have to compare against the baseURI spec because // `val` might have been resolved already. Resolving "" against // the baseURI just produces that URI, so check for equality. - if (!val || val == this.baseURI.spec || !ICONS.includes(icon)) { + if (!val || val == baseURI.spec || !ICONS.includes(icon)) { continue; } let variableName = `--${icon}-icon`; - let resolvedURL = this.baseURI.resolve(val); + let resolvedURL = baseURI.resolve(val); this.lwtStyles.icons[variableName] = resolvedURL; } } @@ -233,13 +236,14 @@ class Theme { let additionalBackgroundsCount = (this.lwtStyles.additionalBackgrounds && this.lwtStyles.additionalBackgrounds.length) || 0; const assertValidAdditionalBackgrounds = (property, valueCount) => { + const {logger} = this.extension; if (!additionalBackgroundsCount) { - this.logger.warn(`The '${property}' property takes effect only when one ` + + logger.warn(`The '${property}' property takes effect only when one ` + `or more additional background images are specified using the 'additional_backgrounds' property.`); return false; } if (additionalBackgroundsCount !== valueCount) { - this.logger.warn(`The amount of values specified for '${property}' ` + + logger.warn(`The amount of values specified for '${property}' ` + `(${valueCount}) is not equal to the amount of additional background ` + `images (${additionalBackgroundsCount}), which may lead to unexpected results.`); } @@ -311,8 +315,10 @@ this.theme = class extends ExtensionAPI { let {extension} = this; let {manifest} = extension; - defaultTheme = new Theme(extension); - defaultTheme.load(manifest.theme); + defaultTheme = new Theme({ + extension, + details: manifest.theme, + }); } onShutdown(reason) { @@ -356,8 +362,11 @@ this.theme = class extends ExtensionAPI { } } - let theme = new Theme(extension, windowId); - theme.load(details); + new Theme({ + extension, + details, + windowId, + }); }, reset: (windowId) => { if (windowId) { From 87760314164c4f5c424e69844a8ce6bb6d9c46f4 Mon Sep 17 00:00:00 2001 From: Dale Harvey Date: Mon, 23 Jul 2018 12:20:30 +0100 Subject: [PATCH 07/26] Bug 1476889 - Update autoplay copy. r=flod MozReview-Commit-ID: D8Ss2U2gZ9q --HG-- extra : rebase_source : 4987f9365b0a8eec31a7031cd2ffea342187aa84 --- browser/components/preferences/in-content/privacy.xul | 6 +++--- browser/components/preferences/permissions.js | 2 +- browser/locales/en-US/browser/preferences/permissions.ftl | 2 +- browser/locales/en-US/browser/preferences/preferences.ftl | 4 ++-- browser/locales/en-US/chrome/browser/browser.properties | 4 ++-- browser/modules/PermissionUI.jsm | 4 ++-- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/browser/components/preferences/in-content/privacy.xul b/browser/components/preferences/in-content/privacy.xul index 02f165ba3309..78e092325858 100644 --- a/browser/components/preferences/in-content/privacy.xul +++ b/browser/components/preferences/in-content/privacy.xul @@ -495,7 +495,7 @@ permissions-button-cancel.label, permissions-button-ok.label, permissions-exceptions-autoplay-media-window.title, - permissions-exceptions-autoplay-media-desc + permissions-exceptions-autoplay-media-desc2 " /> @@ -567,7 +567,7 @@ - + @@ -580,7 +580,7 @@ permissions-button-cancel.label, permissions-button-ok.label, permissions-exceptions-autoplay-media-window.title, - permissions-exceptions-autoplay-media-desc + permissions-exceptions-autoplay-media-desc2 " /> diff --git a/browser/components/preferences/permissions.js b/browser/components/preferences/permissions.js index 4913e15ad133..cfd8a442c463 100644 --- a/browser/components/preferences/permissions.js +++ b/browser/components/preferences/permissions.js @@ -28,7 +28,7 @@ const permissionExceptionsL10n = { }, "autoplay-media": { window: "permissions-exceptions-autoplay-media-window", - description: "permissions-exceptions-autoplay-media-desc", + description: "permissions-exceptions-autoplay-media-desc2", }, }; diff --git a/browser/locales/en-US/browser/preferences/permissions.ftl b/browser/locales/en-US/browser/preferences/permissions.ftl index 26fe60388dd7..1b60167e2522 100644 --- a/browser/locales/en-US/browser/preferences/permissions.ftl +++ b/browser/locales/en-US/browser/preferences/permissions.ftl @@ -101,7 +101,7 @@ permissions-exceptions-addons-desc = You can specify which websites are allowed permissions-exceptions-autoplay-media-window = .title = Allowed Websites - Autoplay .style = { permissions-window.style } -permissions-exceptions-autoplay-media-desc = You can specify which websites are allowed to automatically play media elements. Type the exact address of the site you want to allow and then click Allow. +permissions-exceptions-autoplay-media-desc2 = You can specify which websites are always or never allowed to autoplay media with sound. Type the address of the site you want to manage and then click Block or Allow. ## Site Permissions - Notifications diff --git a/browser/locales/en-US/browser/preferences/preferences.ftl b/browser/locales/en-US/browser/preferences/preferences.ftl index 3835b7cc98ba..f9ef7bc6d264 100644 --- a/browser/locales/en-US/browser/preferences/preferences.ftl +++ b/browser/locales/en-US/browser/preferences/preferences.ftl @@ -854,8 +854,8 @@ autoplay-option-ask = .label = Always Ask autoplay-option-allow = .label = Allow Autoplay -autoplay-option-block = - .label = Block Autoplay +autoplay-option-dont = + .label = Don't Autoplay permissions-block-popups = .label = Block pop-up windows diff --git a/browser/locales/en-US/chrome/browser/browser.properties b/browser/locales/en-US/chrome/browser/browser.properties index 217efcf42fd3..e09db971d117 100644 --- a/browser/locales/en-US/chrome/browser/browser.properties +++ b/browser/locales/en-US/chrome/browser/browser.properties @@ -958,8 +958,8 @@ midi.shareSysexWithFile.message = Will you allow this local file to access your # LOCALIZATION NOTE (midi.shareSysexWithSite.message): %S is the name of the site URL (https://...) requesting MIDI access midi.shareSysexWithSite.message = Will you allow %S to access your MIDI devices and send/receive SysEx messages? -autoplay.Allow.label = Allow Autoplay -autoplay.Allow.accesskey = A +autoplay.Allow2.label = Allow +autoplay.Allow2.accesskey = A autoplay.DontAllow.label = Don’t Allow autoplay.DontAllow.accesskey = n autoplay.remember = Remember this decision diff --git a/browser/modules/PermissionUI.jsm b/browser/modules/PermissionUI.jsm index 676f119ac6b3..d1f74c13e75a 100644 --- a/browser/modules/PermissionUI.jsm +++ b/browser/modules/PermissionUI.jsm @@ -821,8 +821,8 @@ AutoplayPermissionPrompt.prototype = { get promptActions() { return [{ - label: gBrowserBundle.GetStringFromName("autoplay.Allow.label"), - accessKey: gBrowserBundle.GetStringFromName("autoplay.Allow.accesskey"), + label: gBrowserBundle.GetStringFromName("autoplay.Allow2.label"), + accessKey: gBrowserBundle.GetStringFromName("autoplay.Allow2.accesskey"), action: Ci.nsIPermissionManager.ALLOW_ACTION, }, { From c619a45c9ba53b89d0a2f4cf59507c6d8c29e826 Mon Sep 17 00:00:00 2001 From: "Abdoulaye O. Ly" Date: Mon, 23 Jul 2018 19:53:56 +0000 Subject: [PATCH 08/26] Bug 1477724 - 'Move To New Window' moves now contextTab instead of selectedTab in single tab context. r=jaws MozReview-Commit-ID: 2c5QFuSO6kU --HG-- extra : rebase_source : ef1c0a1b113c8684ee43726995598aedcf3d64a9 --- browser/base/content/tabbrowser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/base/content/tabbrowser.js b/browser/base/content/tabbrowser.js index 3d5878ea6fa6..fb83c162e1b9 100644 --- a/browser/base/content/tabbrowser.js +++ b/browser/base/content/tabbrowser.js @@ -3422,7 +3422,7 @@ window._gBrowser = { if (contextTab.multiselected) { tabs = this.selectedTabs; } else { - tabs = [gBrowser.selectedTab]; + tabs = [contextTab]; } if (this.tabs.length == tabs.length) { From f7a7c9d22ab69b9ff280422912de91bd2b60562c Mon Sep 17 00:00:00 2001 From: Brian Birtles Date: Fri, 20 Jul 2018 08:40:50 +0900 Subject: [PATCH 09/26] Bug 1474224 - Make the dock to bottom icon width the same as other icons; r=jdescottes MozReview-Commit-ID: 6gawJzr6NzR --HG-- extra : rebase_source : 4fec75388e2ac117c428fbebabc7e36f638c878e --- devtools/client/themes/images/dock-bottom.svg | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/devtools/client/themes/images/dock-bottom.svg b/devtools/client/themes/images/dock-bottom.svg index 2559c86606a0..98c5b662ed97 100644 --- a/devtools/client/themes/images/dock-bottom.svg +++ b/devtools/client/themes/images/dock-bottom.svg @@ -4,7 +4,7 @@ + M13 1a3 3 0 0 1 3 3v8a3 3 0 0 1-3 3h-10a3 3 0 0 1-3-3v-8a3 3 0 0 1 3-3z + M13 3h-10a1 1 0 0 0-1 1v5h12v-5a1 1 0 0 0-1-1z + M14 10h-12v2a1 1 0 0 0 1 1h10a1 1 0 0 0 1-1z"/> From f93f0d87f2a79dcef36568eb1aa33eecb23265a6 Mon Sep 17 00:00:00 2001 From: Brian Birtles Date: Fri, 20 Jul 2018 08:40:50 +0900 Subject: [PATCH 10/26] Bug 1474224 - Use Photon "Settings" icon in meatball menu; r=jdescottes As per https://design.firefox.com/icons/icons/android/preferences-24.svg but adapted to fit on a 16x16 grid whilst maintaining 2px stroke width. MozReview-Commit-ID: 5Aoywexg7AM --HG-- extra : rebase_source : 48e4e10f57d7daab94e62f58a1a41bc01c5b13db --- .../client/themes/images/tool-options-photon.svg | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/devtools/client/themes/images/tool-options-photon.svg b/devtools/client/themes/images/tool-options-photon.svg index ab548cd49401..22a3681d37b6 100644 --- a/devtools/client/themes/images/tool-options-photon.svg +++ b/devtools/client/themes/images/tool-options-photon.svg @@ -3,7 +3,15 @@ - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> - - + From cc3c0a3582a63de27c9a98b08c4978228b557531 Mon Sep 17 00:00:00 2001 From: Brian Birtles Date: Fri, 20 Jul 2018 09:34:18 +0900 Subject: [PATCH 11/26] Bug 1474224 - Use Photon "Open in New" icon for undock icon; r=jdescottes As per https://design.firefox.com/icons/icons/android/open-in-new-24.svg but rather adapted to match the proportions of the other icons and fit on a 16x16 grid. MozReview-Commit-ID: 9bGWiBjX1P4 --HG-- extra : rebase_source : 1b87badf8ed045368c7f3d003c3c4f4611ab6a4b --- devtools/client/themes/images/dock-undock.svg | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/devtools/client/themes/images/dock-undock.svg b/devtools/client/themes/images/dock-undock.svg index c645424b2ef0..e0ec68bc73ba 100644 --- a/devtools/client/themes/images/dock-undock.svg +++ b/devtools/client/themes/images/dock-undock.svg @@ -4,9 +4,8 @@ - + M5 1a1 1 0 0 1 0 2h-2a1 1 0 0 0-1 1v8a1 1 0 0 0 1 1h10a1 1 0 0 0 1-1 + a1 1 0 0 1 2 0a3 3 0 0 1-3 3h-10a3 3 0 0 1-3-3v-8a3 3 0 0 1 3-3z + M9 3a1 1 0 1 1 0-2h6a1 1 0 0 1 1 1v6a1 1 0 1 1-2 0v-3.586l-5.293 5.293 + a1 1 0 0 1-1.414-1.414l5.293 -5.293z"/> From 04f0622f7b139d27f47d09ed8eb750f1ce17c31d Mon Sep 17 00:00:00 2001 From: Robert Bartlensky Date: Fri, 20 Jul 2018 13:44:35 +0100 Subject: [PATCH 12/26] Bug 1476657: Fix DEAD_STORE issues in js/src/frontend/Parser.cpp. r=arai MozReview-Commit-ID: HI9p2AfOUiU --HG-- extra : rebase_source : a49d1b189886e2eb77550fdcb0e8995b2a09c4c0 --- js/src/frontend/Parser.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index 8bce66dda6e5..b264a0bf9ad0 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -22,6 +22,7 @@ #include "mozilla/Range.h" #include "mozilla/Sprintf.h" #include "mozilla/TypeTraits.h" +#include "mozilla/Unused.h" #include #include @@ -58,6 +59,7 @@ using mozilla::Nothing; using mozilla::PodCopy; using mozilla::PodZero; using mozilla::Some; +using mozilla::Unused; using JS::AutoGCRooter; @@ -1858,7 +1860,7 @@ NewGlobalScopeData(JSContext* context, ParseContext::Scope& scope, LifoAlloc& al cursor = FreshlyInitializeBindings(cursor, lets); bindings->constStart = cursor - start; - cursor = FreshlyInitializeBindings(cursor, consts); + Unused << FreshlyInitializeBindings(cursor, consts); bindings->length = numBindings; } @@ -1928,7 +1930,7 @@ NewModuleScopeData(JSContext* context, ParseContext::Scope& scope, LifoAlloc& al cursor = FreshlyInitializeBindings(cursor, lets); bindings->constStart = cursor - start; - cursor = FreshlyInitializeBindings(cursor, consts); + Unused << FreshlyInitializeBindings(cursor, consts); bindings->length = numBindings; } @@ -1968,7 +1970,7 @@ NewEvalScopeData(JSContext* context, ParseContext::Scope& scope, LifoAlloc& allo BindingName* start = bindings->trailingNames.start(); BindingName* cursor = start; - cursor = FreshlyInitializeBindings(cursor, vars); + Unused << FreshlyInitializeBindings(cursor, vars); bindings->length = numBindings; } @@ -2067,7 +2069,7 @@ NewFunctionScopeData(JSContext* context, ParseContext::Scope& scope, bool hasPar cursor = FreshlyInitializeBindings(cursor, formals); bindings->varStart = cursor - start; - cursor = FreshlyInitializeBindings(cursor, vars); + Unused << FreshlyInitializeBindings(cursor, vars); bindings->length = numBindings; } @@ -2108,7 +2110,7 @@ NewVarScopeData(JSContext* context, ParseContext::Scope& scope, LifoAlloc& alloc BindingName* start = bindings->trailingNames.start(); BindingName* cursor = start; - cursor = FreshlyInitializeBindings(cursor, vars); + Unused << FreshlyInitializeBindings(cursor, vars); bindings->length = numBindings; } @@ -2165,7 +2167,7 @@ NewLexicalScopeData(JSContext* context, ParseContext::Scope& scope, LifoAlloc& a cursor = FreshlyInitializeBindings(cursor, lets); bindings->constStart = cursor - start; - cursor = FreshlyInitializeBindings(cursor, consts); + Unused << FreshlyInitializeBindings(cursor, consts); bindings->length = numBindings; } From d0daa2080382db4a9ecbc6c3a8885353fcfead6a Mon Sep 17 00:00:00 2001 From: Robert Bartlensky Date: Wed, 18 Jul 2018 16:17:22 +0100 Subject: [PATCH 13/26] Bug 1476565: Fix DEAD_STORE errors in dom/base/*. r=baku MozReview-Commit-ID: IjsOrpz9VF3 --HG-- extra : rebase_source : 0ac09534b0d129564bf986d45145e0967e8182d2 --- dom/base/nsContentUtils.cpp | 10 ++++------ dom/base/nsGlobalWindowInner.cpp | 9 +++------ dom/base/nsGlobalWindowOuter.cpp | 2 +- dom/base/nsImageLoadingContent.cpp | 5 ++--- dom/base/nsObjectLoadingContent.cpp | 9 +++------ 5 files changed, 13 insertions(+), 22 deletions(-) diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 40916ed72fa3..5aa956847727 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -2939,7 +2939,6 @@ nsContentUtils::GenerateStateKey(nsIContent* aContent, KeyAppendInt(control->ControlType(), aKey); // If in a form, add form name / index of form / index in form - int32_t index = -1; Element *formElement = control->GetFormElement(); if (formElement) { if (IsAutocompleteOff(formElement)) { @@ -2950,7 +2949,7 @@ nsContentUtils::GenerateStateKey(nsIContent* aContent, KeyAppendString(NS_LITERAL_CSTRING("f"), aKey); // Append the index of the form in the document - index = htmlForms->IndexOf(formElement, false); + int32_t index = htmlForms->IndexOf(formElement, false); if (index <= -1) { // // XXX HACK this uses some state that was dumped into the document @@ -2991,7 +2990,7 @@ nsContentUtils::GenerateStateKey(nsIContent* aContent, // We have to flush sink notifications at this point to make // sure that htmlFormControls is up to date. - index = htmlFormControls->IndexOf(aContent, true); + int32_t index = htmlFormControls->IndexOf(aContent, true); if (index > -1) { KeyAppendInt(index, aKey); generatedUniqueKey = true; @@ -6510,10 +6509,9 @@ nsContentUtils::WrapNative(JSContext *cx, nsISupports *native, MOZ_CRASH(); } - nsresult rv = NS_OK; JS::Rooted scope(cx, JS::CurrentGlobalOrNull(cx)); - rv = sXPConnect->WrapNativeToJSVal(cx, scope, native, cache, aIID, - aAllowWrapping, vp); + nsresult rv = sXPConnect->WrapNativeToJSVal(cx, scope, native, cache, aIID, + aAllowWrapping, vp); return rv; } diff --git a/dom/base/nsGlobalWindowInner.cpp b/dom/base/nsGlobalWindowInner.cpp index 1ed50294776b..bc11b460625d 100644 --- a/dom/base/nsGlobalWindowInner.cpp +++ b/dom/base/nsGlobalWindowInner.cpp @@ -4877,8 +4877,6 @@ nsGlobalWindowInner::DispatchSyncPopState() NS_ASSERTION(nsContentUtils::IsSafeToRunScript(), "Must be safe to run script here."); - nsresult rv = NS_OK; - // Bail if the window is frozen. if (IsFrozen()) { return NS_OK; @@ -4888,12 +4886,11 @@ nsGlobalWindowInner::DispatchSyncPopState() // going to send along with the popstate event. The object is serialized // using structured clone. nsCOMPtr stateObj; - rv = mDoc->GetStateObject(getter_AddRefs(stateObj)); + nsresult rv = mDoc->GetStateObject(getter_AddRefs(stateObj)); NS_ENSURE_SUCCESS(rv, rv); - bool result = true; AutoJSAPI jsapi; - result = jsapi.Init(this); + bool result = jsapi.Init(this); NS_ENSURE_TRUE(result, NS_ERROR_FAILURE); JSContext* cx = jsapi.cx(); @@ -6695,7 +6692,7 @@ nsGlobalWindowInner::RunTimeoutHandler(Timeout* aTimeout, options.setFileAndLine(filename, lineNo); options.setNoScriptRval(true); JS::Rooted global(aes.cx(), FastGetGlobalJSObject()); - nsresult rv = NS_OK; + nsresult rv; { nsJSUtils::ExecutionContext exec(aes.cx(), global); rv = exec.CompileAndExec(options, script); diff --git a/dom/base/nsGlobalWindowOuter.cpp b/dom/base/nsGlobalWindowOuter.cpp index 6a1b6a069d73..6ea790ccda1f 100644 --- a/dom/base/nsGlobalWindowOuter.cpp +++ b/dom/base/nsGlobalWindowOuter.cpp @@ -1695,7 +1695,7 @@ nsGlobalWindowOuter::SetNewDocument(nsIDocument* aDocument, bool reUseInnerWindow = (aForceReuseInnerWindow || wouldReuseInnerWindow) && GetCurrentInnerWindowInternal(); - nsresult rv = NS_OK; + nsresult rv; // We set mDoc even though this is an outer window to avoid // having to *always* reach into the inner window to find the diff --git a/dom/base/nsImageLoadingContent.cpp b/dom/base/nsImageLoadingContent.cpp index 8dbe0ad6adab..89d5778928d4 100644 --- a/dom/base/nsImageLoadingContent.cpp +++ b/dom/base/nsImageLoadingContent.cpp @@ -447,13 +447,12 @@ nsImageLoadingContent::AddObserver(imgINotificationObserver* aObserver) return; } - nsresult rv = NS_OK; RefPtr currentReq; if (mCurrentRequest) { // Scripted observers may not belong to the same document as us, so when we // create the imgRequestProxy, we shouldn't use any. This allows the request // to dispatch notifications from the correct scheduler group. - rv = mCurrentRequest->Clone(aObserver, nullptr, getter_AddRefs(currentReq)); + nsresult rv = mCurrentRequest->Clone(aObserver, nullptr, getter_AddRefs(currentReq)); if (NS_FAILED(rv)) { return; } @@ -462,7 +461,7 @@ nsImageLoadingContent::AddObserver(imgINotificationObserver* aObserver) RefPtr pendingReq; if (mPendingRequest) { // See above for why we don't use the loading document. - rv = mPendingRequest->Clone(aObserver, nullptr, getter_AddRefs(pendingReq)); + nsresult rv = mPendingRequest->Clone(aObserver, nullptr, getter_AddRefs(pendingReq)); if (NS_FAILED(rv)) { mCurrentRequest->CancelAndForgetObserver(NS_BINDING_ABORTED); return; diff --git a/dom/base/nsObjectLoadingContent.cpp b/dom/base/nsObjectLoadingContent.cpp index 8f5f1d82f2da..cfc8dea1e45e 100644 --- a/dom/base/nsObjectLoadingContent.cpp +++ b/dom/base/nsObjectLoadingContent.cpp @@ -725,7 +725,6 @@ nsObjectLoadingContent::InstantiatePluginInstance(bool aIsLoading) return NS_OK; } - nsresult rv = NS_ERROR_FAILURE; RefPtr pluginHost = nsPluginHost::GetInst(); if (!pluginHost) { @@ -742,9 +741,9 @@ nsObjectLoadingContent::InstantiatePluginInstance(bool aIsLoading) } RefPtr newOwner; - rv = pluginHost->InstantiatePluginInstance(mContentType, - mURI.get(), this, - getter_AddRefs(newOwner)); + nsresult rv = pluginHost->InstantiatePluginInstance(mContentType, + mURI.get(), this, + getter_AddRefs(newOwner)); // XXX(johns): We don't suspend native inside stopping plugins... if (appShell) { @@ -2150,7 +2149,6 @@ nsObjectLoadingContent::LoadObject(bool aNotify, mPendingCheckPluginStopEvent || mFinalListener) { MOZ_ASSERT_UNREACHABLE("Trying to load new plugin with existing content"); - rv = NS_ERROR_UNEXPECTED; return NS_OK; } @@ -2158,7 +2156,6 @@ nsObjectLoadingContent::LoadObject(bool aNotify, // If mChannel is set, mChannelLoaded should be set, and vice-versa if (mType != eType_Null && !!mChannel != mChannelLoaded) { MOZ_ASSERT_UNREACHABLE("Trying to load with bad channel state"); - rv = NS_ERROR_UNEXPECTED; return NS_OK; } From 47519229150c66e3914bc3511e788357b80a6f8c Mon Sep 17 00:00:00 2001 From: Mihir Iyer Date: Thu, 19 Jul 2018 12:30:48 -0700 Subject: [PATCH 14/26] Bug 1311892 - Implement 'safe'/'unsafe' for absolutely positioned boxes in grid and flexbox. r=dholbert MozReview-Commit-ID: 1xKmq7KFAM4 --HG-- extra : rebase_source : 64c143d908d31fa9bee522e4995b29ddf67749ca --- layout/generic/nsAbsoluteContainingBlock.cpp | 13 +++-- layout/generic/nsFlexContainerFrame.cpp | 7 ++- layout/generic/nsGridContainerFrame.cpp | 6 +- ...pos-staticpos-align-self-safe-001-ref.html | 47 ++++++++++++++++ ...-abspos-staticpos-align-self-safe-001.html | 49 +++++++++++++++++ ...pos-staticpos-align-self-safe-001-ref.html | 53 ++++++++++++++++++ ...-abspos-staticpos-align-self-safe-001.html | 55 +++++++++++++++++++ .../w3c-css/submitted/align3/reftest.list | 2 + 8 files changed, 220 insertions(+), 12 deletions(-) create mode 100644 layout/reftests/w3c-css/submitted/align3/flex-abspos-staticpos-align-self-safe-001-ref.html create mode 100644 layout/reftests/w3c-css/submitted/align3/flex-abspos-staticpos-align-self-safe-001.html create mode 100644 layout/reftests/w3c-css/submitted/align3/grid-abspos-staticpos-align-self-safe-001-ref.html create mode 100644 layout/reftests/w3c-css/submitted/align3/grid-abspos-staticpos-align-self-safe-001.html diff --git a/layout/generic/nsAbsoluteContainingBlock.cpp b/layout/generic/nsAbsoluteContainingBlock.cpp index a5c87059a16a..fec70bf8a95a 100644 --- a/layout/generic/nsAbsoluteContainingBlock.cpp +++ b/layout/generic/nsAbsoluteContainingBlock.cpp @@ -501,12 +501,13 @@ OffsetToAlignedStaticPos(const ReflowInput& aKidReflowInput, AlignJustifyFlags flags = AlignJustifyFlags::eIgnoreAutoMargins; uint16_t alignConst = aPlaceholderContainer->CSSAlignmentForAbsPosChild(aKidReflowInput, pcAxis); - // XXXdholbert: Handle in bug 1311892 (by conditionally - // setting AlignJustifyFlags::eOverflowSafe in |flags|.) For now, we behave - // as if "unsafe" was the specified value (which is basically equivalent to - // the default behavior, when no value is specified -- though the default - // behavior also has some [at-risk] extra nuance about scroll containers...) - // For now we ignore & strip off bits, until bug 1311892. + // If the safe bit in alignConst is set, set the safe flag in |flags|. + // Note: If no is specified, we behave as 'unsafe'. + // This doesn't quite match the css-align spec, which has an [at-risk] + // "smart default" behavior with some extra nuance about scroll containers. + if (alignConst & NS_STYLE_ALIGN_SAFE) { + flags |= AlignJustifyFlags::eOverflowSafe; + } alignConst &= ~NS_STYLE_ALIGN_FLAG_BITS; // Find out if placeholder-container & the OOF child have the same start-sides diff --git a/layout/generic/nsFlexContainerFrame.cpp b/layout/generic/nsFlexContainerFrame.cpp index 4e167341fc82..7f50f9d5e056 100644 --- a/layout/generic/nsFlexContainerFrame.cpp +++ b/layout/generic/nsFlexContainerFrame.cpp @@ -1217,6 +1217,7 @@ nsFlexContainerFrame::CSSAlignmentForAbsPosChild( : axisTracker.IsCrossAxisReversed(); uint8_t alignment; + uint8_t alignmentFlags = 0; if (isMainAxis) { alignment = SimplifyAlignOrJustifyContentForOneItem( containerStylePos->mJustifyContent, @@ -1234,8 +1235,8 @@ nsFlexContainerFrame::CSSAlignmentForAbsPosChild( // Single-line, or multi-line but the (one) line stretches to fill // container. Respect align-self. alignment = aChildRI.mStylePosition->UsedAlignSelf(Style()); - // XXX strip off bits until we implement it - // (bug 1311892) + // Extract and strip align flag bits + alignmentFlags = alignment & NS_STYLE_ALIGN_FLAG_BITS; alignment &= ~NS_STYLE_ALIGN_FLAG_BITS; if (alignment == NS_STYLE_ALIGN_NORMAL) { @@ -1269,7 +1270,7 @@ nsFlexContainerFrame::CSSAlignmentForAbsPosChild( alignment = NS_STYLE_ALIGN_END; } - return alignment; + return (alignment | alignmentFlags); } UniquePtr diff --git a/layout/generic/nsGridContainerFrame.cpp b/layout/generic/nsGridContainerFrame.cpp index 9d6314b720bf..c3cc092aaf82 100644 --- a/layout/generic/nsGridContainerFrame.cpp +++ b/layout/generic/nsGridContainerFrame.cpp @@ -6596,8 +6596,8 @@ nsGridContainerFrame::CSSAlignmentForAbsPosChild(const ReflowInput& aChildRI, aChildRI.mStylePosition->UsedJustifySelf(Style()) : aChildRI.mStylePosition->UsedAlignSelf(Style()); - // XXX strip off bits until we implement it - // (bug 1311892) + // Extract and strip the flag bits + uint16_t alignmentFlags = alignment & NS_STYLE_ALIGN_FLAG_BITS; alignment &= ~NS_STYLE_ALIGN_FLAG_BITS; if (alignment == NS_STYLE_ALIGN_NORMAL) { @@ -6628,7 +6628,7 @@ nsGridContainerFrame::CSSAlignmentForAbsPosChild(const ReflowInput& aChildRI, alignment = NS_STYLE_ALIGN_END; } - return alignment; + return (alignment | alignmentFlags); } nscoord diff --git a/layout/reftests/w3c-css/submitted/align3/flex-abspos-staticpos-align-self-safe-001-ref.html b/layout/reftests/w3c-css/submitted/align3/flex-abspos-staticpos-align-self-safe-001-ref.html new file mode 100644 index 000000000000..fb1fb93ea997 --- /dev/null +++ b/layout/reftests/w3c-css/submitted/align3/flex-abspos-staticpos-align-self-safe-001-ref.html @@ -0,0 +1,47 @@ + + + + + Reference: Testing safe overflow-position for align-self in absolutely positioned boxes in flex containers + + + + + +
+
+
+
+ + diff --git a/layout/reftests/w3c-css/submitted/align3/flex-abspos-staticpos-align-self-safe-001.html b/layout/reftests/w3c-css/submitted/align3/flex-abspos-staticpos-align-self-safe-001.html new file mode 100644 index 000000000000..2577489655c3 --- /dev/null +++ b/layout/reftests/w3c-css/submitted/align3/flex-abspos-staticpos-align-self-safe-001.html @@ -0,0 +1,49 @@ + + + + + CSS Test: Testing safe overflow-position for align-self in absolutely positioned boxes in flex containers + + + + + + + +
+
+
+
+ + diff --git a/layout/reftests/w3c-css/submitted/align3/grid-abspos-staticpos-align-self-safe-001-ref.html b/layout/reftests/w3c-css/submitted/align3/grid-abspos-staticpos-align-self-safe-001-ref.html new file mode 100644 index 000000000000..5e7fd9f9a9bb --- /dev/null +++ b/layout/reftests/w3c-css/submitted/align3/grid-abspos-staticpos-align-self-safe-001-ref.html @@ -0,0 +1,53 @@ + + + + + + Reference: Testing safe overflow-position for align-self and justify-self in absolutely positioned boxes in grid containers in both horizontal and vertical writing modes + + + + +
+
+
+
+ + diff --git a/layout/reftests/w3c-css/submitted/align3/grid-abspos-staticpos-align-self-safe-001.html b/layout/reftests/w3c-css/submitted/align3/grid-abspos-staticpos-align-self-safe-001.html new file mode 100644 index 000000000000..f8cfa793eb7a --- /dev/null +++ b/layout/reftests/w3c-css/submitted/align3/grid-abspos-staticpos-align-self-safe-001.html @@ -0,0 +1,55 @@ + + + + + + CSS Test: Testing safe overflow-position for align-self and justify-self in absolutely positioned boxes in grid containers in both horizontal and vertical writing modes + + + + + + +
+
+
+
+ + diff --git a/layout/reftests/w3c-css/submitted/align3/reftest.list b/layout/reftests/w3c-css/submitted/align3/reftest.list index 02f84227899f..69ac9afde219 100644 --- a/layout/reftests/w3c-css/submitted/align3/reftest.list +++ b/layout/reftests/w3c-css/submitted/align3/reftest.list @@ -24,6 +24,7 @@ == flex-abspos-staticpos-align-self-rtl-002.html flex-abspos-staticpos-align-self-rtl-002-ref.html == flex-abspos-staticpos-align-self-rtl-003.html flex-abspos-staticpos-align-self-rtl-003-ref.html == flex-abspos-staticpos-align-self-rtl-004.html flex-abspos-staticpos-align-self-rtl-004-ref.html +== flex-abspos-staticpos-align-self-safe-001.html flex-abspos-staticpos-align-self-safe-001-ref.html == flex-abspos-staticpos-align-self-vertWM-001.html flex-abspos-staticpos-align-self-vertWM-001-ref.html == flex-abspos-staticpos-align-self-vertWM-002.html flex-abspos-staticpos-align-self-vertWM-002-ref.html == flex-abspos-staticpos-align-self-vertWM-003.html flex-abspos-staticpos-align-self-vertWM-003-ref.html @@ -58,6 +59,7 @@ == grid-abspos-staticpos-align-self-rtl-002.html grid-abspos-staticpos-align-self-rtl-002-ref.html == grid-abspos-staticpos-align-self-rtl-003.html grid-abspos-staticpos-align-self-rtl-003-ref.html == grid-abspos-staticpos-align-self-rtl-004.html grid-abspos-staticpos-align-self-rtl-004-ref.html +== grid-abspos-staticpos-align-self-safe-001.html grid-abspos-staticpos-align-self-safe-001-ref.html == grid-abspos-staticpos-align-self-vertWM-001.html grid-abspos-staticpos-align-self-vertWM-001-ref.html == grid-abspos-staticpos-align-self-vertWM-002.html grid-abspos-staticpos-align-self-vertWM-002-ref.html == grid-abspos-staticpos-align-self-vertWM-003.html grid-abspos-staticpos-align-self-vertWM-003-ref.html From 2610f9e3c6118072e5758271dfa7db62a4229db8 Mon Sep 17 00:00:00 2001 From: Robert Bartlensky Date: Thu, 19 Jul 2018 11:16:42 +0100 Subject: [PATCH 15/26] Bug 1472681: Fix NULL_DEREFERENCE error in js/src/vm/EnvironmentObject-inl.h. r=luke MozReview-Commit-ID: 9p8jbWeZvhg --HG-- extra : rebase_source : 9c163c7682be1e515dea2015019799ad69fa7d94 --- js/src/vm/EnvironmentObject-inl.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/js/src/vm/EnvironmentObject-inl.h b/js/src/vm/EnvironmentObject-inl.h index 6c52d6ecee7b..673197d281c7 100644 --- a/js/src/vm/EnvironmentObject-inl.h +++ b/js/src/vm/EnvironmentObject-inl.h @@ -17,8 +17,11 @@ namespace js { inline LexicalEnvironmentObject& NearestEnclosingExtensibleLexicalEnvironment(JSObject* env) { - while (!IsExtensibleLexicalEnvironment(env)) + MOZ_ASSERT(env); + while (!IsExtensibleLexicalEnvironment(env)) { env = env->enclosingEnvironment(); + MOZ_ASSERT(env); + } return env->as(); } From 3a5e700ad122f031b2b41bec3b9159980a77564c Mon Sep 17 00:00:00 2001 From: Jonathan Kingston Date: Mon, 23 Jul 2018 22:45:26 +0000 Subject: [PATCH 16/26] Bug 1392710 - Add use counters for marquee events. r=smaug Differential Revision: https://phabricator.services.mozilla.com/D2209 --HG-- extra : moz-landing-system : lando --- dom/base/UseCounters.conf | 5 +++++ dom/events/EventListenerManager.cpp | 21 +++++++++++++++++++++ xpcom/ds/nsGkAtomList.h | 1 + 3 files changed, 27 insertions(+) diff --git a/dom/base/UseCounters.conf b/dom/base/UseCounters.conf index bd06d3075c8e..8d495b39a69d 100644 --- a/dom/base/UseCounters.conf +++ b/dom/base/UseCounters.conf @@ -87,6 +87,11 @@ method DataTransfer.mozGetDataAt attribute DataTransfer.mozUserCancelled attribute DataTransfer.mozSourceNode +// Marquee events +custom onstart sets a onstart event listener +custom onbounce sets a onbounce event listener +custom onfinish sets a onfinish event listener + // JavaScript feature usage custom JS_asmjs uses asm.js custom JS_wasm uses WebAssembly diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp index f8d2198cebb6..7b9fb0f6fa78 100644 --- a/dom/events/EventListenerManager.cpp +++ b/dom/events/EventListenerManager.cpp @@ -414,6 +414,27 @@ EventListenerManager::AddEventListenerInternal( if (nsPIDOMWindowInner* window = GetInnerWindowForTarget()) { window->SetHasSelectionChangeEventListeners(); } + } else if (aTypeAtom == nsGkAtoms::onstart) { + if (nsPIDOMWindowInner* window = GetInnerWindowForTarget()) { + nsCOMPtr doc = window->GetExtantDoc(); + if (doc) { + doc->SetDocumentAndPageUseCounter(eUseCounter_custom_onstart); + } + } + } else if (aTypeAtom == nsGkAtoms::onbounce) { + if (nsPIDOMWindowInner* window = GetInnerWindowForTarget()) { + nsCOMPtr doc = window->GetExtantDoc(); + if (doc) { + doc->SetDocumentAndPageUseCounter(eUseCounter_custom_onbounce); + } + } + } else if (aTypeAtom == nsGkAtoms::onfinish) { + if (nsPIDOMWindowInner* window = GetInnerWindowForTarget()) { + nsCOMPtr doc = window->GetExtantDoc(); + if (doc) { + doc->SetDocumentAndPageUseCounter(eUseCounter_custom_onfinish); + } + } } if (IsApzAwareListener(listener)) { diff --git a/xpcom/ds/nsGkAtomList.h b/xpcom/ds/nsGkAtomList.h index 1da54207d92b..1dac2449cc1e 100644 --- a/xpcom/ds/nsGkAtomList.h +++ b/xpcom/ds/nsGkAtomList.h @@ -692,6 +692,7 @@ GK_ATOM(onbeforescriptexecute, "onbeforescriptexecute") GK_ATOM(onbeforeunload, "onbeforeunload") GK_ATOM(onblocked, "onblocked") GK_ATOM(onblur, "onblur") +GK_ATOM(onbounce, "onbounce") GK_ATOM(onbroadcast, "onbroadcast") GK_ATOM(onbufferedamountlow, "onbufferedamountlow") GK_ATOM(oncached, "oncached") From b0259a189f1f63f6f436ca9a823fd033185e98e5 Mon Sep 17 00:00:00 2001 From: Dipen Patel Date: Fri, 13 Jul 2018 11:48:55 -0700 Subject: [PATCH 17/26] Bug 1475647 - Remove nsISSLStatusProvider interface. r=baku,Gijs,jchen,jryans,keeler,mcmanus - Access nsISSLStatus directly as a member of nsITransportSecurityInfo and nsISecureBrowserUI. This is part of a larger effort to consolidate nsISSLStatus and nsITransportSecurityInfo. - The TabParent implementation of GetSecInfo will always return null. - Removed unnecessary QueryInterface calls - Style adherence updates MozReview-Commit-ID: Dzy6t2zYljL --HG-- extra : rebase_source : b15f75e39d04c8485b4eb63416fd1f1e4175fafe --- browser/base/content/browser-siteIdentity.js | 8 ++--- browser/base/content/browser.js | 6 ++-- browser/base/content/pageinfo/security.js | 5 +--- devtools/shared/security/auth.js | 2 +- devtools/shared/security/socket.js | 2 +- devtools/shared/webconsole/network-helper.js | 1 - .../test/unit/test_security-info-parser.js | 3 +- .../test/unit/test_security-info-state.js | 3 +- .../unit/test_security-info-static-hpkp.js | 3 +- dom/ipc/TabParent.cpp | 10 +++++++ mobile/android/chrome/content/browser.js | 5 ++-- mobile/android/chrome/content/content.js | 3 +- .../modules/geckoview/GeckoViewProgress.jsm | 3 +- netwerk/base/nsISecureBrowserUI.idl | 2 ++ netwerk/protocol/http/AlternateServices.cpp | 2 -- netwerk/protocol/http/Http2Session.cpp | 2 -- netwerk/protocol/http/nsHttpChannel.cpp | 17 +++++------ netwerk/protocol/http/nsHttpNTLMAuth.cpp | 7 ++--- netwerk/socket/nsITransportSecurityInfo.idl | 3 ++ .../pki/resources/content/exceptionDialog.js | 30 ++++++++++++------- .../manager/ssl/TransportSecurityInfo.cpp | 3 +- security/manager/ssl/TransportSecurityInfo.h | 3 -- security/manager/ssl/moz.build | 1 - security/manager/ssl/nsISSLStatusProvider.idl | 13 -------- .../manager/ssl/nsSecureBrowserUIImpl.cpp | 20 +++++-------- security/manager/ssl/nsSecureBrowserUIImpl.h | 9 ++---- security/manager/ssl/tests/unit/head_psm.js | 9 ++---- .../unit/test_cert_overrides_read_only.js | 3 +- security/manager/ssl/tests/unit/test_ct.js | 3 +- .../ssl/tests/unit/test_session_resumption.js | 12 ++------ .../manager/ssl/tests/unit/test_ssl_status.js | 8 ++--- security/manager/tools/getHSTSPreloadList.js | 4 +-- .../firefox/firefox_puppeteer/api/security.py | 3 +- toolkit/content/browser-child.js | 18 ++++++----- toolkit/modules/CertUtils.jsm | 2 +- toolkit/modules/RemoteSecurityUI.jsm | 12 ++++---- toolkit/modules/RemoteWebProgress.jsm | 12 ++++---- toolkit/modules/addons/SecurityInfo.jsm | 1 - .../tests/chrome/test_bug544442_checkCert.xul | 2 +- toolkit/mozapps/update/nsUpdateService.js | 2 +- 40 files changed, 110 insertions(+), 147 deletions(-) delete mode 100644 security/manager/ssl/nsISSLStatusProvider.idl diff --git a/browser/base/content/browser-siteIdentity.js b/browser/base/content/browser-siteIdentity.js index f21fb38f65b3..ca529b5ac125 100644 --- a/browser/base/content/browser-siteIdentity.js +++ b/browser/base/content/browser-siteIdentity.js @@ -347,12 +347,8 @@ var gIdentityHandler = { // Firstly, populate the state properties required to display the UI. See // the documentation of the individual properties for details. this.setURI(uri); - this._sslStatus = gBrowser.securityUI - .QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; - if (this._sslStatus) { - this._sslStatus.QueryInterface(Ci.nsISSLStatus); - } + this._sslStatus = gBrowser.securityUI.secInfo && + gBrowser.securityUI.secInfo.SSLStatus; // Then, update the user interface with the available data. this.refreshIdentityBlock(); diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index 8153922b300f..cb7161319824 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -2996,8 +2996,7 @@ var BrowserOnClick = { } securityInfo = getSecurityInfo(securityInfoAsString); - sslStatus = securityInfo.QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + sslStatus = securityInfo.SSLStatus; let params = { exceptionAdded: false, sslStatus }; @@ -3038,8 +3037,7 @@ var BrowserOnClick = { } securityInfo = getSecurityInfo(securityInfoAsString); - sslStatus = securityInfo.QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + sslStatus = securityInfo.SSLStatus; let errorInfo = getDetailedCertErrorInfo(location, securityInfo); let validityInfo = { diff --git a/browser/base/content/pageinfo/security.js b/browser/base/content/pageinfo/security.js index a57560e794e7..2ab144818ecd 100644 --- a/browser/base/content/pageinfo/security.js +++ b/browser/base/content/pageinfo/security.js @@ -27,7 +27,6 @@ var security = { }, _getSecurityInfo() { - const nsISSLStatusProvider = Ci.nsISSLStatusProvider; const nsISSLStatus = Ci.nsISSLStatus; // We don't have separate info for a frame, return null until further notice @@ -50,11 +49,9 @@ var security = { (ui.state & Ci.nsIWebProgressListener.STATE_IS_INSECURE); var isEV = (ui.state & Ci.nsIWebProgressListener.STATE_IDENTITY_EV_TOPLEVEL); - ui.QueryInterface(nsISSLStatusProvider); - var status = ui.SSLStatus; + var status = ui.secInfo && ui.secInfo.SSLStatus; if (!isInsecure && status) { - status.QueryInterface(nsISSLStatus); var cert = status.serverCert; var issuerName = cert.issuerOrganization || cert.issuerName; diff --git a/devtools/shared/security/auth.js b/devtools/shared/security/auth.js index 98c325186513..86b304c59432 100644 --- a/devtools/shared/security/auth.js +++ b/devtools/shared/security/auth.js @@ -300,7 +300,7 @@ OOBCert.Client.prototype = { // Client verifies that Server's cert matches hash(ServerCert) from the // advertisement dumpv("Validate server cert hash"); - const serverCert = socket.securityInfo.QueryInterface(Ci.nsISSLStatusProvider) + const serverCert = socket.securityInfo.QueryInterface(Ci.nsITransportSecurityInfo) .SSLStatus.serverCert; const advertisedCert = cert; if (serverCert.sha256Fingerprint != advertisedCert.sha256) { diff --git a/devtools/shared/security/socket.js b/devtools/shared/security/socket.js index 55d70ac598fe..98fe37ea38b0 100644 --- a/devtools/shared/security/socket.js +++ b/devtools/shared/security/socket.js @@ -353,7 +353,7 @@ function _isInputAlive(input) { */ function _storeCertOverride(s, host, port) { // eslint-disable-next-line no-shadow - const cert = s.securityInfo.QueryInterface(Ci.nsISSLStatusProvider) + const cert = s.securityInfo.QueryInterface(Ci.nsITransportSecurityInfo) .SSLStatus.serverCert; const overrideBits = Ci.nsICertOverrideService.ERROR_UNTRUSTED | Ci.nsICertOverrideService.ERROR_MISMATCH; diff --git a/devtools/shared/webconsole/network-helper.js b/devtools/shared/webconsole/network-helper.js index 663d70f43213..9c590335f075 100644 --- a/devtools/shared/webconsole/network-helper.js +++ b/devtools/shared/webconsole/network-helper.js @@ -600,7 +600,6 @@ var NetworkHelper = { */ securityInfo.QueryInterface(Ci.nsITransportSecurityInfo); - securityInfo.QueryInterface(Ci.nsISSLStatusProvider); const wpl = Ci.nsIWebProgressListener; const NSSErrorsService = Cc["@mozilla.org/nss_errors_service;1"] diff --git a/devtools/shared/webconsole/test/unit/test_security-info-parser.js b/devtools/shared/webconsole/test/unit/test_security-info-parser.js index 3edddcd97196..fcf05f7d8065 100644 --- a/devtools/shared/webconsole/test/unit/test_security-info-parser.js +++ b/devtools/shared/webconsole/test/unit/test_security-info-parser.js @@ -33,8 +33,7 @@ const MockCertificate = { }; const MockSecurityInfo = { - QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo, - Ci.nsISSLStatusProvider]), + QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo]), securityState: wpl.STATE_IS_SECURE, errorCode: 0, SSLStatus: { diff --git a/devtools/shared/webconsole/test/unit/test_security-info-state.js b/devtools/shared/webconsole/test/unit/test_security-info-state.js index 3d458ef09a21..8e269c4fe1d5 100644 --- a/devtools/shared/webconsole/test/unit/test_security-info-state.js +++ b/devtools/shared/webconsole/test/unit/test_security-info-state.js @@ -19,8 +19,7 @@ Object.defineProperty(this, "NetworkHelper", { const wpl = Ci.nsIWebProgressListener; const MockSecurityInfo = { - QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo, - Ci.nsISSLStatusProvider]), + QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo]), securityState: wpl.STATE_IS_BROKEN, errorCode: 0, SSLStatus: { diff --git a/devtools/shared/webconsole/test/unit/test_security-info-static-hpkp.js b/devtools/shared/webconsole/test/unit/test_security-info-static-hpkp.js index e710d758f9af..e97d43a9c6d9 100644 --- a/devtools/shared/webconsole/test/unit/test_security-info-static-hpkp.js +++ b/devtools/shared/webconsole/test/unit/test_security-info-static-hpkp.js @@ -20,8 +20,7 @@ Object.defineProperty(this, "NetworkHelper", { const wpl = Ci.nsIWebProgressListener; const MockSecurityInfo = { - QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo, - Ci.nsISSLStatusProvider]), + QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo]), securityState: wpl.STATE_IS_SECURE, errorCode: 0, SSLStatus: { diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index 55eb508659a3..4d6385d05038 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -59,6 +59,7 @@ #include "nsIInterfaceRequestorUtils.h" #include "nsILoadInfo.h" #include "nsIPromptFactory.h" +#include "nsITransportSecurityInfo.h" #include "nsIURI.h" #include "nsIWindowWatcher.h" #include "nsIWebBrowserChrome.h" @@ -890,6 +891,15 @@ TabParent::GetState(uint32_t *aState) return NS_OK; } +NS_IMETHODIMP +TabParent::GetSecInfo(nsITransportSecurityInfo** _result) +{ + NS_ENSURE_ARG_POINTER(_result); + NS_WARNING("TransportSecurityInfo not valid here"); + *_result = nullptr; + return NS_OK; +} + NS_IMETHODIMP TabParent::SetDocShell(nsIDocShell *aDocShell) { diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index 9edda1bb867e..02637d24f521 100644 --- a/mobile/android/chrome/content/browser.js +++ b/mobile/android/chrome/content/browser.js @@ -5688,9 +5688,8 @@ var IdentityHandler = { * (if available). Return the data needed to update the UI. */ checkIdentity: function checkIdentity(aState, aBrowser) { - this._lastStatus = aBrowser.securityUI - .QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + this._lastStatus = aBrowser.securityUI.secInfo && + aBrowser.securityUI.secInfo.SSLStatus; // Don't pass in the actual location object, since it can cause us to // hold on to the window object too long. Just pass in the fields we diff --git a/mobile/android/chrome/content/content.js b/mobile/android/chrome/content/content.js index f7ea084b5e9a..dd5de8941d56 100644 --- a/mobile/android/chrome/content/content.js +++ b/mobile/android/chrome/content/content.js @@ -360,8 +360,7 @@ var AboutCertErrorListener = { let securityInfo = docShell.failedChannel && docShell.failedChannel.securityInfo; securityInfo.QueryInterface(Ci.nsITransportSecurityInfo) .QueryInterface(Ci.nsISerializable); - let sslStatus = securityInfo.QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + let sslStatus = securityInfo.SSLStatus; this._setTechDetails(sslStatus, securityInfo, ownerDoc.location.href); }, }; diff --git a/mobile/android/modules/geckoview/GeckoViewProgress.jsm b/mobile/android/modules/geckoview/GeckoViewProgress.jsm index c2f9db5facda..2ed06f85f3cd 100644 --- a/mobile/android/modules/geckoview/GeckoViewProgress.jsm +++ b/mobile/android/modules/geckoview/GeckoViewProgress.jsm @@ -159,8 +159,7 @@ var IdentityHandler = { result.host = uri.host; } - let status = aBrowser.securityUI.QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus.QueryInterface(Ci.nsISSLStatus); + let status = aBrowser.securityUI.secInfo.SSLStatus; let cert = status.serverCert; result.organization = cert.organization; diff --git a/netwerk/base/nsISecureBrowserUI.idl b/netwerk/base/nsISecureBrowserUI.idl index a2782d79b142..3f12d7bd3871 100644 --- a/netwerk/base/nsISecureBrowserUI.idl +++ b/netwerk/base/nsISecureBrowserUI.idl @@ -8,6 +8,7 @@ interface mozIDOMWindowProxy; interface nsIDocShell; +interface nsITransportSecurityInfo; [scriptable, uuid(718c662a-f810-4a80-a6c9-0b1810ecade2)] interface nsISecureBrowserUI : nsISupports @@ -16,6 +17,7 @@ interface nsISecureBrowserUI : nsISupports void setDocShell(in nsIDocShell docShell); readonly attribute unsigned long state; + readonly attribute nsITransportSecurityInfo secInfo; }; %{C++ diff --git a/netwerk/protocol/http/AlternateServices.cpp b/netwerk/protocol/http/AlternateServices.cpp index 4600d138863a..90e714efa3fa 100644 --- a/netwerk/protocol/http/AlternateServices.cpp +++ b/netwerk/protocol/http/AlternateServices.cpp @@ -15,8 +15,6 @@ #include "nsThreadUtils.h" #include "nsHttpTransaction.h" #include "NullHttpTransaction.h" -#include "nsISSLStatusProvider.h" -#include "nsISSLStatus.h" #include "nsISSLSocketControl.h" #include "nsIWellKnownOpportunisticUtils.h" diff --git a/netwerk/protocol/http/Http2Session.cpp b/netwerk/protocol/http/Http2Session.cpp index e3417e0ffcba..2ad359c11a9a 100644 --- a/netwerk/protocol/http/Http2Session.cpp +++ b/netwerk/protocol/http/Http2Session.cpp @@ -27,8 +27,6 @@ #include "nsHttpConnection.h" #include "nsIRequestContext.h" #include "nsISSLSocketControl.h" -#include "nsISSLStatus.h" -#include "nsISSLStatusProvider.h" #include "nsISupportsPriority.h" #include "nsStandardURL.h" #include "nsURLHelper.h" diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 532e9334ccfe..65ca1095f894 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -69,7 +69,6 @@ #include "nsIScriptError.h" #include "nsIScriptSecurityManager.h" #include "nsISSLStatus.h" -#include "nsISSLStatusProvider.h" #include "nsITransportSecurityInfo.h" #include "nsIWebProgressListener.h" #include "LoadContextInfo.h" @@ -1857,11 +1856,11 @@ nsHttpChannel::ProcessSecurityHeaders() uint32_t flags = NS_UsePrivateBrowsing(this) ? nsISocketProvider::NO_PERMANENT_STORAGE : 0; - // Get the SSLStatus - nsCOMPtr sslprov = do_QueryInterface(mSecurityInfo); - NS_ENSURE_TRUE(sslprov, NS_ERROR_FAILURE); + // Get the TransportSecurityInfo + nsCOMPtr transSecInfo = do_QueryInterface(mSecurityInfo); + NS_ENSURE_TRUE(transSecInfo, NS_ERROR_FAILURE); nsCOMPtr sslStatus; - rv = sslprov->GetSSLStatus(getter_AddRefs(sslStatus)); + rv = transSecInfo->GetSSLStatus(getter_AddRefs(sslStatus)); NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_TRUE(sslStatus, NS_ERROR_FAILURE); @@ -1992,17 +1991,15 @@ nsHttpChannel::ProcessSSLInformation() !IsHTTPS() || mPrivateBrowsing) return; - nsCOMPtr statusProvider = + nsCOMPtr securityInfo = do_QueryInterface(mSecurityInfo); - if (!statusProvider) + if (!securityInfo) return; nsCOMPtr sslstat; - statusProvider->GetSSLStatus(getter_AddRefs(sslstat)); + securityInfo->GetSSLStatus(getter_AddRefs(sslstat)); if (!sslstat) return; - nsCOMPtr securityInfo = - do_QueryInterface(mSecurityInfo); uint32_t state; if (securityInfo && NS_SUCCEEDED(securityInfo->GetSecurityState(&state)) && diff --git a/netwerk/protocol/http/nsHttpNTLMAuth.cpp b/netwerk/protocol/http/nsHttpNTLMAuth.cpp index 718197060c09..f59ba704d95c 100644 --- a/netwerk/protocol/http/nsHttpNTLMAuth.cpp +++ b/netwerk/protocol/http/nsHttpNTLMAuth.cpp @@ -24,7 +24,6 @@ #include "nsIChannel.h" #include "nsIX509Cert.h" #include "nsISSLStatus.h" -#include "nsISSLStatusProvider.h" #endif #include "mozilla/Attributes.h" #include "mozilla/Base64.h" @@ -335,12 +334,12 @@ nsHttpNTLMAuth::GenerateCredentials(nsIHttpAuthenticableChannel *authChannel, if (NS_FAILED(rv)) return rv; - nsCOMPtr statusProvider = + nsCOMPtr secInfo = do_QueryInterface(security); - if (mUseNative && statusProvider) { + if (mUseNative && secInfo) { nsCOMPtr status; - rv = statusProvider->GetSSLStatus(getter_AddRefs(status)); + rv = secInfo->GetSSLStatus(getter_AddRefs(status)); if (NS_FAILED(rv)) return rv; diff --git a/netwerk/socket/nsITransportSecurityInfo.idl b/netwerk/socket/nsITransportSecurityInfo.idl index a0a165038cc9..9ed3d2fdd4c6 100644 --- a/netwerk/socket/nsITransportSecurityInfo.idl +++ b/netwerk/socket/nsITransportSecurityInfo.idl @@ -6,6 +6,7 @@ #include "nsISupports.idl" +interface nsISSLStatus; interface nsIX509CertList; [builtinclass, scriptable, uuid(216112d3-28bc-4671-b057-f98cc09ba1ea)] @@ -21,5 +22,7 @@ interface nsITransportSecurityInfo : nsISupports { * If verification succeeded, this will be null. */ readonly attribute nsIX509CertList failedCertChain; + + readonly attribute nsISSLStatus SSLStatus; }; diff --git a/security/manager/pki/resources/content/exceptionDialog.js b/security/manager/pki/resources/content/exceptionDialog.js index 72c7d455e958..3629028c5e4c 100644 --- a/security/manager/pki/resources/content/exceptionDialog.js +++ b/security/manager/pki/resources/content/exceptionDialog.js @@ -26,7 +26,9 @@ function initExceptionDialog() { gNsISecTel = Ci.nsISecurityUITelemetry; var brandName = gBundleBrand.getString("brandShortName"); - setText("warningText", gPKIBundle.getFormattedString("addExceptionBrandedWarning2", [brandName])); + setText("warningText", + gPKIBundle.getFormattedString("addExceptionBrandedWarning2", + [brandName])); gDialog.getButton("extra1").disabled = true; var args = window.arguments; @@ -75,7 +77,7 @@ function initExceptionDialog() { function grabCert(req, evt) { if (req.channel && req.channel.securityInfo) { gSSLStatus = req.channel.securityInfo - .QueryInterface(Ci.nsISSLStatusProvider).SSLStatus; + .QueryInterface(Ci.nsITransportSecurityInfo).SSLStatus; gCert = gSSLStatus ? gSSLStatus.QueryInterface(Ci.nsISSLStatus).serverCert : null; } @@ -159,7 +161,8 @@ function resetDialog() { */ function handleTextChange() { var checkCertButton = document.getElementById("checkCertButton"); - checkCertButton.disabled = !(document.getElementById("locationTextBox").value); + checkCertButton.disabled = + !(document.getElementById("locationTextBox").value); if (gNeedReset) { gNeedReset = false; resetDialog(); @@ -201,7 +204,8 @@ function updateCertStatus() { } } if (gSSLStatus.isUntrusted) { - bucketId += gNsISecTel.WARNING_BAD_CERT_TOP_ADD_EXCEPTION_FLAG_UNTRUSTED; + bucketId += + gNsISecTel.WARNING_BAD_CERT_TOP_ADD_EXCEPTION_FLAG_UNTRUSTED; if (!use1) { use1 = true; shortDesc = uts; @@ -229,7 +233,8 @@ function updateCertStatus() { pe.disabled = inPrivateBrowsing; pe.checked = !inPrivateBrowsing; - setText("headerDescription", gPKIBundle.getString("addExceptionInvalidHeader")); + setText("headerDescription", + gPKIBundle.getString("addExceptionInvalidHeader")); } else { shortDesc = "addExceptionValidShort"; longDesc = "addExceptionValidLong"; @@ -301,22 +306,27 @@ function addException() { var overrideService = Cc["@mozilla.org/security/certoverride;1"] .getService(Ci.nsICertOverrideService); var flags = 0; - let confirmBucketId = gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_BASE; + let confirmBucketId = + gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_BASE; if (gSSLStatus.isUntrusted) { flags |= overrideService.ERROR_UNTRUSTED; - confirmBucketId += gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_UNTRUSTED; + confirmBucketId += + gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_UNTRUSTED; } if (gSSLStatus.isDomainMismatch) { flags |= overrideService.ERROR_MISMATCH; - confirmBucketId += gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_DOMAIN; + confirmBucketId += + gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_DOMAIN; } if (gSSLStatus.isNotValidAtThisTime) { flags |= overrideService.ERROR_TIME; - confirmBucketId += gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_TIME; + confirmBucketId += + gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_TIME; } var permanentCheckbox = document.getElementById("permanent"); - var shouldStorePermanently = permanentCheckbox.checked && !inPrivateBrowsingMode(); + var shouldStorePermanently = permanentCheckbox.checked && + !inPrivateBrowsingMode(); if (!permanentCheckbox.checked) { gSecHistogram.add(gNsISecTel.WARNING_BAD_CERT_TOP_DONT_REMEMBER_EXCEPTION); } diff --git a/security/manager/ssl/TransportSecurityInfo.cpp b/security/manager/ssl/TransportSecurityInfo.cpp index b39949b6c487..287ba44e6c92 100644 --- a/security/manager/ssl/TransportSecurityInfo.cpp +++ b/security/manager/ssl/TransportSecurityInfo.cpp @@ -51,7 +51,6 @@ TransportSecurityInfo::TransportSecurityInfo() NS_IMPL_ISUPPORTS(TransportSecurityInfo, nsITransportSecurityInfo, nsIInterfaceRequestor, - nsISSLStatusProvider, nsIAssociatedContentSecurity, nsISerializable, nsIClassInfo) @@ -365,7 +364,7 @@ TransportSecurityInfo::GetClassIDNoAlloc(nsCID *aClassIDNoAlloc) return NS_OK; } -nsresult +NS_IMETHODIMP TransportSecurityInfo::GetSSLStatus(nsISSLStatus** _result) { NS_ENSURE_ARG_POINTER(_result); diff --git a/security/manager/ssl/TransportSecurityInfo.h b/security/manager/ssl/TransportSecurityInfo.h index 86974e641ac5..4361d8249d87 100644 --- a/security/manager/ssl/TransportSecurityInfo.h +++ b/security/manager/ssl/TransportSecurityInfo.h @@ -16,7 +16,6 @@ #include "nsDataHashtable.h" #include "nsIAssociatedContentSecurity.h" #include "nsIInterfaceRequestor.h" -#include "nsISSLStatusProvider.h" #include "nsITransportSecurityInfo.h" #include "nsSSLStatus.h" #include "nsString.h" @@ -26,7 +25,6 @@ namespace mozilla { namespace psm { class TransportSecurityInfo : public nsITransportSecurityInfo , public nsIInterfaceRequestor - , public nsISSLStatusProvider , public nsIAssociatedContentSecurity , public nsISerializable , public nsIClassInfo @@ -39,7 +37,6 @@ public: NS_DECL_THREADSAFE_ISUPPORTS NS_DECL_NSITRANSPORTSECURITYINFO NS_DECL_NSIINTERFACEREQUESTOR - NS_DECL_NSISSLSTATUSPROVIDER NS_DECL_NSIASSOCIATEDCONTENTSECURITY NS_DECL_NSISERIALIZABLE NS_DECL_NSICLASSINFO diff --git a/security/manager/ssl/moz.build b/security/manager/ssl/moz.build index d4092d2773cc..d8d7512967c6 100644 --- a/security/manager/ssl/moz.build +++ b/security/manager/ssl/moz.build @@ -36,7 +36,6 @@ XPIDL_SOURCES += [ 'nsISecurityUITelemetry.idl', 'nsISiteSecurityService.idl', 'nsISSLStatus.idl', - 'nsISSLStatusProvider.idl', 'nsITokenDialogs.idl', 'nsITokenPasswordDialogs.idl', 'nsIX509Cert.idl', diff --git a/security/manager/ssl/nsISSLStatusProvider.idl b/security/manager/ssl/nsISSLStatusProvider.idl deleted file mode 100644 index 83048f179436..000000000000 --- a/security/manager/ssl/nsISSLStatusProvider.idl +++ /dev/null @@ -1,13 +0,0 @@ -/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* 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 "nsISupports.idl" - -interface nsISSLStatus; - -[scriptable, uuid(179b1ab1-0950-4427-9556-6f496dc4a27f)] -interface nsISSLStatusProvider : nsISupports { - readonly attribute nsISSLStatus SSLStatus; -}; diff --git a/security/manager/ssl/nsSecureBrowserUIImpl.cpp b/security/manager/ssl/nsSecureBrowserUIImpl.cpp index a2f24df7c4af..48cf86723d15 100644 --- a/security/manager/ssl/nsSecureBrowserUIImpl.cpp +++ b/security/manager/ssl/nsSecureBrowserUIImpl.cpp @@ -89,8 +89,7 @@ nsSecureBrowserUIImpl::nsSecureBrowserUIImpl() NS_IMPL_ISUPPORTS(nsSecureBrowserUIImpl, nsISecureBrowserUI, nsIWebProgressListener, - nsISupportsWeakReference, - nsISSLStatusProvider) + nsISupportsWeakReference) NS_IMETHODIMP nsSecureBrowserUIImpl::Init(mozIDOMWindowProxy* aWindow) @@ -374,23 +373,21 @@ nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest* aRequest, ("SecureUI:%p: OnStateChange: remember mNewToplevelSecurityState => %x\n", this, mNewToplevelSecurityState)); - nsCOMPtr sp(do_QueryInterface(info)); - if (sp) { + nsCOMPtr psmInfo(do_QueryInterface(info)); + if (psmInfo) { // Ignore result updateStatus = true; - (void) sp->GetSSLStatus(getter_AddRefs(temp_SSLStatus)); + (void) psmInfo->GetSSLStatus(getter_AddRefs(temp_SSLStatus)); if (temp_SSLStatus) { bool aTemp; if (NS_SUCCEEDED(temp_SSLStatus->GetIsExtendedValidation(&aTemp))) { mNewToplevelIsEV = aTemp; } } + mSecInfo = psmInfo; } mNewToplevelSecurityStateKnown = true; - if (updateStatus) { - mSSLStatus = temp_SSLStatus; - } MOZ_LOG(gSecureDocLog, LogLevel::Debug, ("SecureUI:%p: remember securityInfo %p\n", this, info)); @@ -1014,7 +1011,7 @@ nsSecureBrowserUIImpl::UpdateSecurityState(nsIRequest* aRequest, // If we have no security, we also shouldn't have any SSL status. if (newSecurityState == lis_no_security) { - mSSLStatus = nullptr; + mSecInfo = nullptr; } } @@ -1166,9 +1163,8 @@ nsSecureBrowserUIImpl::OnSecurityChange(nsIWebProgress* aWebProgress, return NS_OK; } -// nsISSLStatusProvider methods NS_IMETHODIMP -nsSecureBrowserUIImpl::GetSSLStatus(nsISSLStatus** _result) +nsSecureBrowserUIImpl::GetSecInfo(nsITransportSecurityInfo** _result) { NS_ENSURE_ARG_POINTER(_result); MOZ_ASSERT(NS_IsMainThread()); @@ -1187,7 +1183,7 @@ nsSecureBrowserUIImpl::GetSSLStatus(nsISSLStatus** _result) return NS_OK; } - *_result = mSSLStatus; + *_result = mSecInfo; NS_IF_ADDREF(*_result); return NS_OK; diff --git a/security/manager/ssl/nsSecureBrowserUIImpl.h b/security/manager/ssl/nsSecureBrowserUIImpl.h index 0c8fae120e6d..d8c3b0f9a7cf 100644 --- a/security/manager/ssl/nsSecureBrowserUIImpl.h +++ b/security/manager/ssl/nsSecureBrowserUIImpl.h @@ -10,14 +10,13 @@ #include "mozilla/ReentrancyGuard.h" #include "nsCOMPtr.h" #include "nsINetUtil.h" -#include "nsISSLStatusProvider.h" #include "nsISecureBrowserUI.h" #include "nsISecurityEventSink.h" #include "nsIURI.h" #include "nsIWebProgressListener.h" #include "nsWeakReference.h" -class nsISSLStatus; +class nsITransportSecurityInfo; class nsIChannel; #define NS_SECURE_BROWSER_UI_CID \ @@ -26,8 +25,7 @@ class nsIChannel; class nsSecureBrowserUIImpl : public nsISecureBrowserUI, public nsIWebProgressListener, - public nsSupportsWeakReference, - public nsISSLStatusProvider + public nsSupportsWeakReference { friend class mozilla::ReentrancyGuard; @@ -37,7 +35,6 @@ public: NS_DECL_ISUPPORTS NS_DECL_NSIWEBPROGRESSLISTENER NS_DECL_NSISECUREBROWSERUI - NS_DECL_NSISSLSTATUSPROVIDER protected: virtual ~nsSecureBrowserUIImpl() {}; @@ -87,7 +84,7 @@ protected: void ObtainEventSink(nsIChannel *channel, nsCOMPtr &sink); - nsCOMPtr mSSLStatus; + nsCOMPtr mSecInfo; nsCOMPtr mCurrentToplevelSecurityInfo; PLDHashTable mTransferringRequests; diff --git a/security/manager/ssl/tests/unit/head_psm.js b/security/manager/ssl/tests/unit/head_psm.js index 5a292a36c42d..5429475586d1 100644 --- a/security/manager/ssl/tests/unit/head_psm.js +++ b/security/manager/ssl/tests/unit/head_psm.js @@ -720,8 +720,7 @@ FakeSSLStatus.prototype = { // Helper function for add_cert_override_test. Probably doesn't need to be // called directly. function add_cert_override(aHost, aExpectedBits, aSecurityInfo) { - let sslstatus = aSecurityInfo.QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + let sslstatus = aSecurityInfo.SSLStatus; let bits = (sslstatus.isUntrusted ? Ci.nsICertOverrideService.ERROR_UNTRUSTED : 0) | (sslstatus.isDomainMismatch ? Ci.nsICertOverrideService.ERROR_MISMATCH : 0) | @@ -749,8 +748,7 @@ function add_cert_override_test(aHost, aExpectedBits, aExpectedError, Ci.nsIWebProgressListener.STATE_CERT_USER_OVERRIDDEN, "Cert override flag should be set on the security state"); if (aExpectedSSLStatus) { - let sslstatus = aSecurityInfo.QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + let sslstatus = aSecurityInfo.SSLStatus; if (aExpectedSSLStatus.failedCertChain) { ok(aExpectedSSLStatus.failedCertChain.equals(sslstatus.failedCertChain)); } @@ -763,8 +761,7 @@ function add_cert_override_test(aHost, aExpectedBits, aExpectedError, // SSLStatus set on it. In this case, the error was not overridable anyway, so // we consider it a success. function attempt_adding_cert_override(aHost, aExpectedBits, aSecurityInfo) { - let sslstatus = aSecurityInfo.QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + let sslstatus = aSecurityInfo.SSLStatus; if (sslstatus) { let bits = (sslstatus.isUntrusted ? Ci.nsICertOverrideService.ERROR_UNTRUSTED : 0) | diff --git a/security/manager/ssl/tests/unit/test_cert_overrides_read_only.js b/security/manager/ssl/tests/unit/test_cert_overrides_read_only.js index 3bdc0c1077be..9910827cfba8 100644 --- a/security/manager/ssl/tests/unit/test_cert_overrides_read_only.js +++ b/security/manager/ssl/tests/unit/test_cert_overrides_read_only.js @@ -10,8 +10,7 @@ // Helper function for add_read_only_cert_override_test. Probably doesn't need // to be called directly. function add_read_only_cert_override(aHost, aExpectedBits, aSecurityInfo) { - let sslstatus = aSecurityInfo.QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + let sslstatus = aSecurityInfo.SSLStatus; let bits = (sslstatus.isUntrusted ? Ci.nsICertOverrideService.ERROR_UNTRUSTED : 0) | (sslstatus.isDomainMismatch ? Ci.nsICertOverrideService.ERROR_MISMATCH : 0) | diff --git a/security/manager/ssl/tests/unit/test_ct.js b/security/manager/ssl/tests/unit/test_ct.js index 08eb74298bab..57dd2c032ed5 100644 --- a/security/manager/ssl/tests/unit/test_ct.js +++ b/security/manager/ssl/tests/unit/test_ct.js @@ -11,8 +11,7 @@ const certdb = Cc["@mozilla.org/security/x509certdb;1"] function expectCT(value) { return (securityInfo) => { - let sslStatus = securityInfo.QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + let sslStatus = securityInfo.SSLStatus; Assert.equal(sslStatus.certificateTransparencyStatus, value, "actual and expected CT status should match"); }; diff --git a/security/manager/ssl/tests/unit/test_session_resumption.js b/security/manager/ssl/tests/unit/test_session_resumption.js index 58303b3e9a20..4923e14f1b68 100644 --- a/security/manager/ssl/tests/unit/test_session_resumption.js +++ b/security/manager/ssl/tests/unit/test_session_resumption.js @@ -41,9 +41,7 @@ function add_resume_non_ev_with_override_test() { ok(transportSecurityInfo.securityState & Ci.nsIWebProgressListener.STATE_CERT_USER_OVERRIDDEN, "expired.example.com should have STATE_CERT_USER_OVERRIDDEN flag"); - let sslStatus = transportSecurityInfo - .QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + let sslStatus = transportSecurityInfo.SSLStatus; ok(!sslStatus.succeededCertChain, "ev-test.example.com should not have succeededCertChain set"); ok(!sslStatus.isDomainMismatch, @@ -68,9 +66,7 @@ function add_one_ev_test() { ok(!(transportSecurityInfo.securityState & Ci.nsIWebProgressListener.STATE_CERT_USER_OVERRIDDEN), "ev-test.example.com should not have STATE_CERT_USER_OVERRIDDEN flag"); - let sslStatus = transportSecurityInfo - .QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + let sslStatus = transportSecurityInfo.SSLStatus; ok(sslStatus.succeededCertChain, "ev-test.example.com should have succeededCertChain set"); ok(!sslStatus.isDomainMismatch, @@ -130,9 +126,7 @@ function add_one_non_ev_test() { ok(!(transportSecurityInfo.securityState & Ci.nsIWebProgressListener.STATE_CERT_USER_OVERRIDDEN), `${GOOD_DOMAIN} should not have STATE_CERT_USER_OVERRIDDEN flag`); - let sslStatus = transportSecurityInfo - .QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + let sslStatus = transportSecurityInfo.SSLStatus; ok(sslStatus.succeededCertChain, `${GOOD_DOMAIN} should have succeededCertChain set`); ok(!sslStatus.isDomainMismatch, diff --git a/security/manager/ssl/tests/unit/test_ssl_status.js b/security/manager/ssl/tests/unit/test_ssl_status.js index 2352e584eedb..e652e78ca421 100644 --- a/security/manager/ssl/tests/unit/test_ssl_status.js +++ b/security/manager/ssl/tests/unit/test_ssl_status.js @@ -20,8 +20,8 @@ function run_test() { // succeededCertChain should be set as expected) add_connection_test( "good.include-subdomains.pinning.example.com", PRErrorCodeSuccess, null, - function withSecurityInfo(aSSLStatus) { - let sslstatus = aSSLStatus.QueryInterface(Ci.nsISSLStatusProvider).SSLStatus; + function withSecurityInfo(aSecInfo) { + let sslstatus = aSecInfo.SSLStatus; equal(sslstatus.failedCertChain, null, "failedCertChain for a successful connection should be null"); ok(sslstatus.succeededCertChain.equals(build_cert_chain(["default-ee", "test-ca"])), @@ -33,8 +33,8 @@ function run_test() { // succeededCertChain should be null) add_connection_test( "expired.example.com", SEC_ERROR_EXPIRED_CERTIFICATE, null, - function withSecurityInfo(aSSLStatus) { - let sslstatus = aSSLStatus.QueryInterface(Ci.nsISSLStatusProvider).SSLStatus; + function withSecurityInfo(aSecInfo) { + let sslstatus = aSecInfo.SSLStatus; equal(sslstatus.succeededCertChain, null, "succeededCertChain for a failed connection should be null"); ok(sslstatus.failedCertChain.equals(build_cert_chain(["expired-ee", "test-ca"])), diff --git a/security/manager/tools/getHSTSPreloadList.js b/security/manager/tools/getHSTSPreloadList.js index 2a0a0efafb53..5e10239a1c81 100644 --- a/security/manager/tools/getHSTSPreloadList.js +++ b/security/manager/tools/getHSTSPreloadList.js @@ -111,8 +111,8 @@ function processStsHeader(host, header, status, securityInfo) { if (header != null && securityInfo != null) { try { let uri = Services.io.newURI("https://" + host.name); - let sslStatus = securityInfo.QueryInterface(Ci.nsISSLStatusProvider) - .SSLStatus; + let sslStatus = securityInfo. + QueryInterface(Ci.nsITransportSecurityInfo).SSLStatus; gSSService.processHeader(Ci.nsISiteSecurityService.HEADER_HSTS, uri, header, sslStatus, 0, Ci.nsISiteSecurityService.SOURCE_PRELOAD_LIST, diff --git a/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/security.py b/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/security.py index b94758f236e2..ad2abf346b7d 100644 --- a/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/security.py +++ b/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/security.py @@ -42,8 +42,7 @@ class Security(BaseLib): """ cert = self.marionette.execute_script(""" var securityUI = arguments[0].linkedBrowser.securityUI; - var status = securityUI.QueryInterface(Components.interfaces.nsISSLStatusProvider) - .SSLStatus; + var status = securityUI.secInfo.SSLStatus; return status ? status.serverCert : null; """, script_args=[tab_element]) diff --git a/toolkit/content/browser-child.js b/toolkit/content/browser-child.js index b29d7eb15faf..91e5d6c04a28 100644 --- a/toolkit/content/browser-child.js +++ b/toolkit/content/browser-child.js @@ -215,7 +215,7 @@ var WebProgressListener = { let objects = this._setupObjects(aWebProgress, aRequest); json.state = aState; - json.status = SecurityUI.getSSLStatusAsString(); + json.secInfo = SecurityUI.getSecInfoAsString(); json.matchedList = null; if (aRequest && aRequest instanceof Ci.nsIClassifiedChannel) { @@ -374,15 +374,17 @@ var WebNavigation = { WebNavigation.init(); var SecurityUI = { - getSSLStatusAsString() { - let status = docShell.securityUI.QueryInterface(Ci.nsISSLStatusProvider).SSLStatus; + getSecInfoAsString() { + let secInfo = docShell.securityUI.secInfo; - if (status) { - let helper = Cc["@mozilla.org/network/serialization-helper;1"] - .getService(Ci.nsISerializationHelper); + if (secInfo) { + if (secInfo) { + let helper = Cc["@mozilla.org/network/serialization-helper;1"] + .getService(Ci.nsISerializationHelper); - status.QueryInterface(Ci.nsISerializable); - return helper.serializeToString(status); + secInfo.QueryInterface(Ci.nsISerializable); + return helper.serializeToString(secInfo); + } } return null; diff --git a/toolkit/modules/CertUtils.jsm b/toolkit/modules/CertUtils.jsm index ab35d04291e5..252b319b43fc 100644 --- a/toolkit/modules/CertUtils.jsm +++ b/toolkit/modules/CertUtils.jsm @@ -143,7 +143,7 @@ function checkCert(aChannel, aAllowNonBuiltInCerts, aCerts) { return; } - let sslStatus = aChannel.securityInfo.QueryInterface(Ci.nsISSLStatusProvider) + let sslStatus = aChannel.securityInfo.QueryInterface(Ci.nsITransportSecurityInfo) .SSLStatus; let cert = sslStatus.serverCert; diff --git a/toolkit/modules/RemoteSecurityUI.jsm b/toolkit/modules/RemoteSecurityUI.jsm index 47fd63b63bcf..10ee842fcf61 100644 --- a/toolkit/modules/RemoteSecurityUI.jsm +++ b/toolkit/modules/RemoteSecurityUI.jsm @@ -8,22 +8,20 @@ var EXPORTED_SYMBOLS = ["RemoteSecurityUI"]; ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); function RemoteSecurityUI() { - this._SSLStatus = null; + this._secInfo = null; this._state = 0; } RemoteSecurityUI.prototype = { - QueryInterface: ChromeUtils.generateQI([Ci.nsISSLStatusProvider, Ci.nsISecureBrowserUI]), - - // nsISSLStatusProvider - get SSLStatus() { return this._SSLStatus; }, + QueryInterface: ChromeUtils.generateQI([Ci.nsISecureBrowserUI]), // nsISecureBrowserUI get state() { return this._state; }, get tooltipText() { return ""; }, + get secInfo() { return this._secInfo; }, - _update(aStatus, aState) { - this._SSLStatus = aStatus; + _update(aSecInfo, aState) { + this._secInfo = aSecInfo; this._state = aState; } }; diff --git a/toolkit/modules/RemoteWebProgress.jsm b/toolkit/modules/RemoteWebProgress.jsm index bcdb4979e266..c384e053e545 100644 --- a/toolkit/modules/RemoteWebProgress.jsm +++ b/toolkit/modules/RemoteWebProgress.jsm @@ -110,14 +110,14 @@ RemoteWebProgressManager.prototype = { this._progressListeners.filter(l => l.listener != aListener); }, - _fixSSLStatusAndState(aStatus, aState) { + _fixSecInfoAndState(aSecInfo, aState) { let deserialized = null; - if (aStatus) { + if (aSecInfo) { let helper = Cc["@mozilla.org/network/serialization-helper;1"] .getService(Ci.nsISerializationHelper); - deserialized = helper.deserializeObject(aStatus); - deserialized.QueryInterface(Ci.nsISSLStatus); + deserialized = helper.deserializeObject(aSecInfo); + deserialized.QueryInterface(Ci.nsITransportSecurityInfo); } return [deserialized, aState]; @@ -241,14 +241,14 @@ RemoteWebProgressManager.prototype = { break; case "Content:SecurityChange": - let [status, state] = this._fixSSLStatusAndState(json.status, json.state); + let [secInfo, state] = this._fixSecInfoAndState(json.secInfo, json.state); if (isTopLevel) { // Invoking this getter triggers the generation of the underlying object, // which we need to access with ._securityUI, because .securityUI returns // a wrapper that makes _update inaccessible. void this._browser.securityUI; - this._browser._securityUI._update(status, state); + this._browser._securityUI._update(secInfo, state); } this._callProgressListeners( diff --git a/toolkit/modules/addons/SecurityInfo.jsm b/toolkit/modules/addons/SecurityInfo.jsm index de0084398aa6..44cbe1c33492 100644 --- a/toolkit/modules/addons/SecurityInfo.jsm +++ b/toolkit/modules/addons/SecurityInfo.jsm @@ -94,7 +94,6 @@ const SecurityInfo = { } securityInfo.QueryInterface(Ci.nsITransportSecurityInfo); - securityInfo.QueryInterface(Ci.nsISSLStatusProvider); const SSLStatus = securityInfo.SSLStatus; if (NSSErrorsService.isNSSErrorCode(securityInfo.errorCode)) { diff --git a/toolkit/modules/tests/chrome/test_bug544442_checkCert.xul b/toolkit/modules/tests/chrome/test_bug544442_checkCert.xul index 213c18de3d7a..e4c96f22ed08 100644 --- a/toolkit/modules/tests/chrome/test_bug544442_checkCert.xul +++ b/toolkit/modules/tests/chrome/test_bug544442_checkCert.xul @@ -87,7 +87,7 @@ function testXHRLoad(aEvent) { "attributes array passed to checkCert has an element that has an " + "issuerName that is not the same as the certificate's"); - var cert = channel.securityInfo.QueryInterface(Ci.nsISSLStatusProvider). + var cert = channel.securityInfo.QueryInterface(Ci.nsITransportSecurityInfo). SSLStatus.QueryInterface(Ci.nsISSLStatus).serverCert; certs = [ { issuerName: cert.issuerName, diff --git a/toolkit/mozapps/update/nsUpdateService.js b/toolkit/mozapps/update/nsUpdateService.js index cc591421495e..110674f3a8fa 100644 --- a/toolkit/mozapps/update/nsUpdateService.js +++ b/toolkit/mozapps/update/nsUpdateService.js @@ -3116,7 +3116,7 @@ Checker.prototype = { // Set MitM pref. try { var sslStatus = request.channel.QueryInterface(Ci.nsIRequest) - .securityInfo.QueryInterface(Ci.nsISSLStatusProvider) + .securityInfo.QueryInterface(Ci.nsITransportSecurityInfo) .SSLStatus.QueryInterface(Ci.nsISSLStatus); if (sslStatus && sslStatus.serverCert && sslStatus.serverCert.issuerName) { Services.prefs.setStringPref("security.pki.mitm_canary_issuer", From 1acb33fb67a2de2602d2426ce013dd82ae207bb0 Mon Sep 17 00:00:00 2001 From: Brad Werth Date: Wed, 11 Jul 2018 16:52:33 -0700 Subject: [PATCH 18/26] Bug 1471894 Part 1: Ensure that absolute positioned blocks with auto margins update their UsedMargin property. r=dholbert MozReview-Commit-ID: EFIQVvazkTg --HG-- extra : rebase_source : 4f221c3df02fa8a9b0dd8b4b7b71af56b1fcbe26 --- layout/generic/ReflowInput.cpp | 38 ++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/layout/generic/ReflowInput.cpp b/layout/generic/ReflowInput.cpp index 0996b58f38f7..0d68d0c163eb 100644 --- a/layout/generic/ReflowInput.cpp +++ b/layout/generic/ReflowInput.cpp @@ -1237,16 +1237,20 @@ ReflowInput::CalculateBorderPaddingMargin( nscoord start, end; // We have to compute the start and end values if (eStyleUnit_Auto == mStyleMargin->mMargin.GetUnit(startSide)) { - // XXX FIXME (or does CalculateBlockSideMargins do this?) - start = 0; // just ignore + // We set this to 0 for now, and fix it up later in + // InitAbsoluteConstraints (which is caller of this function, via + // CalculateHypotheticalPosition). + start = 0; } else { start = nsLayoutUtils:: ComputeCBDependentValue(aContainingBlockSize, mStyleMargin->mMargin.Get(startSide)); } if (eStyleUnit_Auto == mStyleMargin->mMargin.GetUnit(endSide)) { - // XXX FIXME (or does CalculateBlockSideMargins do this?) - end = 0; // just ignore + // We set this to 0 for now, and fix it up later in + // InitAbsoluteConstraints (which is caller of this function, via + // CalculateHypotheticalPosition). + end = 0; } else { end = nsLayoutUtils:: ComputeCBDependentValue(aContainingBlockSize, @@ -1765,6 +1769,10 @@ ReflowInput::InitAbsoluteConstraints(nsPresContext* aPresContext, ComputedLogicalBorderPadding().ConvertTo(cbwm, wm); bool iSizeIsAuto = eStyleUnit_Auto == mStylePosition->ISize(cbwm).GetUnit(); + bool marginIStartIsAuto = false; + bool marginIEndIsAuto = false; + bool marginBStartIsAuto = false; + bool marginBEndIsAuto = false; if (iStartIsAuto) { // We know 'right' is not 'auto' anymore thanks to the hypothetical // box code above. @@ -1835,9 +1843,9 @@ ReflowInput::InitAbsoluteConstraints(nsPresContext* aPresContext, nscoord availMarginSpace = aCBSize.ISize(cbwm) - offsets.IStartEnd(cbwm) - margin.IStartEnd(cbwm) - borderPadding.IStartEnd(cbwm) - computedSize.ISize(cbwm); - bool marginIStartIsAuto = + marginIStartIsAuto = eStyleUnit_Auto == mStyleMargin->mMargin.GetIStartUnit(cbwm); - bool marginIEndIsAuto = + marginIEndIsAuto = eStyleUnit_Auto == mStyleMargin->mMargin.GetIEndUnit(cbwm); if (marginIStartIsAuto) { @@ -1923,9 +1931,9 @@ ReflowInput::InitAbsoluteConstraints(nsPresContext* aPresContext, // * we're dealing with a replaced element // * bsize was constrained by min- or max-bsize. nscoord availMarginSpace = autoBSize - computedSize.BSize(cbwm); - bool marginBStartIsAuto = + marginBStartIsAuto = eStyleUnit_Auto == mStyleMargin->mMargin.GetBStartUnit(cbwm); - bool marginBEndIsAuto = + marginBEndIsAuto = eStyleUnit_Auto == mStyleMargin->mMargin.GetBEndUnit(cbwm); if (marginBStartIsAuto) { @@ -1954,7 +1962,19 @@ ReflowInput::InitAbsoluteConstraints(nsPresContext* aPresContext, ComputedISize() = computedSize.ConvertTo(wm, cbwm).ISize(wm); SetComputedLogicalOffsets(offsets.ConvertTo(wm, cbwm)); - SetComputedLogicalMargin(margin.ConvertTo(wm, cbwm)); + + LogicalMargin marginInOurWM = margin.ConvertTo(wm, cbwm); + SetComputedLogicalMargin(marginInOurWM); + + // If we have auto margins, update our UsedMarginProperty. The property + // will have already been created by InitOffsets if it is needed. + if (marginIStartIsAuto || marginIEndIsAuto || + marginBStartIsAuto || marginBEndIsAuto) { + nsMargin* propValue = mFrame->GetProperty(nsIFrame::UsedMarginProperty()); + MOZ_ASSERT(propValue, "UsedMarginProperty should have been created " + "by InitOffsets."); + *propValue = marginInOurWM.GetPhysicalMargin(wm); + } } // This will not be converted to abstract coordinates because it's only From 51462b9733329259cf74f0ed7a15346fa2b8a7be Mon Sep 17 00:00:00 2001 From: Brad Werth Date: Thu, 12 Jul 2018 11:55:32 -0700 Subject: [PATCH 19/26] Bug 1471894 Part 2: Add a web-platform-test to check computed margin values on auto margin blocks. r=dholbert MozReview-Commit-ID: KbSZzhoX5mu --HG-- extra : rebase_source : 210535f627475e07c85f174b41396f650ac0d29f --- testing/web-platform/meta/MANIFEST.json | 44 ++++++++----- .../tests/css/cssom/computed-style-005.html | 65 +++++++++++++++++++ 2 files changed, 94 insertions(+), 15 deletions(-) create mode 100644 testing/web-platform/tests/css/cssom/computed-style-005.html diff --git a/testing/web-platform/meta/MANIFEST.json b/testing/web-platform/meta/MANIFEST.json index 4f9a23872a3a..38ae8b15a7cd 100644 --- a/testing/web-platform/meta/MANIFEST.json +++ b/testing/web-platform/meta/MANIFEST.json @@ -184511,6 +184511,18 @@ {} ] ], + "svg/linking/reftests/use-descendant-combinator-003.html": [ + [ + "/svg/linking/reftests/use-descendant-combinator-003.html", + [ + [ + "/svg/linking/reftests/use-descendant-combinator-ref.html", + "==" + ] + ], + {} + ] + ], "svg/painting/currentColor-override-pserver-fallback.svg": [ [ "/svg/painting/currentColor-override-pserver-fallback.svg", @@ -184547,18 +184559,6 @@ {} ] ], - "svg/linking/reftests/use-descendant-combinator-003.html": [ - [ - "/svg/linking/reftests/use-descendant-combinator-003.html", - [ - [ - "/svg/linking/reftests/use-descendant-combinator-ref.html", - "==" - ] - ], - {} - ] - ], "svg/painting/reftests/paint-context-001.svg": [ [ "/svg/painting/reftests/paint-context-001.svg", @@ -325276,6 +325276,12 @@ {} ] ], + "css/cssom/computed-style-005.html": [ + [ + "/css/cssom/computed-style-005.html", + {} + ] + ], "css/cssom/computed-style-set-property.html": [ [ "/css/cssom/computed-style-set-property.html", @@ -439008,7 +439014,7 @@ "testharness" ], "content-security-policy/securitypolicyviolation/img-src-redirect-upgrade-reporting.https.html": [ - "e338e94ea726419db64ed5b98c95b862c394409e", + "f6623c80b2b4be3fd9dd0f5dc0a6417652f1b797", "testharness" ], "content-security-policy/securitypolicyviolation/img-src-redirect-upgrade-reporting.https.html.headers": [ @@ -439060,7 +439066,7 @@ "testharness" ], "content-security-policy/securitypolicyviolation/support/inside-worker.sub.js": [ - "f425a7ae6c167bfe9857f08f460897e16bf6ca95", + "d94662579190653a3b3e9d076b79d7b0f01f2dc7", "support" ], "content-security-policy/securitypolicyviolation/support/inside-worker.sub.js.headers": [ @@ -552051,6 +552057,10 @@ "55010cf90dc7fc2ef8ec6cbd13d1ec947a823aed", "testharness" ], + "css/cssom/computed-style-005.html": [ + "29408580d9bed184f5807f8cb769eaafabf5b8dc", + "testharness" + ], "css/cssom/computed-style-set-property.html": [ "cb05ff525eb659d43bf234d932fd860795959c9e", "testharness" @@ -619083,6 +619093,10 @@ "3d51ca0fc007d52147e7ea03493cac7ee1bb7903", "reftest" ], + "svg/linking/reftests/use-descendant-combinator-003.html": [ + "d9155d3b92ecf0735f82ed9a0f2a8fd3fc380d55", + "reftest" + ], "svg/linking/reftests/use-descendant-combinator-ref.html": [ "fb8aec792684b97151d2964b85d1e70829e141ad", "support" @@ -622188,7 +622202,7 @@ "support" ], "webaudio/historical.html": [ - "93068df297042344669093ce899f0230c87ebf54", + "c6e3c7d6751731c708edfb0f4e32df8a6a3b80b0", "testharness" ], "webaudio/idlharness.https.html": [ diff --git a/testing/web-platform/tests/css/cssom/computed-style-005.html b/testing/web-platform/tests/css/cssom/computed-style-005.html new file mode 100644 index 000000000000..84164fe58b35 --- /dev/null +++ b/testing/web-platform/tests/css/cssom/computed-style-005.html @@ -0,0 +1,65 @@ + + + + CSS Test: getComputedStyle on blocks with auto margins + + + + + + + +
+ + + + + From a468f781198ad6b9a0471b86705db9cf86c878e2 Mon Sep 17 00:00:00 2001 From: Tanushree Podder Date: Sun, 22 Jul 2018 01:59:37 -0400 Subject: [PATCH 20/26] Bug 1473699 - Async-scroll the layout viewport even if it's smaller than the visual viewport. r=botond When the layout viewport is smaller than the visual viewport and the user scrolls a web page, the layout viewport remains fixed to its position before the scroll event took place. However, the visual viewport moves according to the new scroll position. This patch addresses this issue by moving the layout viewport along with the visual viewport. MozReview-Commit-ID: 3Mk1o6AF2wr --HG-- extra : rebase_source : 8c6068059593038dc443cb9c96242483de97e9d9 --- gfx/layers/FrameMetrics.cpp | 56 ++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/gfx/layers/FrameMetrics.cpp b/gfx/layers/FrameMetrics.cpp index 0b23a3cd2b1e..90c3adad5600 100644 --- a/gfx/layers/FrameMetrics.cpp +++ b/gfx/layers/FrameMetrics.cpp @@ -26,24 +26,48 @@ FrameMetrics::RecalculateViewportOffset() // Additionally, if the composition bounds changes (due to an orientation // change, window resize, etc.), it may take a few frames for mViewport to // update and during that time, the visual viewport may be larger than the - // layout viewport, so don't attempt to make any adjustments. - if (mViewport.Contains(visualViewport) || - (mViewport.Width() < visualViewport.Width() && - !FuzzyEqualsMultiplicative(mViewport.Width(), visualViewport.Width())) || - (mViewport.Height() < visualViewport.Height() && - !FuzzyEqualsMultiplicative(mViewport.Height(), visualViewport.Height()))) { + // layout viewport. In such situations, we take an early exit if the visual + // viewport contains the layout viewport. + if (mViewport.Contains(visualViewport) || visualViewport.Contains(mViewport)) { return; } - if (visualViewport.X() < mViewport.X()) { - mViewport.MoveToX(visualViewport.X()); - } else if (mViewport.XMost() < visualViewport.XMost()) { - mViewport.MoveByX(visualViewport.XMost() - mViewport.XMost()); - } - if (visualViewport.Y() < mViewport.Y()) { - mViewport.MoveToY(visualViewport.Y()); - } else if (mViewport.YMost() < visualViewport.YMost()) { - mViewport.MoveByY(visualViewport.YMost() - mViewport.YMost()); - } + + // If visual viewport size is greater than the layout viewport, move the layout + // viewport such that it remains inside the visual viewport. Otherwise, + // move the layout viewport such that the visual viewport is contained + // inside the layout viewport. + if ((mViewport.Width() < visualViewport.Width() && + !FuzzyEqualsMultiplicative(mViewport.Width(), visualViewport.Width())) || + (mViewport.Height() < visualViewport.Height() && + !FuzzyEqualsMultiplicative(mViewport.Height(), visualViewport.Height()))) { + + if (mViewport.X() < visualViewport.X()) { + // layout viewport moves right + mViewport.MoveToX(visualViewport.X()); + } else if (visualViewport.XMost() < mViewport.XMost()) { + // layout viewport moves left + mViewport.MoveByX(visualViewport.XMost() - mViewport.XMost()); + } + if (mViewport.Y() < visualViewport.Y()) { + // layout viewport moves down + mViewport.MoveToY(visualViewport.Y()); + } else if (visualViewport.YMost() < mViewport.YMost()) { + // layout viewport moves up + mViewport.MoveByY(visualViewport.YMost() - mViewport.YMost()); + } + } else { + + if (visualViewport.X() < mViewport.X()) { + mViewport.MoveToX(visualViewport.X()); + } else if (mViewport.XMost() < visualViewport.XMost()) { + mViewport.MoveByX(visualViewport.XMost() - mViewport.XMost()); + } + if (visualViewport.Y() < mViewport.Y()) { + mViewport.MoveToY(visualViewport.Y()); + } else if (mViewport.YMost() < visualViewport.YMost()) { + mViewport.MoveByY(visualViewport.YMost() - mViewport.YMost()); + } + } } void From 70ab23ecd2d87bc772161cc1c2af259d5d68985e Mon Sep 17 00:00:00 2001 From: shindli Date: Tue, 24 Jul 2018 02:55:53 +0300 Subject: [PATCH 21/26] Backed out changeset d126a6593e8f (bug 1475647) for mozmake.exe bustage on a CLOSED TREE --- browser/base/content/browser-siteIdentity.js | 8 +++-- browser/base/content/browser.js | 6 ++-- browser/base/content/pageinfo/security.js | 5 +++- devtools/shared/security/auth.js | 2 +- devtools/shared/security/socket.js | 2 +- devtools/shared/webconsole/network-helper.js | 1 + .../test/unit/test_security-info-parser.js | 3 +- .../test/unit/test_security-info-state.js | 3 +- .../unit/test_security-info-static-hpkp.js | 3 +- dom/ipc/TabParent.cpp | 10 ------- mobile/android/chrome/content/browser.js | 5 ++-- mobile/android/chrome/content/content.js | 3 +- .../modules/geckoview/GeckoViewProgress.jsm | 3 +- netwerk/base/nsISecureBrowserUI.idl | 2 -- netwerk/protocol/http/AlternateServices.cpp | 2 ++ netwerk/protocol/http/Http2Session.cpp | 2 ++ netwerk/protocol/http/nsHttpChannel.cpp | 17 ++++++----- netwerk/protocol/http/nsHttpNTLMAuth.cpp | 7 +++-- netwerk/socket/nsITransportSecurityInfo.idl | 3 -- .../pki/resources/content/exceptionDialog.js | 30 +++++++------------ .../manager/ssl/TransportSecurityInfo.cpp | 3 +- security/manager/ssl/TransportSecurityInfo.h | 3 ++ security/manager/ssl/moz.build | 1 + security/manager/ssl/nsISSLStatusProvider.idl | 13 ++++++++ .../manager/ssl/nsSecureBrowserUIImpl.cpp | 20 ++++++++----- security/manager/ssl/nsSecureBrowserUIImpl.h | 9 ++++-- security/manager/ssl/tests/unit/head_psm.js | 9 ++++-- .../unit/test_cert_overrides_read_only.js | 3 +- security/manager/ssl/tests/unit/test_ct.js | 3 +- .../ssl/tests/unit/test_session_resumption.js | 12 ++++++-- .../manager/ssl/tests/unit/test_ssl_status.js | 8 ++--- security/manager/tools/getHSTSPreloadList.js | 4 +-- .../firefox/firefox_puppeteer/api/security.py | 3 +- toolkit/content/browser-child.js | 18 +++++------ toolkit/modules/CertUtils.jsm | 2 +- toolkit/modules/RemoteSecurityUI.jsm | 12 ++++---- toolkit/modules/RemoteWebProgress.jsm | 12 ++++---- toolkit/modules/addons/SecurityInfo.jsm | 1 + .../tests/chrome/test_bug544442_checkCert.xul | 2 +- toolkit/mozapps/update/nsUpdateService.js | 2 +- 40 files changed, 147 insertions(+), 110 deletions(-) create mode 100644 security/manager/ssl/nsISSLStatusProvider.idl diff --git a/browser/base/content/browser-siteIdentity.js b/browser/base/content/browser-siteIdentity.js index ca529b5ac125..f21fb38f65b3 100644 --- a/browser/base/content/browser-siteIdentity.js +++ b/browser/base/content/browser-siteIdentity.js @@ -347,8 +347,12 @@ var gIdentityHandler = { // Firstly, populate the state properties required to display the UI. See // the documentation of the individual properties for details. this.setURI(uri); - this._sslStatus = gBrowser.securityUI.secInfo && - gBrowser.securityUI.secInfo.SSLStatus; + this._sslStatus = gBrowser.securityUI + .QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; + if (this._sslStatus) { + this._sslStatus.QueryInterface(Ci.nsISSLStatus); + } // Then, update the user interface with the available data. this.refreshIdentityBlock(); diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index cb7161319824..8153922b300f 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -2996,7 +2996,8 @@ var BrowserOnClick = { } securityInfo = getSecurityInfo(securityInfoAsString); - sslStatus = securityInfo.SSLStatus; + sslStatus = securityInfo.QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; let params = { exceptionAdded: false, sslStatus }; @@ -3037,7 +3038,8 @@ var BrowserOnClick = { } securityInfo = getSecurityInfo(securityInfoAsString); - sslStatus = securityInfo.SSLStatus; + sslStatus = securityInfo.QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; let errorInfo = getDetailedCertErrorInfo(location, securityInfo); let validityInfo = { diff --git a/browser/base/content/pageinfo/security.js b/browser/base/content/pageinfo/security.js index 2ab144818ecd..a57560e794e7 100644 --- a/browser/base/content/pageinfo/security.js +++ b/browser/base/content/pageinfo/security.js @@ -27,6 +27,7 @@ var security = { }, _getSecurityInfo() { + const nsISSLStatusProvider = Ci.nsISSLStatusProvider; const nsISSLStatus = Ci.nsISSLStatus; // We don't have separate info for a frame, return null until further notice @@ -49,9 +50,11 @@ var security = { (ui.state & Ci.nsIWebProgressListener.STATE_IS_INSECURE); var isEV = (ui.state & Ci.nsIWebProgressListener.STATE_IDENTITY_EV_TOPLEVEL); - var status = ui.secInfo && ui.secInfo.SSLStatus; + ui.QueryInterface(nsISSLStatusProvider); + var status = ui.SSLStatus; if (!isInsecure && status) { + status.QueryInterface(nsISSLStatus); var cert = status.serverCert; var issuerName = cert.issuerOrganization || cert.issuerName; diff --git a/devtools/shared/security/auth.js b/devtools/shared/security/auth.js index 86b304c59432..98c325186513 100644 --- a/devtools/shared/security/auth.js +++ b/devtools/shared/security/auth.js @@ -300,7 +300,7 @@ OOBCert.Client.prototype = { // Client verifies that Server's cert matches hash(ServerCert) from the // advertisement dumpv("Validate server cert hash"); - const serverCert = socket.securityInfo.QueryInterface(Ci.nsITransportSecurityInfo) + const serverCert = socket.securityInfo.QueryInterface(Ci.nsISSLStatusProvider) .SSLStatus.serverCert; const advertisedCert = cert; if (serverCert.sha256Fingerprint != advertisedCert.sha256) { diff --git a/devtools/shared/security/socket.js b/devtools/shared/security/socket.js index 98fe37ea38b0..55d70ac598fe 100644 --- a/devtools/shared/security/socket.js +++ b/devtools/shared/security/socket.js @@ -353,7 +353,7 @@ function _isInputAlive(input) { */ function _storeCertOverride(s, host, port) { // eslint-disable-next-line no-shadow - const cert = s.securityInfo.QueryInterface(Ci.nsITransportSecurityInfo) + const cert = s.securityInfo.QueryInterface(Ci.nsISSLStatusProvider) .SSLStatus.serverCert; const overrideBits = Ci.nsICertOverrideService.ERROR_UNTRUSTED | Ci.nsICertOverrideService.ERROR_MISMATCH; diff --git a/devtools/shared/webconsole/network-helper.js b/devtools/shared/webconsole/network-helper.js index 9c590335f075..663d70f43213 100644 --- a/devtools/shared/webconsole/network-helper.js +++ b/devtools/shared/webconsole/network-helper.js @@ -600,6 +600,7 @@ var NetworkHelper = { */ securityInfo.QueryInterface(Ci.nsITransportSecurityInfo); + securityInfo.QueryInterface(Ci.nsISSLStatusProvider); const wpl = Ci.nsIWebProgressListener; const NSSErrorsService = Cc["@mozilla.org/nss_errors_service;1"] diff --git a/devtools/shared/webconsole/test/unit/test_security-info-parser.js b/devtools/shared/webconsole/test/unit/test_security-info-parser.js index fcf05f7d8065..3edddcd97196 100644 --- a/devtools/shared/webconsole/test/unit/test_security-info-parser.js +++ b/devtools/shared/webconsole/test/unit/test_security-info-parser.js @@ -33,7 +33,8 @@ const MockCertificate = { }; const MockSecurityInfo = { - QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo]), + QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo, + Ci.nsISSLStatusProvider]), securityState: wpl.STATE_IS_SECURE, errorCode: 0, SSLStatus: { diff --git a/devtools/shared/webconsole/test/unit/test_security-info-state.js b/devtools/shared/webconsole/test/unit/test_security-info-state.js index 8e269c4fe1d5..3d458ef09a21 100644 --- a/devtools/shared/webconsole/test/unit/test_security-info-state.js +++ b/devtools/shared/webconsole/test/unit/test_security-info-state.js @@ -19,7 +19,8 @@ Object.defineProperty(this, "NetworkHelper", { const wpl = Ci.nsIWebProgressListener; const MockSecurityInfo = { - QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo]), + QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo, + Ci.nsISSLStatusProvider]), securityState: wpl.STATE_IS_BROKEN, errorCode: 0, SSLStatus: { diff --git a/devtools/shared/webconsole/test/unit/test_security-info-static-hpkp.js b/devtools/shared/webconsole/test/unit/test_security-info-static-hpkp.js index e97d43a9c6d9..e710d758f9af 100644 --- a/devtools/shared/webconsole/test/unit/test_security-info-static-hpkp.js +++ b/devtools/shared/webconsole/test/unit/test_security-info-static-hpkp.js @@ -20,7 +20,8 @@ Object.defineProperty(this, "NetworkHelper", { const wpl = Ci.nsIWebProgressListener; const MockSecurityInfo = { - QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo]), + QueryInterface: ChromeUtils.generateQI([Ci.nsITransportSecurityInfo, + Ci.nsISSLStatusProvider]), securityState: wpl.STATE_IS_SECURE, errorCode: 0, SSLStatus: { diff --git a/dom/ipc/TabParent.cpp b/dom/ipc/TabParent.cpp index 4d6385d05038..55eb508659a3 100644 --- a/dom/ipc/TabParent.cpp +++ b/dom/ipc/TabParent.cpp @@ -59,7 +59,6 @@ #include "nsIInterfaceRequestorUtils.h" #include "nsILoadInfo.h" #include "nsIPromptFactory.h" -#include "nsITransportSecurityInfo.h" #include "nsIURI.h" #include "nsIWindowWatcher.h" #include "nsIWebBrowserChrome.h" @@ -891,15 +890,6 @@ TabParent::GetState(uint32_t *aState) return NS_OK; } -NS_IMETHODIMP -TabParent::GetSecInfo(nsITransportSecurityInfo** _result) -{ - NS_ENSURE_ARG_POINTER(_result); - NS_WARNING("TransportSecurityInfo not valid here"); - *_result = nullptr; - return NS_OK; -} - NS_IMETHODIMP TabParent::SetDocShell(nsIDocShell *aDocShell) { diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index 02637d24f521..9edda1bb867e 100644 --- a/mobile/android/chrome/content/browser.js +++ b/mobile/android/chrome/content/browser.js @@ -5688,8 +5688,9 @@ var IdentityHandler = { * (if available). Return the data needed to update the UI. */ checkIdentity: function checkIdentity(aState, aBrowser) { - this._lastStatus = aBrowser.securityUI.secInfo && - aBrowser.securityUI.secInfo.SSLStatus; + this._lastStatus = aBrowser.securityUI + .QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; // Don't pass in the actual location object, since it can cause us to // hold on to the window object too long. Just pass in the fields we diff --git a/mobile/android/chrome/content/content.js b/mobile/android/chrome/content/content.js index dd5de8941d56..f7ea084b5e9a 100644 --- a/mobile/android/chrome/content/content.js +++ b/mobile/android/chrome/content/content.js @@ -360,7 +360,8 @@ var AboutCertErrorListener = { let securityInfo = docShell.failedChannel && docShell.failedChannel.securityInfo; securityInfo.QueryInterface(Ci.nsITransportSecurityInfo) .QueryInterface(Ci.nsISerializable); - let sslStatus = securityInfo.SSLStatus; + let sslStatus = securityInfo.QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; this._setTechDetails(sslStatus, securityInfo, ownerDoc.location.href); }, }; diff --git a/mobile/android/modules/geckoview/GeckoViewProgress.jsm b/mobile/android/modules/geckoview/GeckoViewProgress.jsm index 2ed06f85f3cd..c2f9db5facda 100644 --- a/mobile/android/modules/geckoview/GeckoViewProgress.jsm +++ b/mobile/android/modules/geckoview/GeckoViewProgress.jsm @@ -159,7 +159,8 @@ var IdentityHandler = { result.host = uri.host; } - let status = aBrowser.securityUI.secInfo.SSLStatus; + let status = aBrowser.securityUI.QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus.QueryInterface(Ci.nsISSLStatus); let cert = status.serverCert; result.organization = cert.organization; diff --git a/netwerk/base/nsISecureBrowserUI.idl b/netwerk/base/nsISecureBrowserUI.idl index 3f12d7bd3871..a2782d79b142 100644 --- a/netwerk/base/nsISecureBrowserUI.idl +++ b/netwerk/base/nsISecureBrowserUI.idl @@ -8,7 +8,6 @@ interface mozIDOMWindowProxy; interface nsIDocShell; -interface nsITransportSecurityInfo; [scriptable, uuid(718c662a-f810-4a80-a6c9-0b1810ecade2)] interface nsISecureBrowserUI : nsISupports @@ -17,7 +16,6 @@ interface nsISecureBrowserUI : nsISupports void setDocShell(in nsIDocShell docShell); readonly attribute unsigned long state; - readonly attribute nsITransportSecurityInfo secInfo; }; %{C++ diff --git a/netwerk/protocol/http/AlternateServices.cpp b/netwerk/protocol/http/AlternateServices.cpp index 90e714efa3fa..4600d138863a 100644 --- a/netwerk/protocol/http/AlternateServices.cpp +++ b/netwerk/protocol/http/AlternateServices.cpp @@ -15,6 +15,8 @@ #include "nsThreadUtils.h" #include "nsHttpTransaction.h" #include "NullHttpTransaction.h" +#include "nsISSLStatusProvider.h" +#include "nsISSLStatus.h" #include "nsISSLSocketControl.h" #include "nsIWellKnownOpportunisticUtils.h" diff --git a/netwerk/protocol/http/Http2Session.cpp b/netwerk/protocol/http/Http2Session.cpp index 2ad359c11a9a..e3417e0ffcba 100644 --- a/netwerk/protocol/http/Http2Session.cpp +++ b/netwerk/protocol/http/Http2Session.cpp @@ -27,6 +27,8 @@ #include "nsHttpConnection.h" #include "nsIRequestContext.h" #include "nsISSLSocketControl.h" +#include "nsISSLStatus.h" +#include "nsISSLStatusProvider.h" #include "nsISupportsPriority.h" #include "nsStandardURL.h" #include "nsURLHelper.h" diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 65ca1095f894..532e9334ccfe 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -69,6 +69,7 @@ #include "nsIScriptError.h" #include "nsIScriptSecurityManager.h" #include "nsISSLStatus.h" +#include "nsISSLStatusProvider.h" #include "nsITransportSecurityInfo.h" #include "nsIWebProgressListener.h" #include "LoadContextInfo.h" @@ -1856,11 +1857,11 @@ nsHttpChannel::ProcessSecurityHeaders() uint32_t flags = NS_UsePrivateBrowsing(this) ? nsISocketProvider::NO_PERMANENT_STORAGE : 0; - // Get the TransportSecurityInfo - nsCOMPtr transSecInfo = do_QueryInterface(mSecurityInfo); - NS_ENSURE_TRUE(transSecInfo, NS_ERROR_FAILURE); + // Get the SSLStatus + nsCOMPtr sslprov = do_QueryInterface(mSecurityInfo); + NS_ENSURE_TRUE(sslprov, NS_ERROR_FAILURE); nsCOMPtr sslStatus; - rv = transSecInfo->GetSSLStatus(getter_AddRefs(sslStatus)); + rv = sslprov->GetSSLStatus(getter_AddRefs(sslStatus)); NS_ENSURE_SUCCESS(rv, rv); NS_ENSURE_TRUE(sslStatus, NS_ERROR_FAILURE); @@ -1991,15 +1992,17 @@ nsHttpChannel::ProcessSSLInformation() !IsHTTPS() || mPrivateBrowsing) return; - nsCOMPtr securityInfo = + nsCOMPtr statusProvider = do_QueryInterface(mSecurityInfo); - if (!securityInfo) + if (!statusProvider) return; nsCOMPtr sslstat; - securityInfo->GetSSLStatus(getter_AddRefs(sslstat)); + statusProvider->GetSSLStatus(getter_AddRefs(sslstat)); if (!sslstat) return; + nsCOMPtr securityInfo = + do_QueryInterface(mSecurityInfo); uint32_t state; if (securityInfo && NS_SUCCEEDED(securityInfo->GetSecurityState(&state)) && diff --git a/netwerk/protocol/http/nsHttpNTLMAuth.cpp b/netwerk/protocol/http/nsHttpNTLMAuth.cpp index f59ba704d95c..718197060c09 100644 --- a/netwerk/protocol/http/nsHttpNTLMAuth.cpp +++ b/netwerk/protocol/http/nsHttpNTLMAuth.cpp @@ -24,6 +24,7 @@ #include "nsIChannel.h" #include "nsIX509Cert.h" #include "nsISSLStatus.h" +#include "nsISSLStatusProvider.h" #endif #include "mozilla/Attributes.h" #include "mozilla/Base64.h" @@ -334,12 +335,12 @@ nsHttpNTLMAuth::GenerateCredentials(nsIHttpAuthenticableChannel *authChannel, if (NS_FAILED(rv)) return rv; - nsCOMPtr secInfo = + nsCOMPtr statusProvider = do_QueryInterface(security); - if (mUseNative && secInfo) { + if (mUseNative && statusProvider) { nsCOMPtr status; - rv = secInfo->GetSSLStatus(getter_AddRefs(status)); + rv = statusProvider->GetSSLStatus(getter_AddRefs(status)); if (NS_FAILED(rv)) return rv; diff --git a/netwerk/socket/nsITransportSecurityInfo.idl b/netwerk/socket/nsITransportSecurityInfo.idl index 9ed3d2fdd4c6..a0a165038cc9 100644 --- a/netwerk/socket/nsITransportSecurityInfo.idl +++ b/netwerk/socket/nsITransportSecurityInfo.idl @@ -6,7 +6,6 @@ #include "nsISupports.idl" -interface nsISSLStatus; interface nsIX509CertList; [builtinclass, scriptable, uuid(216112d3-28bc-4671-b057-f98cc09ba1ea)] @@ -22,7 +21,5 @@ interface nsITransportSecurityInfo : nsISupports { * If verification succeeded, this will be null. */ readonly attribute nsIX509CertList failedCertChain; - - readonly attribute nsISSLStatus SSLStatus; }; diff --git a/security/manager/pki/resources/content/exceptionDialog.js b/security/manager/pki/resources/content/exceptionDialog.js index 3629028c5e4c..72c7d455e958 100644 --- a/security/manager/pki/resources/content/exceptionDialog.js +++ b/security/manager/pki/resources/content/exceptionDialog.js @@ -26,9 +26,7 @@ function initExceptionDialog() { gNsISecTel = Ci.nsISecurityUITelemetry; var brandName = gBundleBrand.getString("brandShortName"); - setText("warningText", - gPKIBundle.getFormattedString("addExceptionBrandedWarning2", - [brandName])); + setText("warningText", gPKIBundle.getFormattedString("addExceptionBrandedWarning2", [brandName])); gDialog.getButton("extra1").disabled = true; var args = window.arguments; @@ -77,7 +75,7 @@ function initExceptionDialog() { function grabCert(req, evt) { if (req.channel && req.channel.securityInfo) { gSSLStatus = req.channel.securityInfo - .QueryInterface(Ci.nsITransportSecurityInfo).SSLStatus; + .QueryInterface(Ci.nsISSLStatusProvider).SSLStatus; gCert = gSSLStatus ? gSSLStatus.QueryInterface(Ci.nsISSLStatus).serverCert : null; } @@ -161,8 +159,7 @@ function resetDialog() { */ function handleTextChange() { var checkCertButton = document.getElementById("checkCertButton"); - checkCertButton.disabled = - !(document.getElementById("locationTextBox").value); + checkCertButton.disabled = !(document.getElementById("locationTextBox").value); if (gNeedReset) { gNeedReset = false; resetDialog(); @@ -204,8 +201,7 @@ function updateCertStatus() { } } if (gSSLStatus.isUntrusted) { - bucketId += - gNsISecTel.WARNING_BAD_CERT_TOP_ADD_EXCEPTION_FLAG_UNTRUSTED; + bucketId += gNsISecTel.WARNING_BAD_CERT_TOP_ADD_EXCEPTION_FLAG_UNTRUSTED; if (!use1) { use1 = true; shortDesc = uts; @@ -233,8 +229,7 @@ function updateCertStatus() { pe.disabled = inPrivateBrowsing; pe.checked = !inPrivateBrowsing; - setText("headerDescription", - gPKIBundle.getString("addExceptionInvalidHeader")); + setText("headerDescription", gPKIBundle.getString("addExceptionInvalidHeader")); } else { shortDesc = "addExceptionValidShort"; longDesc = "addExceptionValidLong"; @@ -306,27 +301,22 @@ function addException() { var overrideService = Cc["@mozilla.org/security/certoverride;1"] .getService(Ci.nsICertOverrideService); var flags = 0; - let confirmBucketId = - gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_BASE; + let confirmBucketId = gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_BASE; if (gSSLStatus.isUntrusted) { flags |= overrideService.ERROR_UNTRUSTED; - confirmBucketId += - gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_UNTRUSTED; + confirmBucketId += gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_UNTRUSTED; } if (gSSLStatus.isDomainMismatch) { flags |= overrideService.ERROR_MISMATCH; - confirmBucketId += - gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_DOMAIN; + confirmBucketId += gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_DOMAIN; } if (gSSLStatus.isNotValidAtThisTime) { flags |= overrideService.ERROR_TIME; - confirmBucketId += - gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_TIME; + confirmBucketId += gNsISecTel.WARNING_BAD_CERT_TOP_CONFIRM_ADD_EXCEPTION_FLAG_TIME; } var permanentCheckbox = document.getElementById("permanent"); - var shouldStorePermanently = permanentCheckbox.checked && - !inPrivateBrowsingMode(); + var shouldStorePermanently = permanentCheckbox.checked && !inPrivateBrowsingMode(); if (!permanentCheckbox.checked) { gSecHistogram.add(gNsISecTel.WARNING_BAD_CERT_TOP_DONT_REMEMBER_EXCEPTION); } diff --git a/security/manager/ssl/TransportSecurityInfo.cpp b/security/manager/ssl/TransportSecurityInfo.cpp index 287ba44e6c92..b39949b6c487 100644 --- a/security/manager/ssl/TransportSecurityInfo.cpp +++ b/security/manager/ssl/TransportSecurityInfo.cpp @@ -51,6 +51,7 @@ TransportSecurityInfo::TransportSecurityInfo() NS_IMPL_ISUPPORTS(TransportSecurityInfo, nsITransportSecurityInfo, nsIInterfaceRequestor, + nsISSLStatusProvider, nsIAssociatedContentSecurity, nsISerializable, nsIClassInfo) @@ -364,7 +365,7 @@ TransportSecurityInfo::GetClassIDNoAlloc(nsCID *aClassIDNoAlloc) return NS_OK; } -NS_IMETHODIMP +nsresult TransportSecurityInfo::GetSSLStatus(nsISSLStatus** _result) { NS_ENSURE_ARG_POINTER(_result); diff --git a/security/manager/ssl/TransportSecurityInfo.h b/security/manager/ssl/TransportSecurityInfo.h index 4361d8249d87..86974e641ac5 100644 --- a/security/manager/ssl/TransportSecurityInfo.h +++ b/security/manager/ssl/TransportSecurityInfo.h @@ -16,6 +16,7 @@ #include "nsDataHashtable.h" #include "nsIAssociatedContentSecurity.h" #include "nsIInterfaceRequestor.h" +#include "nsISSLStatusProvider.h" #include "nsITransportSecurityInfo.h" #include "nsSSLStatus.h" #include "nsString.h" @@ -25,6 +26,7 @@ namespace mozilla { namespace psm { class TransportSecurityInfo : public nsITransportSecurityInfo , public nsIInterfaceRequestor + , public nsISSLStatusProvider , public nsIAssociatedContentSecurity , public nsISerializable , public nsIClassInfo @@ -37,6 +39,7 @@ public: NS_DECL_THREADSAFE_ISUPPORTS NS_DECL_NSITRANSPORTSECURITYINFO NS_DECL_NSIINTERFACEREQUESTOR + NS_DECL_NSISSLSTATUSPROVIDER NS_DECL_NSIASSOCIATEDCONTENTSECURITY NS_DECL_NSISERIALIZABLE NS_DECL_NSICLASSINFO diff --git a/security/manager/ssl/moz.build b/security/manager/ssl/moz.build index d8d7512967c6..d4092d2773cc 100644 --- a/security/manager/ssl/moz.build +++ b/security/manager/ssl/moz.build @@ -36,6 +36,7 @@ XPIDL_SOURCES += [ 'nsISecurityUITelemetry.idl', 'nsISiteSecurityService.idl', 'nsISSLStatus.idl', + 'nsISSLStatusProvider.idl', 'nsITokenDialogs.idl', 'nsITokenPasswordDialogs.idl', 'nsIX509Cert.idl', diff --git a/security/manager/ssl/nsISSLStatusProvider.idl b/security/manager/ssl/nsISSLStatusProvider.idl new file mode 100644 index 000000000000..83048f179436 --- /dev/null +++ b/security/manager/ssl/nsISSLStatusProvider.idl @@ -0,0 +1,13 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* 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 "nsISupports.idl" + +interface nsISSLStatus; + +[scriptable, uuid(179b1ab1-0950-4427-9556-6f496dc4a27f)] +interface nsISSLStatusProvider : nsISupports { + readonly attribute nsISSLStatus SSLStatus; +}; diff --git a/security/manager/ssl/nsSecureBrowserUIImpl.cpp b/security/manager/ssl/nsSecureBrowserUIImpl.cpp index 48cf86723d15..a2f24df7c4af 100644 --- a/security/manager/ssl/nsSecureBrowserUIImpl.cpp +++ b/security/manager/ssl/nsSecureBrowserUIImpl.cpp @@ -89,7 +89,8 @@ nsSecureBrowserUIImpl::nsSecureBrowserUIImpl() NS_IMPL_ISUPPORTS(nsSecureBrowserUIImpl, nsISecureBrowserUI, nsIWebProgressListener, - nsISupportsWeakReference) + nsISupportsWeakReference, + nsISSLStatusProvider) NS_IMETHODIMP nsSecureBrowserUIImpl::Init(mozIDOMWindowProxy* aWindow) @@ -373,21 +374,23 @@ nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState(nsIRequest* aRequest, ("SecureUI:%p: OnStateChange: remember mNewToplevelSecurityState => %x\n", this, mNewToplevelSecurityState)); - nsCOMPtr psmInfo(do_QueryInterface(info)); - if (psmInfo) { + nsCOMPtr sp(do_QueryInterface(info)); + if (sp) { // Ignore result updateStatus = true; - (void) psmInfo->GetSSLStatus(getter_AddRefs(temp_SSLStatus)); + (void) sp->GetSSLStatus(getter_AddRefs(temp_SSLStatus)); if (temp_SSLStatus) { bool aTemp; if (NS_SUCCEEDED(temp_SSLStatus->GetIsExtendedValidation(&aTemp))) { mNewToplevelIsEV = aTemp; } } - mSecInfo = psmInfo; } mNewToplevelSecurityStateKnown = true; + if (updateStatus) { + mSSLStatus = temp_SSLStatus; + } MOZ_LOG(gSecureDocLog, LogLevel::Debug, ("SecureUI:%p: remember securityInfo %p\n", this, info)); @@ -1011,7 +1014,7 @@ nsSecureBrowserUIImpl::UpdateSecurityState(nsIRequest* aRequest, // If we have no security, we also shouldn't have any SSL status. if (newSecurityState == lis_no_security) { - mSecInfo = nullptr; + mSSLStatus = nullptr; } } @@ -1163,8 +1166,9 @@ nsSecureBrowserUIImpl::OnSecurityChange(nsIWebProgress* aWebProgress, return NS_OK; } +// nsISSLStatusProvider methods NS_IMETHODIMP -nsSecureBrowserUIImpl::GetSecInfo(nsITransportSecurityInfo** _result) +nsSecureBrowserUIImpl::GetSSLStatus(nsISSLStatus** _result) { NS_ENSURE_ARG_POINTER(_result); MOZ_ASSERT(NS_IsMainThread()); @@ -1183,7 +1187,7 @@ nsSecureBrowserUIImpl::GetSecInfo(nsITransportSecurityInfo** _result) return NS_OK; } - *_result = mSecInfo; + *_result = mSSLStatus; NS_IF_ADDREF(*_result); return NS_OK; diff --git a/security/manager/ssl/nsSecureBrowserUIImpl.h b/security/manager/ssl/nsSecureBrowserUIImpl.h index d8c3b0f9a7cf..0c8fae120e6d 100644 --- a/security/manager/ssl/nsSecureBrowserUIImpl.h +++ b/security/manager/ssl/nsSecureBrowserUIImpl.h @@ -10,13 +10,14 @@ #include "mozilla/ReentrancyGuard.h" #include "nsCOMPtr.h" #include "nsINetUtil.h" +#include "nsISSLStatusProvider.h" #include "nsISecureBrowserUI.h" #include "nsISecurityEventSink.h" #include "nsIURI.h" #include "nsIWebProgressListener.h" #include "nsWeakReference.h" -class nsITransportSecurityInfo; +class nsISSLStatus; class nsIChannel; #define NS_SECURE_BROWSER_UI_CID \ @@ -25,7 +26,8 @@ class nsIChannel; class nsSecureBrowserUIImpl : public nsISecureBrowserUI, public nsIWebProgressListener, - public nsSupportsWeakReference + public nsSupportsWeakReference, + public nsISSLStatusProvider { friend class mozilla::ReentrancyGuard; @@ -35,6 +37,7 @@ public: NS_DECL_ISUPPORTS NS_DECL_NSIWEBPROGRESSLISTENER NS_DECL_NSISECUREBROWSERUI + NS_DECL_NSISSLSTATUSPROVIDER protected: virtual ~nsSecureBrowserUIImpl() {}; @@ -84,7 +87,7 @@ protected: void ObtainEventSink(nsIChannel *channel, nsCOMPtr &sink); - nsCOMPtr mSecInfo; + nsCOMPtr mSSLStatus; nsCOMPtr mCurrentToplevelSecurityInfo; PLDHashTable mTransferringRequests; diff --git a/security/manager/ssl/tests/unit/head_psm.js b/security/manager/ssl/tests/unit/head_psm.js index 5429475586d1..5a292a36c42d 100644 --- a/security/manager/ssl/tests/unit/head_psm.js +++ b/security/manager/ssl/tests/unit/head_psm.js @@ -720,7 +720,8 @@ FakeSSLStatus.prototype = { // Helper function for add_cert_override_test. Probably doesn't need to be // called directly. function add_cert_override(aHost, aExpectedBits, aSecurityInfo) { - let sslstatus = aSecurityInfo.SSLStatus; + let sslstatus = aSecurityInfo.QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; let bits = (sslstatus.isUntrusted ? Ci.nsICertOverrideService.ERROR_UNTRUSTED : 0) | (sslstatus.isDomainMismatch ? Ci.nsICertOverrideService.ERROR_MISMATCH : 0) | @@ -748,7 +749,8 @@ function add_cert_override_test(aHost, aExpectedBits, aExpectedError, Ci.nsIWebProgressListener.STATE_CERT_USER_OVERRIDDEN, "Cert override flag should be set on the security state"); if (aExpectedSSLStatus) { - let sslstatus = aSecurityInfo.SSLStatus; + let sslstatus = aSecurityInfo.QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; if (aExpectedSSLStatus.failedCertChain) { ok(aExpectedSSLStatus.failedCertChain.equals(sslstatus.failedCertChain)); } @@ -761,7 +763,8 @@ function add_cert_override_test(aHost, aExpectedBits, aExpectedError, // SSLStatus set on it. In this case, the error was not overridable anyway, so // we consider it a success. function attempt_adding_cert_override(aHost, aExpectedBits, aSecurityInfo) { - let sslstatus = aSecurityInfo.SSLStatus; + let sslstatus = aSecurityInfo.QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; if (sslstatus) { let bits = (sslstatus.isUntrusted ? Ci.nsICertOverrideService.ERROR_UNTRUSTED : 0) | diff --git a/security/manager/ssl/tests/unit/test_cert_overrides_read_only.js b/security/manager/ssl/tests/unit/test_cert_overrides_read_only.js index 9910827cfba8..3bdc0c1077be 100644 --- a/security/manager/ssl/tests/unit/test_cert_overrides_read_only.js +++ b/security/manager/ssl/tests/unit/test_cert_overrides_read_only.js @@ -10,7 +10,8 @@ // Helper function for add_read_only_cert_override_test. Probably doesn't need // to be called directly. function add_read_only_cert_override(aHost, aExpectedBits, aSecurityInfo) { - let sslstatus = aSecurityInfo.SSLStatus; + let sslstatus = aSecurityInfo.QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; let bits = (sslstatus.isUntrusted ? Ci.nsICertOverrideService.ERROR_UNTRUSTED : 0) | (sslstatus.isDomainMismatch ? Ci.nsICertOverrideService.ERROR_MISMATCH : 0) | diff --git a/security/manager/ssl/tests/unit/test_ct.js b/security/manager/ssl/tests/unit/test_ct.js index 57dd2c032ed5..08eb74298bab 100644 --- a/security/manager/ssl/tests/unit/test_ct.js +++ b/security/manager/ssl/tests/unit/test_ct.js @@ -11,7 +11,8 @@ const certdb = Cc["@mozilla.org/security/x509certdb;1"] function expectCT(value) { return (securityInfo) => { - let sslStatus = securityInfo.SSLStatus; + let sslStatus = securityInfo.QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; Assert.equal(sslStatus.certificateTransparencyStatus, value, "actual and expected CT status should match"); }; diff --git a/security/manager/ssl/tests/unit/test_session_resumption.js b/security/manager/ssl/tests/unit/test_session_resumption.js index 4923e14f1b68..58303b3e9a20 100644 --- a/security/manager/ssl/tests/unit/test_session_resumption.js +++ b/security/manager/ssl/tests/unit/test_session_resumption.js @@ -41,7 +41,9 @@ function add_resume_non_ev_with_override_test() { ok(transportSecurityInfo.securityState & Ci.nsIWebProgressListener.STATE_CERT_USER_OVERRIDDEN, "expired.example.com should have STATE_CERT_USER_OVERRIDDEN flag"); - let sslStatus = transportSecurityInfo.SSLStatus; + let sslStatus = transportSecurityInfo + .QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; ok(!sslStatus.succeededCertChain, "ev-test.example.com should not have succeededCertChain set"); ok(!sslStatus.isDomainMismatch, @@ -66,7 +68,9 @@ function add_one_ev_test() { ok(!(transportSecurityInfo.securityState & Ci.nsIWebProgressListener.STATE_CERT_USER_OVERRIDDEN), "ev-test.example.com should not have STATE_CERT_USER_OVERRIDDEN flag"); - let sslStatus = transportSecurityInfo.SSLStatus; + let sslStatus = transportSecurityInfo + .QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; ok(sslStatus.succeededCertChain, "ev-test.example.com should have succeededCertChain set"); ok(!sslStatus.isDomainMismatch, @@ -126,7 +130,9 @@ function add_one_non_ev_test() { ok(!(transportSecurityInfo.securityState & Ci.nsIWebProgressListener.STATE_CERT_USER_OVERRIDDEN), `${GOOD_DOMAIN} should not have STATE_CERT_USER_OVERRIDDEN flag`); - let sslStatus = transportSecurityInfo.SSLStatus; + let sslStatus = transportSecurityInfo + .QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; ok(sslStatus.succeededCertChain, `${GOOD_DOMAIN} should have succeededCertChain set`); ok(!sslStatus.isDomainMismatch, diff --git a/security/manager/ssl/tests/unit/test_ssl_status.js b/security/manager/ssl/tests/unit/test_ssl_status.js index e652e78ca421..2352e584eedb 100644 --- a/security/manager/ssl/tests/unit/test_ssl_status.js +++ b/security/manager/ssl/tests/unit/test_ssl_status.js @@ -20,8 +20,8 @@ function run_test() { // succeededCertChain should be set as expected) add_connection_test( "good.include-subdomains.pinning.example.com", PRErrorCodeSuccess, null, - function withSecurityInfo(aSecInfo) { - let sslstatus = aSecInfo.SSLStatus; + function withSecurityInfo(aSSLStatus) { + let sslstatus = aSSLStatus.QueryInterface(Ci.nsISSLStatusProvider).SSLStatus; equal(sslstatus.failedCertChain, null, "failedCertChain for a successful connection should be null"); ok(sslstatus.succeededCertChain.equals(build_cert_chain(["default-ee", "test-ca"])), @@ -33,8 +33,8 @@ function run_test() { // succeededCertChain should be null) add_connection_test( "expired.example.com", SEC_ERROR_EXPIRED_CERTIFICATE, null, - function withSecurityInfo(aSecInfo) { - let sslstatus = aSecInfo.SSLStatus; + function withSecurityInfo(aSSLStatus) { + let sslstatus = aSSLStatus.QueryInterface(Ci.nsISSLStatusProvider).SSLStatus; equal(sslstatus.succeededCertChain, null, "succeededCertChain for a failed connection should be null"); ok(sslstatus.failedCertChain.equals(build_cert_chain(["expired-ee", "test-ca"])), diff --git a/security/manager/tools/getHSTSPreloadList.js b/security/manager/tools/getHSTSPreloadList.js index 5e10239a1c81..2a0a0efafb53 100644 --- a/security/manager/tools/getHSTSPreloadList.js +++ b/security/manager/tools/getHSTSPreloadList.js @@ -111,8 +111,8 @@ function processStsHeader(host, header, status, securityInfo) { if (header != null && securityInfo != null) { try { let uri = Services.io.newURI("https://" + host.name); - let sslStatus = securityInfo. - QueryInterface(Ci.nsITransportSecurityInfo).SSLStatus; + let sslStatus = securityInfo.QueryInterface(Ci.nsISSLStatusProvider) + .SSLStatus; gSSService.processHeader(Ci.nsISiteSecurityService.HEADER_HSTS, uri, header, sslStatus, 0, Ci.nsISiteSecurityService.SOURCE_PRELOAD_LIST, diff --git a/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/security.py b/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/security.py index ad2abf346b7d..b94758f236e2 100644 --- a/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/security.py +++ b/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/security.py @@ -42,7 +42,8 @@ class Security(BaseLib): """ cert = self.marionette.execute_script(""" var securityUI = arguments[0].linkedBrowser.securityUI; - var status = securityUI.secInfo.SSLStatus; + var status = securityUI.QueryInterface(Components.interfaces.nsISSLStatusProvider) + .SSLStatus; return status ? status.serverCert : null; """, script_args=[tab_element]) diff --git a/toolkit/content/browser-child.js b/toolkit/content/browser-child.js index 91e5d6c04a28..b29d7eb15faf 100644 --- a/toolkit/content/browser-child.js +++ b/toolkit/content/browser-child.js @@ -215,7 +215,7 @@ var WebProgressListener = { let objects = this._setupObjects(aWebProgress, aRequest); json.state = aState; - json.secInfo = SecurityUI.getSecInfoAsString(); + json.status = SecurityUI.getSSLStatusAsString(); json.matchedList = null; if (aRequest && aRequest instanceof Ci.nsIClassifiedChannel) { @@ -374,17 +374,15 @@ var WebNavigation = { WebNavigation.init(); var SecurityUI = { - getSecInfoAsString() { - let secInfo = docShell.securityUI.secInfo; + getSSLStatusAsString() { + let status = docShell.securityUI.QueryInterface(Ci.nsISSLStatusProvider).SSLStatus; - if (secInfo) { - if (secInfo) { - let helper = Cc["@mozilla.org/network/serialization-helper;1"] - .getService(Ci.nsISerializationHelper); + if (status) { + let helper = Cc["@mozilla.org/network/serialization-helper;1"] + .getService(Ci.nsISerializationHelper); - secInfo.QueryInterface(Ci.nsISerializable); - return helper.serializeToString(secInfo); - } + status.QueryInterface(Ci.nsISerializable); + return helper.serializeToString(status); } return null; diff --git a/toolkit/modules/CertUtils.jsm b/toolkit/modules/CertUtils.jsm index 252b319b43fc..ab35d04291e5 100644 --- a/toolkit/modules/CertUtils.jsm +++ b/toolkit/modules/CertUtils.jsm @@ -143,7 +143,7 @@ function checkCert(aChannel, aAllowNonBuiltInCerts, aCerts) { return; } - let sslStatus = aChannel.securityInfo.QueryInterface(Ci.nsITransportSecurityInfo) + let sslStatus = aChannel.securityInfo.QueryInterface(Ci.nsISSLStatusProvider) .SSLStatus; let cert = sslStatus.serverCert; diff --git a/toolkit/modules/RemoteSecurityUI.jsm b/toolkit/modules/RemoteSecurityUI.jsm index 10ee842fcf61..47fd63b63bcf 100644 --- a/toolkit/modules/RemoteSecurityUI.jsm +++ b/toolkit/modules/RemoteSecurityUI.jsm @@ -8,20 +8,22 @@ var EXPORTED_SYMBOLS = ["RemoteSecurityUI"]; ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); function RemoteSecurityUI() { - this._secInfo = null; + this._SSLStatus = null; this._state = 0; } RemoteSecurityUI.prototype = { - QueryInterface: ChromeUtils.generateQI([Ci.nsISecureBrowserUI]), + QueryInterface: ChromeUtils.generateQI([Ci.nsISSLStatusProvider, Ci.nsISecureBrowserUI]), + + // nsISSLStatusProvider + get SSLStatus() { return this._SSLStatus; }, // nsISecureBrowserUI get state() { return this._state; }, get tooltipText() { return ""; }, - get secInfo() { return this._secInfo; }, - _update(aSecInfo, aState) { - this._secInfo = aSecInfo; + _update(aStatus, aState) { + this._SSLStatus = aStatus; this._state = aState; } }; diff --git a/toolkit/modules/RemoteWebProgress.jsm b/toolkit/modules/RemoteWebProgress.jsm index c384e053e545..bcdb4979e266 100644 --- a/toolkit/modules/RemoteWebProgress.jsm +++ b/toolkit/modules/RemoteWebProgress.jsm @@ -110,14 +110,14 @@ RemoteWebProgressManager.prototype = { this._progressListeners.filter(l => l.listener != aListener); }, - _fixSecInfoAndState(aSecInfo, aState) { + _fixSSLStatusAndState(aStatus, aState) { let deserialized = null; - if (aSecInfo) { + if (aStatus) { let helper = Cc["@mozilla.org/network/serialization-helper;1"] .getService(Ci.nsISerializationHelper); - deserialized = helper.deserializeObject(aSecInfo); - deserialized.QueryInterface(Ci.nsITransportSecurityInfo); + deserialized = helper.deserializeObject(aStatus); + deserialized.QueryInterface(Ci.nsISSLStatus); } return [deserialized, aState]; @@ -241,14 +241,14 @@ RemoteWebProgressManager.prototype = { break; case "Content:SecurityChange": - let [secInfo, state] = this._fixSecInfoAndState(json.secInfo, json.state); + let [status, state] = this._fixSSLStatusAndState(json.status, json.state); if (isTopLevel) { // Invoking this getter triggers the generation of the underlying object, // which we need to access with ._securityUI, because .securityUI returns // a wrapper that makes _update inaccessible. void this._browser.securityUI; - this._browser._securityUI._update(secInfo, state); + this._browser._securityUI._update(status, state); } this._callProgressListeners( diff --git a/toolkit/modules/addons/SecurityInfo.jsm b/toolkit/modules/addons/SecurityInfo.jsm index 44cbe1c33492..de0084398aa6 100644 --- a/toolkit/modules/addons/SecurityInfo.jsm +++ b/toolkit/modules/addons/SecurityInfo.jsm @@ -94,6 +94,7 @@ const SecurityInfo = { } securityInfo.QueryInterface(Ci.nsITransportSecurityInfo); + securityInfo.QueryInterface(Ci.nsISSLStatusProvider); const SSLStatus = securityInfo.SSLStatus; if (NSSErrorsService.isNSSErrorCode(securityInfo.errorCode)) { diff --git a/toolkit/modules/tests/chrome/test_bug544442_checkCert.xul b/toolkit/modules/tests/chrome/test_bug544442_checkCert.xul index e4c96f22ed08..213c18de3d7a 100644 --- a/toolkit/modules/tests/chrome/test_bug544442_checkCert.xul +++ b/toolkit/modules/tests/chrome/test_bug544442_checkCert.xul @@ -87,7 +87,7 @@ function testXHRLoad(aEvent) { "attributes array passed to checkCert has an element that has an " + "issuerName that is not the same as the certificate's"); - var cert = channel.securityInfo.QueryInterface(Ci.nsITransportSecurityInfo). + var cert = channel.securityInfo.QueryInterface(Ci.nsISSLStatusProvider). SSLStatus.QueryInterface(Ci.nsISSLStatus).serverCert; certs = [ { issuerName: cert.issuerName, diff --git a/toolkit/mozapps/update/nsUpdateService.js b/toolkit/mozapps/update/nsUpdateService.js index 110674f3a8fa..cc591421495e 100644 --- a/toolkit/mozapps/update/nsUpdateService.js +++ b/toolkit/mozapps/update/nsUpdateService.js @@ -3116,7 +3116,7 @@ Checker.prototype = { // Set MitM pref. try { var sslStatus = request.channel.QueryInterface(Ci.nsIRequest) - .securityInfo.QueryInterface(Ci.nsITransportSecurityInfo) + .securityInfo.QueryInterface(Ci.nsISSLStatusProvider) .SSLStatus.QueryInterface(Ci.nsISSLStatus); if (sslStatus && sslStatus.serverCert && sslStatus.serverCert.issuerName) { Services.prefs.setStringPref("security.pki.mitm_canary_issuer", From ed684136aa48dbf43216385c448975f208a02fb9 Mon Sep 17 00:00:00 2001 From: shindli Date: Tue, 24 Jul 2018 02:56:41 +0300 Subject: [PATCH 22/26] Backed out 2 changesets (bug 1471894) for linting failure in /builds/worker/checkouts/gecko/tools/lint/wpt.yml on a CLOSED TREE Backed out changeset 82b13a9a70e6 (bug 1471894) Backed out changeset 9a6e53d96b83 (bug 1471894) --- layout/generic/ReflowInput.cpp | 38 +++-------- testing/web-platform/meta/MANIFEST.json | 44 +++++-------- .../tests/css/cssom/computed-style-005.html | 65 ------------------- 3 files changed, 24 insertions(+), 123 deletions(-) delete mode 100644 testing/web-platform/tests/css/cssom/computed-style-005.html diff --git a/layout/generic/ReflowInput.cpp b/layout/generic/ReflowInput.cpp index 0d68d0c163eb..0996b58f38f7 100644 --- a/layout/generic/ReflowInput.cpp +++ b/layout/generic/ReflowInput.cpp @@ -1237,20 +1237,16 @@ ReflowInput::CalculateBorderPaddingMargin( nscoord start, end; // We have to compute the start and end values if (eStyleUnit_Auto == mStyleMargin->mMargin.GetUnit(startSide)) { - // We set this to 0 for now, and fix it up later in - // InitAbsoluteConstraints (which is caller of this function, via - // CalculateHypotheticalPosition). - start = 0; + // XXX FIXME (or does CalculateBlockSideMargins do this?) + start = 0; // just ignore } else { start = nsLayoutUtils:: ComputeCBDependentValue(aContainingBlockSize, mStyleMargin->mMargin.Get(startSide)); } if (eStyleUnit_Auto == mStyleMargin->mMargin.GetUnit(endSide)) { - // We set this to 0 for now, and fix it up later in - // InitAbsoluteConstraints (which is caller of this function, via - // CalculateHypotheticalPosition). - end = 0; + // XXX FIXME (or does CalculateBlockSideMargins do this?) + end = 0; // just ignore } else { end = nsLayoutUtils:: ComputeCBDependentValue(aContainingBlockSize, @@ -1769,10 +1765,6 @@ ReflowInput::InitAbsoluteConstraints(nsPresContext* aPresContext, ComputedLogicalBorderPadding().ConvertTo(cbwm, wm); bool iSizeIsAuto = eStyleUnit_Auto == mStylePosition->ISize(cbwm).GetUnit(); - bool marginIStartIsAuto = false; - bool marginIEndIsAuto = false; - bool marginBStartIsAuto = false; - bool marginBEndIsAuto = false; if (iStartIsAuto) { // We know 'right' is not 'auto' anymore thanks to the hypothetical // box code above. @@ -1843,9 +1835,9 @@ ReflowInput::InitAbsoluteConstraints(nsPresContext* aPresContext, nscoord availMarginSpace = aCBSize.ISize(cbwm) - offsets.IStartEnd(cbwm) - margin.IStartEnd(cbwm) - borderPadding.IStartEnd(cbwm) - computedSize.ISize(cbwm); - marginIStartIsAuto = + bool marginIStartIsAuto = eStyleUnit_Auto == mStyleMargin->mMargin.GetIStartUnit(cbwm); - marginIEndIsAuto = + bool marginIEndIsAuto = eStyleUnit_Auto == mStyleMargin->mMargin.GetIEndUnit(cbwm); if (marginIStartIsAuto) { @@ -1931,9 +1923,9 @@ ReflowInput::InitAbsoluteConstraints(nsPresContext* aPresContext, // * we're dealing with a replaced element // * bsize was constrained by min- or max-bsize. nscoord availMarginSpace = autoBSize - computedSize.BSize(cbwm); - marginBStartIsAuto = + bool marginBStartIsAuto = eStyleUnit_Auto == mStyleMargin->mMargin.GetBStartUnit(cbwm); - marginBEndIsAuto = + bool marginBEndIsAuto = eStyleUnit_Auto == mStyleMargin->mMargin.GetBEndUnit(cbwm); if (marginBStartIsAuto) { @@ -1962,19 +1954,7 @@ ReflowInput::InitAbsoluteConstraints(nsPresContext* aPresContext, ComputedISize() = computedSize.ConvertTo(wm, cbwm).ISize(wm); SetComputedLogicalOffsets(offsets.ConvertTo(wm, cbwm)); - - LogicalMargin marginInOurWM = margin.ConvertTo(wm, cbwm); - SetComputedLogicalMargin(marginInOurWM); - - // If we have auto margins, update our UsedMarginProperty. The property - // will have already been created by InitOffsets if it is needed. - if (marginIStartIsAuto || marginIEndIsAuto || - marginBStartIsAuto || marginBEndIsAuto) { - nsMargin* propValue = mFrame->GetProperty(nsIFrame::UsedMarginProperty()); - MOZ_ASSERT(propValue, "UsedMarginProperty should have been created " - "by InitOffsets."); - *propValue = marginInOurWM.GetPhysicalMargin(wm); - } + SetComputedLogicalMargin(margin.ConvertTo(wm, cbwm)); } // This will not be converted to abstract coordinates because it's only diff --git a/testing/web-platform/meta/MANIFEST.json b/testing/web-platform/meta/MANIFEST.json index 38ae8b15a7cd..4f9a23872a3a 100644 --- a/testing/web-platform/meta/MANIFEST.json +++ b/testing/web-platform/meta/MANIFEST.json @@ -184511,18 +184511,6 @@ {} ] ], - "svg/linking/reftests/use-descendant-combinator-003.html": [ - [ - "/svg/linking/reftests/use-descendant-combinator-003.html", - [ - [ - "/svg/linking/reftests/use-descendant-combinator-ref.html", - "==" - ] - ], - {} - ] - ], "svg/painting/currentColor-override-pserver-fallback.svg": [ [ "/svg/painting/currentColor-override-pserver-fallback.svg", @@ -184559,6 +184547,18 @@ {} ] ], + "svg/linking/reftests/use-descendant-combinator-003.html": [ + [ + "/svg/linking/reftests/use-descendant-combinator-003.html", + [ + [ + "/svg/linking/reftests/use-descendant-combinator-ref.html", + "==" + ] + ], + {} + ] + ], "svg/painting/reftests/paint-context-001.svg": [ [ "/svg/painting/reftests/paint-context-001.svg", @@ -325276,12 +325276,6 @@ {} ] ], - "css/cssom/computed-style-005.html": [ - [ - "/css/cssom/computed-style-005.html", - {} - ] - ], "css/cssom/computed-style-set-property.html": [ [ "/css/cssom/computed-style-set-property.html", @@ -439014,7 +439008,7 @@ "testharness" ], "content-security-policy/securitypolicyviolation/img-src-redirect-upgrade-reporting.https.html": [ - "f6623c80b2b4be3fd9dd0f5dc0a6417652f1b797", + "e338e94ea726419db64ed5b98c95b862c394409e", "testharness" ], "content-security-policy/securitypolicyviolation/img-src-redirect-upgrade-reporting.https.html.headers": [ @@ -439066,7 +439060,7 @@ "testharness" ], "content-security-policy/securitypolicyviolation/support/inside-worker.sub.js": [ - "d94662579190653a3b3e9d076b79d7b0f01f2dc7", + "f425a7ae6c167bfe9857f08f460897e16bf6ca95", "support" ], "content-security-policy/securitypolicyviolation/support/inside-worker.sub.js.headers": [ @@ -552057,10 +552051,6 @@ "55010cf90dc7fc2ef8ec6cbd13d1ec947a823aed", "testharness" ], - "css/cssom/computed-style-005.html": [ - "29408580d9bed184f5807f8cb769eaafabf5b8dc", - "testharness" - ], "css/cssom/computed-style-set-property.html": [ "cb05ff525eb659d43bf234d932fd860795959c9e", "testharness" @@ -619093,10 +619083,6 @@ "3d51ca0fc007d52147e7ea03493cac7ee1bb7903", "reftest" ], - "svg/linking/reftests/use-descendant-combinator-003.html": [ - "d9155d3b92ecf0735f82ed9a0f2a8fd3fc380d55", - "reftest" - ], "svg/linking/reftests/use-descendant-combinator-ref.html": [ "fb8aec792684b97151d2964b85d1e70829e141ad", "support" @@ -622202,7 +622188,7 @@ "support" ], "webaudio/historical.html": [ - "c6e3c7d6751731c708edfb0f4e32df8a6a3b80b0", + "93068df297042344669093ce899f0230c87ebf54", "testharness" ], "webaudio/idlharness.https.html": [ diff --git a/testing/web-platform/tests/css/cssom/computed-style-005.html b/testing/web-platform/tests/css/cssom/computed-style-005.html deleted file mode 100644 index 84164fe58b35..000000000000 --- a/testing/web-platform/tests/css/cssom/computed-style-005.html +++ /dev/null @@ -1,65 +0,0 @@ - - - - CSS Test: getComputedStyle on blocks with auto margins - - - - - - - -
- - - - - From 319f5fcf1d3f5fdc80451296c97d86b84bb337da Mon Sep 17 00:00:00 2001 From: Ivan Pozdeev Date: Mon, 23 Jul 2018 18:09:15 -0700 Subject: [PATCH 23/26] Bug 1323381 - Make 8dot3 error message useful for fixing; r=gps --HG-- extra : amend_source : 72f43dbe21c9b0af738dccda80dff3b24da8e481 --- build/moz.configure/util.configure | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/build/moz.configure/util.configure b/build/moz.configure/util.configure index 5ba65f77c351..73de48cb482a 100644 --- a/build/moz.configure/util.configure +++ b/build/moz.configure/util.configure @@ -146,8 +146,12 @@ def normalize_path(): needed = GetShortPathNameW(path, out, size) if size >= needed: if ' ' in out.value: - die("GetShortPathName returned a long path name. " - "Are 8dot3 filenames disabled?") + die("GetShortPathName returned a long path name: `%s`. " + "Use `fsutil file setshortname' " + "to create a short name " + "for any components of this path " + "that have spaces.", + out.value) return normsep(out.value) size = needed From ea566d079b7c1d136372b7cb64b70ea391964f51 Mon Sep 17 00:00:00 2001 From: Chris Pearce Date: Wed, 18 Jul 2018 15:34:04 +1200 Subject: [PATCH 24/26] Bug 1476456 - Add telemetry to report whether autoplay would be not allowed if autoplay was disabled. r=baku,francois We'd like to add telemetry to help inform the decision as to how enabling block autoplay will affect video playback in the wild. Our data science team would also like some input to help them estimate the rate at which our shield study would receive pings, and the telemetry collected here will help them estimate that. We'd like to collect the following, on a per session basis: * Count of the number of top level content documents loaded, as denominator for other stats collected here. * Count of the number of top level content documents which contained (directly or in a descendant document) playback of an audible media element. * Count of the number of top level content documents which contained (directly or in a descendant document) a muted media element that was paused by the autoplay policy because it tried to unmute and it wasn't allowed to autoplay audibly. * Count of the total number of audible autoplay videos that would have not been allowed to play if block autoplay was enabled. We'd either prompt for permission on these videos, or block outright depending on user's settings. * Count of the total number of audible autoplay videos that would have been allowed to play if block autoplay was enabled. MozReview-Commit-ID: vHWJPyqHjT --HG-- extra : rebase_source : e1f22ec83fda8b65d78f1de9f02cf060d424019c --- dom/base/nsDocument.cpp | 28 ++++++++ dom/base/nsIDocument.h | 14 ++++ dom/html/HTMLMediaElement.cpp | 19 ++++++ dom/html/HTMLMediaElement.h | 2 + dom/media/AutoplayPolicy.cpp | 22 ++++-- dom/media/AutoplayPolicy.h | 7 ++ toolkit/components/telemetry/Scalars.yaml | 81 +++++++++++++++++++++++ 7 files changed, 166 insertions(+), 7 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 91e87364214c..f3779bc5d156 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -1459,6 +1459,8 @@ nsIDocument::nsIDocument() mParserAborted(false), mReportedUseCounters(false), mHasReportedShadowDOMUsage(false), + mDocTreeHadAudibleMedia(false), + mDocTreeHadPlayRevoked(false), #ifdef DEBUG mWillReparent(false), #endif @@ -1636,6 +1638,14 @@ nsDocument::~nsDocument() if (MOZ_UNLIKELY(mMathMLEnabled)) { ScalarAdd(Telemetry::ScalarID::MATHML_DOC_COUNT, 1); } + + ScalarAdd(Telemetry::ScalarID::MEDIA_PAGE_COUNT, 1); + if (mDocTreeHadAudibleMedia) { + ScalarAdd(Telemetry::ScalarID::MEDIA_PAGE_HAD_MEDIA_COUNT, 1); + } + if (mDocTreeHadPlayRevoked) { + ScalarAdd(Telemetry::ScalarID::MEDIA_PAGE_HAD_PLAY_REVOKED_COUNT, 1); + } } } @@ -12489,6 +12499,24 @@ nsIDocument::NotifyUserGestureActivation() } } +void +nsIDocument::SetDocTreeHadAudibleMedia() +{ + nsIDocument* topLevelDoc = GetTopLevelContentDocument(); + if (topLevelDoc) { + topLevelDoc->mDocTreeHadAudibleMedia = true; + } +} + +void +nsIDocument::SetDocTreeHadPlayRevoked() +{ + nsIDocument* topLevelDoc = GetTopLevelContentDocument(); + if (topLevelDoc) { + topLevelDoc->mDocTreeHadPlayRevoked = true; + } +} + void nsIDocument::MaybeAllowStorageForOpener() { diff --git a/dom/base/nsIDocument.h b/dom/base/nsIDocument.h index 57f4f7aa87ab..35572a79ac24 100644 --- a/dom/base/nsIDocument.h +++ b/dom/base/nsIDocument.h @@ -3575,6 +3575,10 @@ public: void ReportShadowDOMUsage(); + // Sets flags for media autoplay telemetry. + void SetDocTreeHadAudibleMedia(); + void SetDocTreeHadPlayRevoked(); + protected: void DoUpdateSVGUseElementShadowTrees(); @@ -4096,6 +4100,16 @@ protected: bool mHasReportedShadowDOMUsage: 1; + // True if this document contained, either directly or in a subdocument, + // an HTMLMediaElement that played audibly. This should only be set on + // top level content documents. + bool mDocTreeHadAudibleMedia: 1; + // True if this document contained, either directly or in a subdocument, + // an HTMLMediaElement that was playing inaudibly and became audible and we + // paused the HTMLMediaElement because it wasn't allowed to autoplay audibly. + // This should only be set on top level content documents. + bool mDocTreeHadPlayRevoked: 1; + #ifdef DEBUG public: bool mWillReparent: 1; diff --git a/dom/html/HTMLMediaElement.cpp b/dom/html/HTMLMediaElement.cpp index af84e419ca52..2a0bcc08b269 100644 --- a/dom/html/HTMLMediaElement.cpp +++ b/dom/html/HTMLMediaElement.cpp @@ -3057,6 +3057,7 @@ HTMLMediaElement::PauseIfShouldNotBePlaying() if (AutoplayPolicy::IsAllowedToPlay(*this) != nsIAutoplay::ALLOWED) { ErrorResult rv; Pause(rv); + OwnerDoc()->SetDocTreeHadPlayRevoked(); } } @@ -4004,6 +4005,21 @@ HTMLMediaElement::AudioChannelAgentDelayingPlayback() return mAudioChannelWrapper && mAudioChannelWrapper->IsPlaybackBlocked(); } +void +HTMLMediaElement::ReportAutoplayTelemetry() const +{ + // If we're audible, and autoplaying... + if ((Volume() > 0.0 && !Muted()) && + (!OwnerDoc()->HasBeenUserGestureActivated() || Autoplay())) { + OwnerDoc()->SetDocTreeHadAudibleMedia(); + if (AutoplayPolicy::WouldBeAllowedToPlayIfAutoplayDisabled(*this)) { + ScalarAdd(Telemetry::ScalarID::MEDIA_AUTOPLAY_WOULD_BE_ALLOWED_COUNT, 1); + } else { + ScalarAdd(Telemetry::ScalarID::MEDIA_AUTOPLAY_WOULD_NOT_BE_ALLOWED_COUNT, 1); + } + } +} + already_AddRefed HTMLMediaElement::Play(ErrorResult& aRv) { @@ -4063,6 +4079,8 @@ HTMLMediaElement::Play(ErrorResult& aRv) return promise.forget(); } + ReportAutoplayTelemetry(); + const bool handlingUserInput = EventStateManager::IsHandlingUserInput(); switch (AutoplayPolicy::IsAllowedToPlay(*this)) { case nsIAutoplay::ALLOWED: { @@ -6221,6 +6239,7 @@ HTMLMediaElement::CheckAutoplayDataReady() return; } + ReportAutoplayTelemetry(); switch (AutoplayPolicy::IsAllowedToPlay(*this)) { case nsIAutoplay::BLOCKED: return; diff --git a/dom/html/HTMLMediaElement.h b/dom/html/HTMLMediaElement.h index de66fc63bd0f..3322df0399c9 100644 --- a/dom/html/HTMLMediaElement.h +++ b/dom/html/HTMLMediaElement.h @@ -1781,6 +1781,8 @@ private: already_AddRefed CreatePlayPromise(ErrorResult& aRv) const; + void ReportAutoplayTelemetry() const; + /** * This function is called by AfterSetAttr and OnAttrSetButNotChanged. * It will not be called if the value is being unset. diff --git a/dom/media/AutoplayPolicy.cpp b/dom/media/AutoplayPolicy.cpp index c5282d8f3430..c35b7ab6eba0 100644 --- a/dom/media/AutoplayPolicy.cpp +++ b/dom/media/AutoplayPolicy.cpp @@ -100,6 +100,20 @@ DefaultAutoplayBehaviour() return prefValue; } +static bool +IsMediaElementAllowedToPlay(const HTMLMediaElement& aElement) +{ + return ((aElement.Volume() == 0.0 || aElement.Muted()) && + Preferences::GetBool("media.autoplay.allow-muted", true)) || + IsWindowAllowedToPlay(aElement.OwnerDoc()->GetInnerWindow()); +} + +/* static */ bool +AutoplayPolicy::WouldBeAllowedToPlayIfAutoplayDisabled(const HTMLMediaElement& aElement) +{ + return IsMediaElementAllowedToPlay(aElement); +} + /* static */ uint32_t AutoplayPolicy::IsAllowedToPlay(const HTMLMediaElement& aElement) { @@ -114,13 +128,7 @@ AutoplayPolicy::IsAllowedToPlay(const HTMLMediaElement& aElement) ? nsIAutoplay::ALLOWED : nsIAutoplay::BLOCKED; } - // Muted content - if ((aElement.Volume() == 0.0 || aElement.Muted()) && - Preferences::GetBool("media.autoplay.allow-muted", true)) { - return nsIAutoplay::ALLOWED; - } - - if (IsWindowAllowedToPlay(aElement.OwnerDoc()->GetInnerWindow())) { + if (IsMediaElementAllowedToPlay(aElement)) { return nsIAutoplay::ALLOWED; } diff --git a/dom/media/AutoplayPolicy.h b/dom/media/AutoplayPolicy.h index 3858c0661a35..e072c66a3c2a 100644 --- a/dom/media/AutoplayPolicy.h +++ b/dom/media/AutoplayPolicy.h @@ -38,6 +38,13 @@ public: // Returns whether a given media element is allowed to play. static uint32_t IsAllowedToPlay(const HTMLMediaElement& aElement); + // Returns true if a given media element would be allowed to play + // if block autoplay was enabled. If this returns false, it means we would + // either block or ask for permission. + // Note: this is for telemetry purposes, and doesn't check the prefs + // which enable/disable block autoplay. Do not use for blocking logic! + static bool WouldBeAllowedToPlayIfAutoplayDisabled(const HTMLMediaElement& aElement); + // Returns whether a given AudioContext is allowed to play. static bool IsAudioContextAllowedToPlay(NotNull aContext); diff --git a/toolkit/components/telemetry/Scalars.yaml b/toolkit/components/telemetry/Scalars.yaml index 51e17f22ee5a..4d40c7ef3c29 100644 --- a/toolkit/components/telemetry/Scalars.yaml +++ b/toolkit/components/telemetry/Scalars.yaml @@ -757,6 +757,87 @@ mediarecorder: - main - content +media: + page_count: + bug_numbers: + - 1476456 + description: > + The number of times a top level document is loaded. + expires: "68" + kind: uint + notification_emails: + - cpearce@mozilla.com + - alwu@mozilla.com + - nohlmeier@mozilla.com + release_channel_collection: opt-in + record_in_processes: + - main + - content + + page_had_media_count: + bug_numbers: + - 1476456 + description: > + The number of times a document hierarchy contained at least one audible HTMLMediaElement that had play() called upon it. + expires: "68" + kind: uint + notification_emails: + - cpearce@mozilla.com + - alwu@mozilla.com + - nohlmeier@mozilla.com + release_channel_collection: opt-in + record_in_processes: + - main + - content + + page_had_play_revoked_count: + bug_numbers: + - 1476456 + description: > + The number of times a document hierarchy contained at least one muted playing HTMLMediaElement that was paused due to becoming unmuted while not being allowed to autoplay. + expires: "68" + kind: uint + notification_emails: + - cpearce@mozilla.com + - alwu@mozilla.com + - nohlmeier@mozilla.com + release_channel_collection: opt-in + record_in_processes: + - main + - content + + autoplay_would_not_be_allowed_count: + bug_numbers: + - 1476456 + description: > + The number of HTMLMediaElement autoplays on audible HTMLMediaElements which would not be allowed to play if block autoplay was enabled; we'd either prompt for permission to play or block outright depending on user preferences. + expires: "68" + kind: uint + notification_emails: + - cpearce@mozilla.com + - alwu@mozilla.com + - nohlmeier@mozilla.com + release_channel_collection: opt-in + record_in_processes: + - main + - content + + autoplay_would_be_allowed_count: + bug_numbers: + - 1476456 + description: > + The number of HTMLMediaElement autoplays on audible HTMLMediaElements that would have been allowed (not blocked) by the autoplay policy if block autoplay was enabled. + expires: "68" + kind: uint + notification_emails: + - cpearce@mozilla.com + - alwu@mozilla.com + - nohlmeier@mozilla.com + release_channel_collection: opt-in + record_in_processes: + - main + - content + # The following section contains content process base counters. dom.contentprocess: troubled_due_to_memory: From 6089f7eebc7d7ef0503ee073ba735649a2c6ccea Mon Sep 17 00:00:00 2001 From: Jeff Gilbert Date: Mon, 23 Jul 2018 20:48:51 +0000 Subject: [PATCH 25/26] Bug 1475010 - Also, assert in DEBUG if we don't get a KeyedMutex. - r=kvark Disableable via env var. Differential Revision: https://phabricator.services.mozilla.com/D2276 --HG-- extra : moz-landing-system : lando --- gfx/gl/SharedSurfaceANGLE.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/gfx/gl/SharedSurfaceANGLE.cpp b/gfx/gl/SharedSurfaceANGLE.cpp index 536457d6d991..42d4cc9580b7 100644 --- a/gfx/gl/SharedSurfaceANGLE.cpp +++ b/gfx/gl/SharedSurfaceANGLE.cpp @@ -73,6 +73,18 @@ SharedSurface_ANGLEShareHandle::Create(GLContext* gl, EGLConfig config, LOCAL_EGL_DXGI_KEYED_MUTEX_ANGLE, &opaqueKeyedMutex); RefPtr keyedMutex = static_cast(opaqueKeyedMutex); +#ifdef DEBUG + if (!keyedMutex) { + std::string envStr("1"); + static auto env = PR_GetEnv("MOZ_REQUIRE_KEYED_MUTEX"); + if (env) { + envStr = env; + } + if (envStr != "0") { + MOZ_ASSERT(keyedMutex, "set MOZ_REQUIRE_KEYED_MUTEX=0 to allow"); + } + } +#endif typedef SharedSurface_ANGLEShareHandle ptrT; UniquePtr ret( new ptrT(gl, egl, size, hasAlpha, pbuffer, shareHandle, From 1764b38048c1e3bee168b93a9af0473e5ed484f3 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 18 Jul 2018 00:30:05 +0900 Subject: [PATCH 26/26] Bug 1476294 - Make HTMLEditorDocumentCommands.cpp use non-virtual methods of nsCommandParams instead of virtual methods of nsICommandParams r=Ehsan This is remaining part of bug 1450882. I forgot to update HTMLEditorDocumentCommands.cpp when I work on the bug. This patch makes HTMLEditorDocumentCommands.cpp use non-virtual methods of nsCommandParams instead of virtual methods of nsICommandParams. MozReview-Commit-ID: 12AjXbeYNOa --HG-- extra : rebase_source : 8d73866b21f6dd1c436244a771fb39bbe2debd40 --- .../libeditor/HTMLEditorDocumentCommands.cpp | 362 +++++++++++------- 1 file changed, 231 insertions(+), 131 deletions(-) diff --git a/editor/libeditor/HTMLEditorDocumentCommands.cpp b/editor/libeditor/HTMLEditorDocumentCommands.cpp index 475d4b735c8b..e0b878e8caa9 100644 --- a/editor/libeditor/HTMLEditorDocumentCommands.cpp +++ b/editor/libeditor/HTMLEditorDocumentCommands.cpp @@ -7,11 +7,11 @@ #include "mozilla/HTMLEditor.h" // for HTMLEditor #include "mozilla/HTMLEditorCommands.h" // for SetDocumentOptionsCommand, etc #include "mozilla/TextEditor.h" // for TextEditor +#include "nsCommandParams.h" // for nsCommandParams #include "nsCOMPtr.h" // for nsCOMPtr, do_QueryInterface, etc #include "nsCRT.h" // for nsCRT #include "nsDebug.h" // for NS_ENSURE_ARG_POINTER, etc #include "nsError.h" // for NS_ERROR_INVALID_ARG, etc -#include "nsICommandParams.h" // for nsICommandParams #include "nsIDocShell.h" // for nsIDocShell #include "nsIDocument.h" // for nsIDocument #include "nsIEditingSession.h" // for nsIEditingSession, etc @@ -40,7 +40,10 @@ SetDocumentOptionsCommand::IsCommandEnabled(const char* aCommandName, nsISupports* refCon, bool* outCmdEnabled) { - NS_ENSURE_ARG_POINTER(outCmdEnabled); + if (NS_WARN_IF(!outCmdEnabled)) { + return NS_ERROR_INVALID_ARG; + } + nsCOMPtr editor = do_QueryInterface(refCon); if (!editor) { *outCmdEnabled = false; @@ -64,7 +67,9 @@ SetDocumentOptionsCommand::DoCommandParams(const char* aCommandName, nsICommandParams* aParams, nsISupports* refCon) { - NS_ENSURE_ARG_POINTER(aParams); + if (NS_WARN_IF(!aParams)) { + return NS_ERROR_INVALID_ARG; + } nsCOMPtr editor = do_QueryInterface(refCon); if (NS_WARN_IF(!editor)) { @@ -78,24 +83,32 @@ SetDocumentOptionsCommand::DoCommandParams(const char* aCommandName, return NS_ERROR_FAILURE; } - int32_t animationMode; - nsresult rv = aParams->GetLongValue("imageAnimation", &animationMode); - if (NS_SUCCEEDED(rv)) { + nsCommandParams* params = aParams->AsCommandParams(); + + IgnoredErrorResult error; + int32_t animationMode = params->GetInt("imageAnimation", error); + if (!error.Failed()) { // for possible values of animation mode, see: // http://lxr.mozilla.org/seamonkey/source/image/public/imgIContainer.idl presContext->SetImageAnimationMode(animationMode); + } else { + error.SuppressException(); } - bool allowPlugins; - rv = aParams->GetBooleanValue("plugins", &allowPlugins); - if (NS_SUCCEEDED(rv)) { - nsCOMPtr docShell(presContext->GetDocShell()); - NS_ENSURE_TRUE(docShell, NS_ERROR_FAILURE); - - rv = docShell->SetAllowPlugins(allowPlugins); - NS_ENSURE_SUCCESS(rv, rv); + bool allowPlugins = params->GetBool("plugins", error); + if (error.Failed()) { + return NS_OK; } + nsCOMPtr docShell(presContext->GetDocShell()); + if (NS_WARN_IF(!docShell)) { + return NS_ERROR_FAILURE; + } + + nsresult rv = docShell->SetAllowPlugins(allowPlugins); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } return NS_OK; } @@ -104,8 +117,9 @@ SetDocumentOptionsCommand::GetCommandStateParams(const char* aCommandName, nsICommandParams* aParams, nsISupports* refCon) { - NS_ENSURE_ARG_POINTER(aParams); - NS_ENSURE_ARG_POINTER(refCon); + if (NS_WARN_IF(!aParams) || NS_WARN_IF(!refCon)) { + return NS_ERROR_INVALID_ARG; + } // The base editor owns most state info nsCOMPtr editor = do_QueryInterface(refCon); @@ -115,11 +129,15 @@ SetDocumentOptionsCommand::GetCommandStateParams(const char* aCommandName, TextEditor* textEditor = editor->AsTextEditor(); MOZ_ASSERT(textEditor); + nsCommandParams* params = aParams->AsCommandParams(); + // Always get the enabled state bool outCmdEnabled = false; IsCommandEnabled(aCommandName, refCon, &outCmdEnabled); - nsresult rv = aParams->SetBooleanValue(STATE_ENABLED, outCmdEnabled); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = params->SetBool(STATE_ENABLED, outCmdEnabled); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } // get pres context RefPtr presContext = textEditor->GetPresContext(); @@ -127,28 +145,34 @@ SetDocumentOptionsCommand::GetCommandStateParams(const char* aCommandName, return NS_ERROR_FAILURE; } - int32_t animationMode; - rv = aParams->GetLongValue("imageAnimation", &animationMode); - if (NS_SUCCEEDED(rv)) { + IgnoredErrorResult error; + Unused << params->GetInt("imageAnimation", error); + if (!error.Failed()) { // for possible values of animation mode, see // http://lxr.mozilla.org/seamonkey/source/image/public/imgIContainer.idl - rv = aParams->SetLongValue("imageAnimation", - presContext->ImageAnimationMode()); - NS_ENSURE_SUCCESS(rv, rv); + rv = params->SetInt("imageAnimation", presContext->ImageAnimationMode()); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + } else { + error.SuppressException(); } - bool allowPlugins = false; - rv = aParams->GetBooleanValue("plugins", &allowPlugins); - if (NS_SUCCEEDED(rv)) { - nsCOMPtr docShell(presContext->GetDocShell()); - NS_ENSURE_TRUE(docShell, NS_ERROR_FAILURE); - - allowPlugins = docShell->PluginsAllowedInCurrentDoc(); - - rv = aParams->SetBooleanValue("plugins", allowPlugins); - NS_ENSURE_SUCCESS(rv, rv); + bool allowPlugins = params->GetBool("plugins", error); + if (error.Failed()) { + return NS_OK; } + nsCOMPtr docShell(presContext->GetDocShell()); + if (NS_WARN_IF(!docShell)) { + return NS_ERROR_FAILURE; + } + + allowPlugins = docShell->PluginsAllowedInCurrentDoc(); + rv = params->SetBool("plugins", allowPlugins); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } return NS_OK; } @@ -166,8 +190,11 @@ SetDocumentStateCommand::IsCommandEnabled(const char* aCommandName, nsISupports* refCon, bool* outCmdEnabled) { + if (NS_WARN_IF(!outCmdEnabled)) { + return NS_ERROR_INVALID_ARG; + } + // These commands are always enabled - NS_ENSURE_ARG_POINTER(outCmdEnabled); *outCmdEnabled = true; return NS_OK; } @@ -184,6 +211,10 @@ SetDocumentStateCommand::DoCommandParams(const char* aCommandName, nsICommandParams* aParams, nsISupports* refCon) { + if (NS_WARN_IF(!aParams)) { + return NS_ERROR_INVALID_ARG; + } + nsCOMPtr editor = do_QueryInterface(refCon); if (NS_WARN_IF(!editor)) { return NS_ERROR_INVALID_ARG; @@ -191,73 +222,95 @@ SetDocumentStateCommand::DoCommandParams(const char* aCommandName, TextEditor* textEditor = editor->AsTextEditor(); MOZ_ASSERT(textEditor); + nsCommandParams* params = aParams->AsCommandParams(); + if (!nsCRT::strcmp(aCommandName, "cmd_setDocumentModified")) { - NS_ENSURE_ARG_POINTER(aParams); - - bool modified; - nsresult rv = aParams->GetBooleanValue(STATE_ATTRIBUTE, &modified); - + ErrorResult error; + bool modified = params->GetBool(STATE_ATTRIBUTE, error); // Should we fail if this param wasn't set? // I'm not sure we should be that strict - NS_ENSURE_SUCCESS(rv, rv); - - if (modified) { - return textEditor->IncrementModificationCount(1); + if (NS_WARN_IF(error.Failed())) { + return error.StealNSResult(); } - - return textEditor->ResetModificationCount(); + if (modified) { + nsresult rv = textEditor->IncrementModificationCount(1); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; + } + nsresult rv = textEditor->ResetModificationCount(); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } if (!nsCRT::strcmp(aCommandName, "cmd_setDocumentReadOnly")) { - NS_ENSURE_ARG_POINTER(aParams); - bool isReadOnly; - nsresult rvRO = aParams->GetBooleanValue(STATE_ATTRIBUTE, &isReadOnly); - NS_ENSURE_SUCCESS(rvRO, rvRO); - return isReadOnly ? - textEditor->AddFlags(nsIPlaintextEditor::eEditorReadonlyMask) : + ErrorResult error; + bool isReadOnly = params->GetBool(STATE_ATTRIBUTE, error); + if (NS_WARN_IF(error.Failed())) { + return error.StealNSResult(); + } + if (isReadOnly) { + nsresult rv = + textEditor->AddFlags(nsIPlaintextEditor::eEditorReadonlyMask); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; + } + nsresult rv = textEditor->RemoveFlags(nsIPlaintextEditor::eEditorReadonlyMask); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } if (!nsCRT::strcmp(aCommandName, "cmd_setDocumentUseCSS")) { - NS_ENSURE_ARG_POINTER(aParams); HTMLEditor* htmlEditor = textEditor->AsHTMLEditor(); if (NS_WARN_IF(!htmlEditor)) { return NS_ERROR_INVALID_ARG; } - - bool desireCSS; - nsresult rvCSS = aParams->GetBooleanValue(STATE_ATTRIBUTE, &desireCSS); - NS_ENSURE_SUCCESS(rvCSS, rvCSS); - - return htmlEditor->SetIsCSSEnabled(desireCSS); + ErrorResult error; + bool desireCSS = params->GetBool(STATE_ATTRIBUTE, error); + if (NS_WARN_IF(error.Failed())) { + return error.StealNSResult(); + } + nsresult rv = htmlEditor->SetIsCSSEnabled(desireCSS); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } if (!nsCRT::strcmp(aCommandName, "cmd_insertBrOnReturn")) { - NS_ENSURE_ARG_POINTER(aParams); HTMLEditor* htmlEditor = textEditor->AsHTMLEditor(); if (NS_WARN_IF(!htmlEditor)) { return NS_ERROR_INVALID_ARG; } - - bool insertBrOnReturn; - nsresult rvBR = aParams->GetBooleanValue(STATE_ATTRIBUTE, - &insertBrOnReturn); - NS_ENSURE_SUCCESS(rvBR, rvBR); - - return htmlEditor->SetReturnInParagraphCreatesNewParagraph(!insertBrOnReturn); + ErrorResult error; + bool insertBrOnReturn = params->GetBool(STATE_ATTRIBUTE, error); + if (NS_WARN_IF(error.Failed())) { + return error.StealNSResult(); + } + nsresult rv = + htmlEditor->SetReturnInParagraphCreatesNewParagraph(!insertBrOnReturn); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } if (!nsCRT::strcmp(aCommandName, "cmd_defaultParagraphSeparator")) { - if (NS_WARN_IF(!aParams)) { - return NS_ERROR_NULL_POINTER; - } HTMLEditor* htmlEditor = textEditor->AsHTMLEditor(); if (NS_WARN_IF(!htmlEditor)) { return NS_ERROR_INVALID_ARG; } nsAutoCString newValue; - nsresult rv = aParams->GetCStringValue(STATE_ATTRIBUTE, newValue); + nsresult rv = params->GetCString(STATE_ATTRIBUTE, newValue); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -282,31 +335,37 @@ SetDocumentStateCommand::DoCommandParams(const char* aCommandName, } if (!nsCRT::strcmp(aCommandName, "cmd_enableObjectResizing")) { - NS_ENSURE_ARG_POINTER(aParams); HTMLEditor* htmlEditor = textEditor->AsHTMLEditor(); if (NS_WARN_IF(!htmlEditor)) { return NS_ERROR_INVALID_ARG; } - - bool enabled; - nsresult rvOR = aParams->GetBooleanValue(STATE_ATTRIBUTE, &enabled); - NS_ENSURE_SUCCESS(rvOR, rvOR); - - return htmlEditor->SetObjectResizingEnabled(enabled); + ErrorResult error; + bool enabled = params->GetBool(STATE_ATTRIBUTE, error); + if (NS_WARN_IF(error.Failed())) { + return error.StealNSResult(); + } + nsresult rv = htmlEditor->SetObjectResizingEnabled(enabled); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } if (!nsCRT::strcmp(aCommandName, "cmd_enableInlineTableEditing")) { - NS_ENSURE_ARG_POINTER(aParams); HTMLEditor* htmlEditor = textEditor->AsHTMLEditor(); if (NS_WARN_IF(!htmlEditor)) { return NS_ERROR_INVALID_ARG; } - - bool enabled; - nsresult rvOR = aParams->GetBooleanValue(STATE_ATTRIBUTE, &enabled); - NS_ENSURE_SUCCESS(rvOR, rvOR); - - return htmlEditor->SetInlineTableEditingEnabled(enabled); + ErrorResult error; + bool enabled = params->GetBool(STATE_ATTRIBUTE, error); + if (NS_WARN_IF(error.Failed())) { + return error.StealNSResult(); + } + nsresult rv = htmlEditor->SetInlineTableEditingEnabled(enabled); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } return NS_ERROR_NOT_IMPLEMENTED; @@ -317,8 +376,9 @@ SetDocumentStateCommand::GetCommandStateParams(const char* aCommandName, nsICommandParams* aParams, nsISupports* refCon) { - NS_ENSURE_ARG_POINTER(aParams); - NS_ENSURE_ARG_POINTER(refCon); + if (NS_WARN_IF(!aParams) || NS_WARN_IF(!refCon)) { + return NS_ERROR_INVALID_ARG; + } // The base editor owns most state info nsCOMPtr editor = do_QueryInterface(refCon); @@ -328,71 +388,93 @@ SetDocumentStateCommand::GetCommandStateParams(const char* aCommandName, TextEditor* textEditor = editor->AsTextEditor(); MOZ_ASSERT(textEditor); + nsCommandParams* params = aParams->AsCommandParams(); + // Always get the enabled state bool outCmdEnabled = false; IsCommandEnabled(aCommandName, refCon, &outCmdEnabled); - nsresult rv = aParams->SetBooleanValue(STATE_ENABLED, outCmdEnabled); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = params->SetBool(STATE_ENABLED, outCmdEnabled); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } if (!nsCRT::strcmp(aCommandName, "cmd_setDocumentModified")) { bool modified; rv = textEditor->GetDocumentModified(&modified); - NS_ENSURE_SUCCESS(rv, rv); - - return aParams->SetBooleanValue(STATE_ATTRIBUTE, modified); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + rv = params->SetBool(STATE_ATTRIBUTE, modified); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } if (!nsCRT::strcmp(aCommandName, "cmd_setDocumentReadOnly")) { - NS_ENSURE_ARG_POINTER(aParams); - return aParams->SetBooleanValue(STATE_ATTRIBUTE, textEditor->IsReadonly()); + rv = params->SetBool(STATE_ATTRIBUTE, textEditor->IsReadonly()); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } if (!nsCRT::strcmp(aCommandName, "cmd_setDocumentUseCSS")) { - NS_ENSURE_ARG_POINTER(aParams); HTMLEditor* htmlEditor = textEditor->AsHTMLEditor(); if (NS_WARN_IF(!htmlEditor)) { return NS_ERROR_INVALID_ARG; } - bool isCSS; htmlEditor->GetIsCSSEnabled(&isCSS); - return aParams->SetBooleanValue(STATE_ALL, isCSS); + rv = params->SetBool(STATE_ALL, isCSS); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } if (!nsCRT::strcmp(aCommandName, "cmd_insertBrOnReturn")) { - NS_ENSURE_ARG_POINTER(aParams); HTMLEditor* htmlEditor = textEditor->AsHTMLEditor(); if (NS_WARN_IF(!htmlEditor)) { return NS_ERROR_INVALID_ARG; } - bool createPOnReturn; htmlEditor->GetReturnInParagraphCreatesNewParagraph(&createPOnReturn); - return aParams->SetBooleanValue(STATE_ATTRIBUTE, !createPOnReturn); + rv = params->SetBool(STATE_ATTRIBUTE, !createPOnReturn); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } if (!nsCRT::strcmp(aCommandName, "cmd_defaultParagraphSeparator")) { - if (NS_WARN_IF(!aParams)) { - return NS_ERROR_NULL_POINTER; - } HTMLEditor* htmlEditor = textEditor->AsHTMLEditor(); if (NS_WARN_IF(!htmlEditor)) { return NS_ERROR_INVALID_ARG; } switch (htmlEditor->GetDefaultParagraphSeparator()) { - case ParagraphSeparator::div: - aParams->SetCStringValue(STATE_ATTRIBUTE, NS_LITERAL_CSTRING("div")); + case ParagraphSeparator::div: { + DebugOnly rv = + params->SetCString(STATE_ATTRIBUTE, NS_LITERAL_CSTRING("div")); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), + "Failed to set command params to return \"div\""); return NS_OK; - - case ParagraphSeparator::p: - aParams->SetCStringValue(STATE_ATTRIBUTE, NS_LITERAL_CSTRING("p")); + } + case ParagraphSeparator::p: { + DebugOnly rv = + params->SetCString(STATE_ATTRIBUTE, NS_LITERAL_CSTRING("p")); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), + "Failed to set command params to return \"p\""); return NS_OK; - - case ParagraphSeparator::br: - aParams->SetCStringValue(STATE_ATTRIBUTE, NS_LITERAL_CSTRING("br")); + } + case ParagraphSeparator::br: { + DebugOnly rv = + params->SetCString(STATE_ATTRIBUTE, NS_LITERAL_CSTRING("br")); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), + "Failed to set command params to return \"br\""); return NS_OK; - + } default: MOZ_ASSERT_UNREACHABLE("Invalid paragraph separator value"); return NS_ERROR_UNEXPECTED; @@ -400,27 +482,31 @@ SetDocumentStateCommand::GetCommandStateParams(const char* aCommandName, } if (!nsCRT::strcmp(aCommandName, "cmd_enableObjectResizing")) { - NS_ENSURE_ARG_POINTER(aParams); HTMLEditor* htmlEditor = textEditor->AsHTMLEditor(); if (NS_WARN_IF(!htmlEditor)) { return NS_ERROR_INVALID_ARG; } - bool enabled; htmlEditor->GetObjectResizingEnabled(&enabled); - return aParams->SetBooleanValue(STATE_ATTRIBUTE, enabled); + rv = params->SetBool(STATE_ATTRIBUTE, enabled); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } if (!nsCRT::strcmp(aCommandName, "cmd_enableInlineTableEditing")) { - NS_ENSURE_ARG_POINTER(aParams); HTMLEditor* htmlEditor = textEditor->AsHTMLEditor(); if (NS_WARN_IF(!htmlEditor)) { return NS_ERROR_INVALID_ARG; } - bool enabled; htmlEditor->GetInlineTableEditingEnabled(&enabled); - return aParams->SetBooleanValue(STATE_ATTRIBUTE, enabled); + rv = params->SetBool(STATE_ATTRIBUTE, enabled); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } return NS_ERROR_NOT_IMPLEMENTED; @@ -468,7 +554,10 @@ DocumentStateCommand::IsCommandEnabled(const char* aCommandName, nsISupports* refCon, bool* outCmdEnabled) { - NS_ENSURE_ARG_POINTER(outCmdEnabled); + if (NS_WARN_IF(!outCmdEnabled)) { + return NS_ERROR_INVALID_ARG; + } + // Always return false to discourage callers from using DoCommand() *outCmdEnabled = false; return NS_OK; @@ -494,9 +583,11 @@ DocumentStateCommand::GetCommandStateParams(const char* aCommandName, nsICommandParams* aParams, nsISupports* refCon) { - NS_ENSURE_ARG_POINTER(aParams); - NS_ENSURE_ARG_POINTER(aCommandName); - nsresult rv; + if (NS_WARN_IF(!aParams) || NS_WARN_IF(!aCommandName)) { + return NS_ERROR_INVALID_ARG; + } + + nsCommandParams* params = aParams->AsCommandParams(); if (!nsCRT::strcmp(aCommandName, "obs_documentCreated")) { uint32_t editorStatus = nsIEditingSession::eEditorErrorUnknown; @@ -508,8 +599,10 @@ DocumentStateCommand::GetCommandStateParams(const char* aCommandName, // Embedder gets error status if this fails // If called before startup is finished, // status = eEditorCreationInProgress - rv = editingSession->GetEditorStatus(&editorStatus); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = editingSession->GetEditorStatus(&editorStatus); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } else { // If refCon is an editor, then everything started up OK! nsCOMPtr editor = do_QueryInterface(refCon); @@ -520,7 +613,8 @@ DocumentStateCommand::GetCommandStateParams(const char* aCommandName, // Note that if refCon is not-null, but is neither // an nsIEditingSession or nsIEditor, we return "eEditorErrorUnknown" - aParams->SetLongValue(STATE_DATA, editorStatus); + DebugOnly rv = params->SetInt(STATE_DATA, editorStatus); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to set editor status"); return NS_OK; } @@ -533,12 +627,18 @@ DocumentStateCommand::GetCommandStateParams(const char* aCommandName, MOZ_ASSERT(textEditor); nsCOMPtr doc = textEditor->GetDocument(); - NS_ENSURE_TRUE(doc, NS_ERROR_FAILURE); - - nsIURI *uri = doc->GetDocumentURI(); - NS_ENSURE_TRUE(uri, NS_ERROR_FAILURE); - - return aParams->SetISupportsValue(STATE_DATA, (nsISupports*)uri); + if (NS_WARN_IF(!doc)) { + return NS_ERROR_FAILURE; + } + nsIURI* uri = doc->GetDocumentURI(); + if (NS_WARN_IF(!uri)) { + return NS_ERROR_FAILURE; + } + nsresult rv = params->SetISupports(STATE_DATA, uri); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } return NS_ERROR_NOT_IMPLEMENTED;