Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement cosmetic filtering #3303

Merged
merged 9 commits into from Dec 14, 2019

work around browser-internal race condition

Occasionally, chrome.tabs.insertCSS fails when triggered from
chrome.webNavigation events. It cannot work unless some page content is
already loaded.

See https://bugs.chromium.org/p/chromium/issues/detail?id=331654#c15
  • Loading branch information
antonok-edm committed Aug 27, 2019
commit c45be3c54666474824e3d690d217656a5c612a14
@@ -137,3 +137,11 @@ export const shieldsReady: actions.ShieldsReady = () => {
type: types.SHIELDS_READY
}
}

export const contentScriptsLoaded: actions.ContentScriptsLoaded = (tabId: number, url: string) => {
return {
type: types.CONTENT_SCRIPTS_LOADED,
tabId,
url
}
}
@@ -39,7 +39,7 @@ export const removeSiteFilter = (origin: string) => {
}

export const applyAdblockCosmeticFilters = (tabId: number, hostname: string) => {
chrome.braveShields.hostnameCosmeticResources(hostname, (resources) => {
chrome.braveShields.hostnameCosmeticResources(hostname, async (resources) => {
if (chrome.runtime.lastError) {
console.warn('Unable to get cosmetic filter data for the current host')
return
@@ -4,6 +4,7 @@ import {
removeSiteFilter,
removeAllFilters
} from '../api/cosmeticFilterAPI'
import shieldsPanelActions from '../actions/shieldsPanelActions'

export let rule = {
host: '',
@@ -59,6 +60,21 @@ chrome.runtime.onMessage.addListener((msg, sender, sendResponse) => {
})

This comment has been minimized.

Copy link
@petemill

petemill Aug 30, 2019

Member

) should be on separate line since { is separate line to (

This comment has been minimized.

Copy link
@antonok-edm

antonok-edm Sep 3, 2019

Author Collaborator

Hmm, not sure what you mean? The first }) matches to ...insertCSS({, the second matches to ...Stylesheet(... => {.

break
}
case 'contentScriptsLoaded': {
const tab = sender.tab
if (tab === undefined) {
break
}
const tabId = tab.id
if (tabId === undefined) {
break
}
const url = tab.url
if (url === undefined) {
break
}
shieldsPanelActions.contentScriptsLoaded(tabId, url)
}
}
})

@@ -61,7 +61,6 @@ export default function shieldsPanelReducer (
state = shieldsPanelState.resetBlockingResources(state, action.tabId)
state = noScriptState.resetNoScriptInfo(state, action.tabId, new window.URL(action.url).origin)
}
applyAdblockCosmeticFilters(action.tabId, getHostname(action.url))
applyCSSCosmeticFilters(action.tabId, getHostname(action.url))
break
}
@@ -344,6 +343,9 @@ export default function shieldsPanelReducer (
})
break
}
case shieldsPanelTypes.CONTENT_SCRIPTS_LOADED: {
applyAdblockCosmeticFilters(action.tabId, getHostname(action.url))
}
}

if (!areObjectsEqual(state.persistentData, initialPersistentData)) {
@@ -20,3 +20,4 @@ export const SET_FINAL_SCRIPTS_BLOCKED_ONCE_STATE = 'SET_FINAL_SCRIPTS_BLOCKED_O
export const SET_ADVANCED_VIEW_FIRST_ACCESS = 'SET_ADVANCED_VIEW_FIRST_ACCESS'
export const TOGGLE_ADVANCED_VIEW = 'TOGGLE_ADVANCED_VIEW'
export const SHIELDS_READY = 'SHIELDS_READY'
export const CONTENT_SCRIPTS_LOADED = 'CONTENT_SCRIPTS_LOADED'
@@ -1,3 +1,13 @@
// Notify the background script as soon as the content script has loaded.
// chrome.tabs.insertCSS may sometimes fail to inject CSS in a newly navigated
// page when using the chrome.webNavigation API.
// See: https://bugs.chromium.org/p/chromium/issues/detail?id=331654#c15
// The RenderView should always be ready when the content script begins, so
// this message is used to trigger CSS insertion instead.
chrome.runtime.sendMessage({
type: 'contentScriptsLoaded'
})

const unique = require('unique-selector').default

let target: EventTarget | null
@@ -172,6 +172,16 @@ export interface ShieldsReady {
(): ShieldsReadyReturn
}

interface ContentScriptsLoadedReturn {
type: types.CONTENT_SCRIPTS_LOADED,
tabId: number,
url: string,
}

export interface ContentScriptsLoaded {
(tabId: number, url: string): ContentScriptsLoadedReturn
}

export type shieldPanelActions =
ShieldsPanelDataUpdatedReturn |
ShieldsToggledReturn |
@@ -189,4 +199,5 @@ export type shieldPanelActions =
SetAllScriptsBlockedCurrentStateReturn |
SetFinalScriptsBlockedStateReturn |
SetAdvancedViewFirstAccessReturn |
ShieldsReadyReturn
ShieldsReadyReturn |
ContentScriptsLoadedReturn
@@ -22,3 +22,4 @@ export type SET_FINAL_SCRIPTS_BLOCKED_ONCE_STATE = typeof types.SET_FINAL_SCRIPT
export type SET_ADVANCED_VIEW_FIRST_ACCESS = typeof types.SET_ADVANCED_VIEW_FIRST_ACCESS
export type TOGGLE_ADVANCED_VIEW = typeof types.TOGGLE_ADVANCED_VIEW
export type SHIELDS_READY = typeof types.SHIELDS_READY
export type CONTENT_SCRIPTS_LOADED = typeof types.CONTENT_SCRIPTS_LOADED
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.