Skip to content
This repository was archived by the owner on Dec 11, 2019. It is now read-only.

Commit 13331a3

Browse files
bbondycezaraugusto
authored andcommitted
Closed frame events sometimes get sent to first frame
Fix #6513 Auditors: @darkdh When we close a frame we remove the frame props, this is removed even before the react control and DOM elments know. An event can come in after this and dispatch. Eventually most of these webview events will be moved into app/browser/tab.js.
1 parent 67211a0 commit 13331a3

File tree

1 file changed

+90
-3
lines changed

1 file changed

+90
-3
lines changed

js/components/frame.js

+90-3
Original file line numberDiff line numberDiff line change
@@ -588,19 +588,31 @@ class Frame extends ImmutableComponent {
588588

589589
addEventListeners () {
590590
this.webview.addEventListener('content-blocked', (e) => {
591+
if (this.frame.isEmpty()) {
592+
return
593+
}
591594
if (e.details[0] === 'javascript' && e.details[1]) {
592595
windowActions.setBlockedBy(this.frame, 'noScript', e.details[1])
593596
}
594597
})
595598
this.webview.addEventListener('did-block-run-insecure-content', (e) => {
599+
if (this.frame.isEmpty()) {
600+
return
601+
}
596602
windowActions.setBlockedRunInsecureContent(this.frame, e.details[0])
597603
})
598604
this.webview.addEventListener('enable-pepper-menu', (e) => {
605+
if (this.frame.isEmpty()) {
606+
return
607+
}
599608
contextMenus.onFlashContextMenu(e.params, this.frame)
600609
e.preventDefault()
601610
e.stopPropagation()
602611
})
603612
this.webview.addEventListener('context-menu', (e) => {
613+
if (this.frame.isEmpty()) {
614+
return
615+
}
604616
contextMenus.onMainContextMenu(e.params, this.frame)
605617
e.preventDefault()
606618
e.stopPropagation()
@@ -616,6 +628,9 @@ class Frame extends ImmutableComponent {
616628
windowActions.setLinkHoverPreview(e.url, showOnRight)
617629
})
618630
this.webview.addEventListener('set-active', (e) => {
631+
if (this.frame.isEmpty()) {
632+
return
633+
}
619634
if (e.active && currentWindow.isFocused()) {
620635
windowActions.setFocusedFrame(this.frame)
621636
}
@@ -631,31 +646,49 @@ class Frame extends ImmutableComponent {
631646
currentWindow.webContents.send(messages.DISABLE_SWIPE_GESTURE)
632647
})
633648
this.webview.addEventListener('did-attach', (e) => {
649+
if (this.frame.isEmpty()) {
650+
return
651+
}
634652
let tabId = this.webview.getId()
635653
if (this.props.tabId !== tabId) {
636654
windowActions.setFrameTabId(this.frame, tabId)
637655
}
638656
})
639657
this.webview.addEventListener('destroyed', (e) => {
658+
if (this.frame.isEmpty()) {
659+
return
660+
}
640661
this.props.onCloseFrame(this.frame)
641662
})
642663
this.webview.addEventListener('close', () => {
664+
if (this.frame.isEmpty()) {
665+
return
666+
}
643667
this.props.onCloseFrame(this.frame)
644668
})
645669
this.webview.addEventListener('page-favicon-updated', (e) => {
670+
if (this.frame.isEmpty()) {
671+
return
672+
}
646673
if (e.favicons && e.favicons.length > 0) {
647674
imageUtil.getWorkingImageUrl(e.favicons[0], (imageFound) => {
648675
windowActions.setFavicon(this.frame, imageFound ? e.favicons[0] : null)
649676
})
650677
}
651678
})
652679
this.webview.addEventListener('page-title-updated', ({title}) => {
680+
if (this.frame.isEmpty()) {
681+
return
682+
}
653683
windowActions.setFrameTitle(this.frame, title)
654684
})
655685
this.webview.addEventListener('show-autofill-settings', (e) => {
656686
windowActions.newFrame({ location: 'about:autofill' }, true)
657687
})
658688
this.webview.addEventListener('show-autofill-popup', (e) => {
689+
if (this.frame.isEmpty()) {
690+
return
691+
}
659692
contextMenus.onShowAutofillMenu(e.suggestions, e.rect, this.frame)
660693
})
661694
this.webview.addEventListener('hide-autofill-popup', (e) => {
@@ -670,16 +703,25 @@ class Frame extends ImmutableComponent {
670703
this.updateAboutDetails()
671704
break
672705
case messages.GOT_CANVAS_FINGERPRINTING:
706+
if (this.frame.isEmpty()) {
707+
return
708+
}
673709
method = (detail) => {
674710
const description = [detail.type, detail.scriptUrl || this.props.provisionalLocation].join(': ')
675711
windowActions.setBlockedBy(this.frame, 'fingerprintingProtection', description)
676712
}
677713
break
678714
case messages.THEME_COLOR_COMPUTED:
715+
if (this.frame.isEmpty()) {
716+
return
717+
}
679718
method = (computedThemeColor) =>
680719
windowActions.setThemeColor(this.frame, undefined, computedThemeColor || null)
681720
break
682721
case messages.CONTEXT_MENU_OPENED:
722+
if (this.frame.isEmpty()) {
723+
return
724+
}
683725
method = (nodeProps, contextMenuType) => {
684726
contextMenus.onMainContextMenu(nodeProps, this.frame, contextMenuType)
685727
}
@@ -719,6 +761,9 @@ class Frame extends ImmutableComponent {
719761
})
720762

721763
const loadStart = (e) => {
764+
if (this.frame.isEmpty()) {
765+
return
766+
}
722767
if (e.isMainFrame && !e.isErrorPage && !e.isFrameSrcDoc) {
723768
windowActions.onWebviewLoadStart(this.frame, e.url)
724769
// Clear security state
@@ -731,6 +776,9 @@ class Frame extends ImmutableComponent {
731776
}
732777

733778
const loadEnd = (savePage) => {
779+
if (this.frame.isEmpty()) {
780+
return
781+
}
734782
windowActions.onWebviewLoadEnd(
735783
this.frame,
736784
this.webview.getURL())
@@ -764,6 +812,9 @@ class Frame extends ImmutableComponent {
764812
}
765813

766814
const loadFail = (e, provisionLoadFailure = false) => {
815+
if (this.frame.isEmpty()) {
816+
return
817+
}
767818
if (isFrameError(e.errorCode)) {
768819
// temporary workaround for https://github.com/brave/browser-laptop/issues/1817
769820
if (e.validatedURL === aboutUrls.get('about:newtab') ||
@@ -794,6 +845,9 @@ class Frame extends ImmutableComponent {
794845
}
795846
}
796847
this.webview.addEventListener('security-style-changed', (e) => {
848+
if (this.frame.isEmpty()) {
849+
return
850+
}
797851
let isSecure = null
798852
let runInsecureContent = this.runInsecureContent()
799853
// 'warning' and 'passive mixed content' should never upgrade the
@@ -835,18 +889,25 @@ class Frame extends ImmutableComponent {
835889
if (this.props.isActive && !isNewTabPage && document.activeElement !== this.webview) {
836890
this.webview.focus()
837891
}
838-
windowActions.setNavigated(e.url, this.props.frameKey, false, this.frame.get('tabId'))
892+
if (!this.frame.isEmpty()) {
893+
windowActions.setNavigated(e.url, this.props.frameKey, false, this.frame.get('tabId'))
894+
}
839895
// force temporary url display for tabnapping protection
840896
windowActions.setMouseInTitlebar(true)
841897

842898
// After navigating to the URL via back/forward buttons, set correct frame title
843899
if (!e.isRendererInitiated) {
844900
let index = this.webview.getCurrentEntryIndex()
845901
let title = this.webview.getTitleAtIndex(index)
846-
windowActions.setFrameTitle(this.frame, title)
902+
if (!this.frame.isEmpty()) {
903+
windowActions.setFrameTitle(this.frame, title)
904+
}
847905
}
848906
})
849907
this.webview.addEventListener('crashed', (e) => {
908+
if (this.frame.isEmpty()) {
909+
return
910+
}
850911
windowActions.setFrameError(this.frame, {
851912
event_type: 'crashed',
852913
title: 'unexpectedError',
@@ -874,33 +935,54 @@ class Frame extends ImmutableComponent {
874935
}
875936
})
876937
this.webview.addEventListener('did-navigate-in-page', (e) => {
938+
if (this.frame.isEmpty()) {
939+
return
940+
}
877941
if (e.isMainFrame) {
878942
windowActions.setNavigated(e.url, this.props.frameKey, true, this.frame.get('tabId'))
879943
loadEnd(true)
880944
}
881945
})
882946
this.webview.addEventListener('enter-html-full-screen', () => {
947+
if (this.frame.isEmpty()) {
948+
return
949+
}
883950
windowActions.setFullScreen(this.frame, true, true)
884951
// disable the fullscreen warning after 5 seconds
885952
setTimeout(windowActions.setFullScreen.bind(this, this.frame, undefined, false), 5000)
886953
})
887954
this.webview.addEventListener('leave-html-full-screen', () => {
955+
if (this.frame.isEmpty()) {
956+
return
957+
}
888958
windowActions.setFullScreen(this.frame, false)
889959
})
890960
this.webview.addEventListener('media-started-playing', ({title}) => {
961+
if (this.frame.isEmpty()) {
962+
return
963+
}
891964
windowActions.setAudioPlaybackActive(this.frame, true)
892965
})
893966
this.webview.addEventListener('media-paused', ({title}) => {
967+
if (this.frame.isEmpty()) {
968+
return
969+
}
894970
windowActions.setAudioPlaybackActive(this.frame, false)
895971
})
896972
this.webview.addEventListener('did-change-theme-color', ({themeColor}) => {
973+
if (this.frame.isEmpty()) {
974+
return
975+
}
897976
// Due to a bug in Electron, after navigating to a page with a theme color
898977
// to a page without a theme color, the background is sent to us as black
899978
// even know there is no background. To work around this we just ignore
900979
// the theme color in that case and let the computed theme color take over.
901980
windowActions.setThemeColor(this.frame, themeColor !== '#000000' ? themeColor : null)
902981
})
903982
this.webview.addEventListener('found-in-page', (e) => {
983+
if (this.frame.isEmpty()) {
984+
return
985+
}
904986
if (e.result !== undefined && (e.result.matches !== undefined || e.result.activeMatchOrdinal !== undefined)) {
905987
if (e.result.matches === 0) {
906988
windowActions.setFindDetail(this.frame, Immutable.fromJS({
@@ -916,6 +998,9 @@ class Frame extends ImmutableComponent {
916998
}
917999
})
9181000
this.webview.addEventListener('did-get-response-details', (details) => {
1001+
if (this.frame.isEmpty()) {
1002+
return
1003+
}
9191004
windowActions.gotResponseDetails(this.frame.get('tabId'), details)
9201005
})
9211006
// Handle zoom using Ctrl/Cmd and the mouse wheel.
@@ -982,7 +1067,9 @@ class Frame extends ImmutableComponent {
9821067
}
9831068

9841069
onFocus () {
985-
windowActions.setTabPageIndexByFrame(this.frame)
1070+
if (!this.frame.isEmpty()) {
1071+
windowActions.setTabPageIndexByFrame(this.frame)
1072+
}
9861073

9871074
// Make sure urlBar focused state is updated so that on tab
9881075
// changes the focus state doesn't go back to the urlBar

0 commit comments

Comments
 (0)