perf(filtering): avoid caching invalid WebKit rules
- commit
- 908e247b71ba8dd77e85244a008e05da6e15f3f6
- author
- bnema · · 2025-12-06 02:49 UTC
- parent
- 8ab8046
- signature
- unsigned
2 files · +14 -7
M internal/filtering/converter/easylist_to_webkit.go +5 -3
| @@ -150,10 +150,12 @@ func (fc *FilterConverter) parseNetworkFilter(line string) error { | |||
| 150 | 150 | ifDomains = append(ifDomains, d) | |
| 151 | 151 | } | |
| 152 | 152 | } | |
| 153 | - | if len(ifDomains) > 0 { | |
| 153 | + | // WebKit only allows ONE of: if-domain, unless-domain, if-top-url, unless-top-url | |
| 154 | + | // Prefer if-domain when both exist (more common use case) | |
| 155 | + | switch { | |
| 156 | + | case len(ifDomains) > 0: | |
| 154 | 157 | rule.Trigger.IfDomain = ifDomains | |
| 155 | - | } | |
| 156 | - | if len(unlessDomains) > 0 { | |
| 158 | + | case len(unlessDomains) > 0: | |
| 157 | 159 | rule.Trigger.UnlessDomain = unlessDomains | |
| 158 | 160 | } | |
| 159 | 161 | case opt == "script": | |
| @@ -150,10 +150,12 @@ func (fc *FilterConverter) parseNetworkFilter(line string) error { | |||||
| 150 | ifDomains = append(ifDomains, d) | 150 | ifDomains = append(ifDomains, d) | ||
| 151 | } | 151 | } | ||
| 152 | } | 152 | } | ||
| 153 | - | if len(ifDomains) > 0 { | 153 | + | // WebKit only allows ONE of: if-domain, unless-domain, if-top-url, unless-top-url |
| 154 | + | // Prefer if-domain when both exist (more common use case) | |||
| 155 | + | switch { | |||
| 156 | + | case len(ifDomains) > 0: | |||
| 154 | rule.Trigger.IfDomain = ifDomains | 157 | rule.Trigger.IfDomain = ifDomains | ||
| 155 | - | } | 158 | + | case len(unlessDomains) > 0: |
| 156 | - | if len(unlessDomains) > 0 { | |||
| 157 | rule.Trigger.UnlessDomain = unlessDomains | 159 | rule.Trigger.UnlessDomain = unlessDomains | ||
| 158 | } | 160 | } | ||
| 159 | case opt == "script": | 161 | case opt == "script": | ||
M pkg/webkit/content_blocking.go +9 -4
| @@ -65,6 +65,7 @@ func (cbm *ContentBlockingManager) CompileFilters(identifier string, jsonRules [ | |||
| 65 | 65 | filter, err := cbm.filterStore.SaveFinish(result) | |
| 66 | 66 | if err != nil { | |
| 67 | 67 | logging.Error(fmt.Sprintf("[webkit] Failed to compile content filter: %v", err)) | |
| 68 | + | // Don't store nil filter - prevents GC from trying to unref it | |
| 68 | 69 | if onComplete != nil { | |
| 69 | 70 | onComplete(fmt.Errorf("failed to compile filter: %w", err)) | |
| 70 | 71 | } | |
| @@ -79,7 +80,7 @@ func (cbm *ContentBlockingManager) CompileFilters(identifier string, jsonRules [ | |||
| 79 | 80 | return | |
| 80 | 81 | } | |
| 81 | 82 | ||
| 82 | - | // Cache the compiled filter | |
| 83 | + | // Cache the compiled filter (only if valid) | |
| 83 | 84 | cbm.mu.Lock() | |
| 84 | 85 | cbm.compiledFilter = filter | |
| 85 | 86 | cbm.mu.Unlock() | |
| @@ -153,6 +154,8 @@ func (cbm *ContentBlockingManager) ApplyFiltersFromJSON(ucm *webkit.UserContentM | |||
| 153 | 154 | if err != nil { | |
| 154 | 155 | logging.Error(fmt.Sprintf("[webkit] Failed to compile content filter: %v", err)) | |
| 155 | 156 | done <- fmt.Errorf("failed to compile filter: %w", err) | |
| 157 | + | // Explicitly nil to prevent GC issues with partial objects | |
| 158 | + | filter = nil | |
| 156 | 159 | return | |
| 157 | 160 | } | |
| 158 | 161 | ||
| @@ -162,15 +165,17 @@ func (cbm *ContentBlockingManager) ApplyFiltersFromJSON(ucm *webkit.UserContentM | |||
| 162 | 165 | return | |
| 163 | 166 | } | |
| 164 | 167 | ||
| 165 | - | // Cache for future use | |
| 168 | + | // Cache for future use (only valid filters) | |
| 166 | 169 | cbm.mu.Lock() | |
| 167 | 170 | cbm.compiledFilter = filter | |
| 168 | 171 | cbm.filterID = identifier | |
| 169 | 172 | cbm.mu.Unlock() | |
| 170 | 173 | ||
| 171 | 174 | RunOnMainThread(func() { | |
| 172 | - | ucm.AddFilter(filter) | |
| 173 | - | logging.Debug(fmt.Sprintf("[webkit] Successfully added content filter: %s", identifier)) | |
| 175 | + | if filter != nil { | |
| 176 | + | ucm.AddFilter(filter) | |
| 177 | + | logging.Debug(fmt.Sprintf("[webkit] Successfully added content filter: %s", identifier)) | |
| 178 | + | } | |
| 174 | 179 | done <- nil | |
| 175 | 180 | }) | |
| 176 | 181 | }) | |
| @@ -65,6 +65,7 @@ func (cbm *ContentBlockingManager) CompileFilters(identifier string, jsonRules [ | |||||
| 65 | filter, err := cbm.filterStore.SaveFinish(result) | 65 | filter, err := cbm.filterStore.SaveFinish(result) | ||
| 66 | if err != nil { | 66 | if err != nil { | ||
| 67 | logging.Error(fmt.Sprintf("[webkit] Failed to compile content filter: %v", err)) | 67 | logging.Error(fmt.Sprintf("[webkit] Failed to compile content filter: %v", err)) | ||
| 68 | + | // Don't store nil filter - prevents GC from trying to unref it | |||
| 68 | if onComplete != nil { | 69 | if onComplete != nil { | ||
| 69 | onComplete(fmt.Errorf("failed to compile filter: %w", err)) | 70 | onComplete(fmt.Errorf("failed to compile filter: %w", err)) | ||
| 70 | } | 71 | } | ||
| @@ -79,7 +80,7 @@ func (cbm *ContentBlockingManager) CompileFilters(identifier string, jsonRules [ | |||||
| 79 | return | 80 | return | ||
| 80 | } | 81 | } | ||
| 81 | 82 | ||||
| 82 | - | // Cache the compiled filter | 83 | + | // Cache the compiled filter (only if valid) |
| 83 | cbm.mu.Lock() | 84 | cbm.mu.Lock() | ||
| 84 | cbm.compiledFilter = filter | 85 | cbm.compiledFilter = filter | ||
| 85 | cbm.mu.Unlock() | 86 | cbm.mu.Unlock() | ||
| @@ -153,6 +154,8 @@ func (cbm *ContentBlockingManager) ApplyFiltersFromJSON(ucm *webkit.UserContentM | |||||
| 153 | if err != nil { | 154 | if err != nil { | ||
| 154 | logging.Error(fmt.Sprintf("[webkit] Failed to compile content filter: %v", err)) | 155 | logging.Error(fmt.Sprintf("[webkit] Failed to compile content filter: %v", err)) | ||
| 155 | done <- fmt.Errorf("failed to compile filter: %w", err) | 156 | done <- fmt.Errorf("failed to compile filter: %w", err) | ||
| 157 | + | // Explicitly nil to prevent GC issues with partial objects | |||
| 158 | + | filter = nil | |||
| 156 | return | 159 | return | ||
| 157 | } | 160 | } | ||
| 158 | 161 | ||||
| @@ -162,15 +165,17 @@ func (cbm *ContentBlockingManager) ApplyFiltersFromJSON(ucm *webkit.UserContentM | |||||
| 162 | return | 165 | return | ||
| 163 | } | 166 | } | ||
| 164 | 167 | ||||
| 165 | - | // Cache for future use | 168 | + | // Cache for future use (only valid filters) |
| 166 | cbm.mu.Lock() | 169 | cbm.mu.Lock() | ||
| 167 | cbm.compiledFilter = filter | 170 | cbm.compiledFilter = filter | ||
| 168 | cbm.filterID = identifier | 171 | cbm.filterID = identifier | ||
| 169 | cbm.mu.Unlock() | 172 | cbm.mu.Unlock() | ||
| 170 | 173 | ||||
| 171 | RunOnMainThread(func() { | 174 | RunOnMainThread(func() { | ||
| 172 | - | ucm.AddFilter(filter) | 175 | + | if filter != nil { |
| 173 | - | logging.Debug(fmt.Sprintf("[webkit] Successfully added content filter: %s", identifier)) | 176 | + | ucm.AddFilter(filter) |
| 177 | + | logging.Debug(fmt.Sprintf("[webkit] Successfully added content filter: %s", identifier)) | |||
| 178 | + | } | |||
| 174 | done <- nil | 179 | done <- nil | ||
| 175 | }) | 180 | }) | ||
| 176 | }) | 181 | }) | ||