Skip to content

Commit

Permalink
Closed frame events sometimes get sent to first frame
Browse files Browse the repository at this point in the history 
Fix brave#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.
  • Loading branch information
@bbondy
bbondy committed Jan 9, 2017
1 parent 6168b39 commit 6923c7f
Showing 1 changed file with 90 additions and 3 deletions.
93 changes: 90 additions & 3 deletions js/components/frame.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -588,19 +588,31 @@ class Frame extends ImmutableComponent {

addEventListeners () {
this.webview.addEventListener('content-blocked', (e) => {
if (this.frame.isEmpty()) {
return
}
if (e.details[0] === 'javascript' && e.details[1]) {
windowActions.setBlockedBy(this.frame, 'noScript', e.details[1])
}
})
this.webview.addEventListener('did-block-run-insecure-content', (e) => {
if (this.frame.isEmpty()) {
return
}
windowActions.setBlockedRunInsecureContent(this.frame, e.details[0])
})
this.webview.addEventListener('enable-pepper-menu', (e) => {
if (this.frame.isEmpty()) {
return
}
contextMenus.onFlashContextMenu(e.params, this.frame)
e.preventDefault()
e.stopPropagation()
})
this.webview.addEventListener('context-menu', (e) => {
if (this.frame.isEmpty()) {
return
}
contextMenus.onMainContextMenu(e.params, this.frame)
e.preventDefault()
e.stopPropagation()
Expand All @@ -616,6 +628,9 @@ class Frame extends ImmutableComponent {
windowActions.setLinkHoverPreview(e.url, showOnRight)
})
this.webview.addEventListener('set-active', (e) => {
if (this.frame.isEmpty()) {
return
}
if (e.active && currentWindow.isFocused()) {
windowActions.setFocusedFrame(this.frame)
}
Expand All @@ -631,31 +646,49 @@ class Frame extends ImmutableComponent {
currentWindow.webContents.send(messages.DISABLE_SWIPE_GESTURE)
})
this.webview.addEventListener('did-attach', (e) => {
if (this.frame.isEmpty()) {
return
}
let tabId = this.webview.getId()
if (this.props.tabId !== tabId) {
windowActions.setFrameTabId(this.frame, tabId)
}
})
this.webview.addEventListener('destroyed', (e) => {
if (this.frame.isEmpty()) {
return
}
this.props.onCloseFrame(this.frame)
})
this.webview.addEventListener('close', () => {
if (this.frame.isEmpty()) {
return
}
this.props.onCloseFrame(this.frame)
})
this.webview.addEventListener('page-favicon-updated', (e) => {
if (this.frame.isEmpty()) {
return
}
if (e.favicons && e.favicons.length > 0) {
imageUtil.getWorkingImageUrl(e.favicons[0], (imageFound) => {
windowActions.setFavicon(this.frame, imageFound ? e.favicons[0] : null)
})
}
})
this.webview.addEventListener('page-title-updated', ({title}) => {
if (this.frame.isEmpty()) {
return
}
windowActions.setFrameTitle(this.frame, title)
})
this.webview.addEventListener('show-autofill-settings', (e) => {
windowActions.newFrame({ location: 'about:autofill' }, true)
})
this.webview.addEventListener('show-autofill-popup', (e) => {
if (this.frame.isEmpty()) {
return
}
contextMenus.onShowAutofillMenu(e.suggestions, e.rect, this.frame)
})
this.webview.addEventListener('hide-autofill-popup', (e) => {
Expand All @@ -670,16 +703,25 @@ class Frame extends ImmutableComponent {
this.updateAboutDetails()
break
case messages.GOT_CANVAS_FINGERPRINTING:
if (this.frame.isEmpty()) {
return
}
method = (detail) => {
const description = [detail.type, detail.scriptUrl || this.props.provisionalLocation].join(': ')
windowActions.setBlockedBy(this.frame, 'fingerprintingProtection', description)
}
break
case messages.THEME_COLOR_COMPUTED:
if (this.frame.isEmpty()) {
return
}
method = (computedThemeColor) =>
windowActions.setThemeColor(this.frame, undefined, computedThemeColor || null)
break
case messages.CONTEXT_MENU_OPENED:
if (this.frame.isEmpty()) {
return
}
method = (nodeProps, contextMenuType) => {
contextMenus.onMainContextMenu(nodeProps, this.frame, contextMenuType)
}
Expand Down Expand Up @@ -719,6 +761,9 @@ class Frame extends ImmutableComponent {
})

const loadStart = (e) => {
if (this.frame.isEmpty()) {
return
}
if (e.isMainFrame && !e.isErrorPage && !e.isFrameSrcDoc) {
windowActions.onWebviewLoadStart(this.frame, e.url)
// Clear security state
Expand All @@ -731,6 +776,9 @@ class Frame extends ImmutableComponent {
}

const loadEnd = (savePage) => {
if (this.frame.isEmpty()) {
return
}
windowActions.onWebviewLoadEnd(
this.frame,
this.webview.getURL())
Expand Down Expand Up @@ -764,6 +812,9 @@ class Frame extends ImmutableComponent {
}

const loadFail = (e, provisionLoadFailure = false) => {
if (this.frame.isEmpty()) {
return
}
if (isFrameError(e.errorCode)) {
// temporary workaround for https://github.com/brave/browser-laptop/issues/1817
if (e.validatedURL === aboutUrls.get('about:newtab') ||
Expand Down Expand Up @@ -794,6 +845,9 @@ class Frame extends ImmutableComponent {
}
}
this.webview.addEventListener('security-style-changed', (e) => {
if (this.frame.isEmpty()) {
return
}
let isSecure = null
let runInsecureContent = this.runInsecureContent()
// 'warning' and 'passive mixed content' should never upgrade the
Expand Down Expand Up @@ -835,18 +889,25 @@ class Frame extends ImmutableComponent {
if (this.props.isActive && !isNewTabPage && document.activeElement !== this.webview) {
this.webview.focus()
}
windowActions.setNavigated(e.url, this.props.frameKey, false, this.frame.get('tabId'))
if (!this.frame.isEmpty()) {
windowActions.setNavigated(e.url, this.props.frameKey, false, this.frame.get('tabId'))
}
// force temporary url display for tabnapping protection
windowActions.setMouseInTitlebar(true)

// After navigating to the URL via back/forward buttons, set correct frame title
if (!e.isRendererInitiated) {
let index = this.webview.getCurrentEntryIndex()
let title = this.webview.getTitleAtIndex(index)
windowActions.setFrameTitle(this.frame, title)
if (!this.frame.isEmpty()) {
windowActions.setFrameTitle(this.frame, title)
}
}
})
this.webview.addEventListener('crashed', (e) => {
if (this.frame.isEmpty()) {
return
}
windowActions.setFrameError(this.frame, {
event_type: 'crashed',
title: 'unexpectedError',
Expand Down Expand Up @@ -874,33 +935,54 @@ class Frame extends ImmutableComponent {
}
})
this.webview.addEventListener('did-navigate-in-page', (e) => {
if (this.frame.isEmpty()) {
return
}
if (e.isMainFrame) {
windowActions.setNavigated(e.url, this.props.frameKey, true, this.frame.get('tabId'))
loadEnd(true)
}
})
this.webview.addEventListener('enter-html-full-screen', () => {
if (this.frame.isEmpty()) {
return
}
windowActions.setFullScreen(this.frame, true, true)
// disable the fullscreen warning after 5 seconds
setTimeout(windowActions.setFullScreen.bind(this, this.frame, undefined, false), 5000)
})
this.webview.addEventListener('leave-html-full-screen', () => {
if (this.frame.isEmpty()) {
return
}
windowActions.setFullScreen(this.frame, false)
})
this.webview.addEventListener('media-started-playing', ({title}) => {
if (this.frame.isEmpty()) {
return
}
windowActions.setAudioPlaybackActive(this.frame, true)
})
this.webview.addEventListener('media-paused', ({title}) => {
if (this.frame.isEmpty()) {
return
}
windowActions.setAudioPlaybackActive(this.frame, false)
})
this.webview.addEventListener('did-change-theme-color', ({themeColor}) => {
if (this.frame.isEmpty()) {
return
}
// Due to a bug in Electron, after navigating to a page with a theme color
// to a page without a theme color, the background is sent to us as black
// even know there is no background. To work around this we just ignore
// the theme color in that case and let the computed theme color take over.
windowActions.setThemeColor(this.frame, themeColor !== '#000000' ? themeColor : null)
})
this.webview.addEventListener('found-in-page', (e) => {
if (this.frame.isEmpty()) {
return
}
if (e.result !== undefined && (e.result.matches !== undefined || e.result.activeMatchOrdinal !== undefined)) {
if (e.result.matches === 0) {
windowActions.setFindDetail(this.frame, Immutable.fromJS({
Expand All @@ -916,6 +998,9 @@ class Frame extends ImmutableComponent {
}
})
this.webview.addEventListener('did-get-response-details', (details) => {
if (this.frame.isEmpty()) {
return
}
windowActions.gotResponseDetails(this.frame.get('tabId'), details)
})
// Handle zoom using Ctrl/Cmd and the mouse wheel.
Expand Down Expand Up @@ -982,7 +1067,9 @@ class Frame extends ImmutableComponent {
}

onFocus () {
windowActions.setTabPageIndexByFrame(this.frame)
if (!this.frame.isEmpty()) {
windowActions.setTabPageIndexByFrame(this.frame)
}

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

0 comments on commit 6923c7f

Please sign in to comment.