Bug 145663

Summary: [Content Extensions] Make max rule count
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: WebCore Misc.Assignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 145871    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Alex Christensen 2015-06-04 14:49:43 PDT
This introduces a max rule count, and it makes the limits on rules and NFA size NSUserDefaults, with default fallback values.
Comment 1 Alex Christensen 2015-06-04 14:52:52 PDT
Created attachment 254304 [details]
Patch
Comment 2 Benjamin Poulain 2015-06-04 16:17:02 PDT
Comment on attachment 254304 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=254304&action=review

> Source/WebCore/contentextensions/cocoa/ContentExtensionDefaultsCocoa.mm:37
> +    NSInteger maxNFASize = [[NSUserDefaults standardUserDefaults] integerForKey:@"ContentExtensionMaxNFASize"];

Shouldn't you use WebKit preferences that are populated by UserDefaults?

Having NSUserDefaults in here is weird
Comment 3 Alex Christensen 2015-06-04 17:11:45 PDT
(In reply to comment #2)
> Shouldn't you use WebKit preferences that are populated by UserDefaults?
Could you point me to code that does something similar?  Would we be able to change/manage that like we want to?
Comment 4 Alex Christensen 2015-06-05 15:26:08 PDT
rdar://problem/21242407
Comment 5 Alex Christensen 2015-06-10 17:46:43 PDT
Created attachment 254694 [details]
Patch
Comment 6 Benjamin Poulain 2015-06-10 17:49:04 PDT
Comment on attachment 254694 [details]
Patch

Can you report the number with the error message?
Can you tweak the number based on the device?
Can you also limit the number of machines we generate?
Comment 7 Alex Christensen 2015-06-10 17:54:18 PDT
http://trac.webkit.org/changeset/185442

I forgot to change the tools change log.  This just added the max rule count.
Comment 8 Alex Christensen 2015-06-10 17:54:43 PDT
(In reply to comment #6)
> Comment on attachment 254694 [details]
> Patch
> 
> Can you report the number with the error message?
> Can you tweak the number based on the device?
> Can you also limit the number of machines we generate?
These could be added later.
Comment 9 WebKit Commit Bot 2015-06-10 21:12:54 PDT
Re-opened since this is blocked by bug 145871
Comment 10 Alex Christensen 2015-06-10 21:15:35 PDT
This caused lots of strange errors in WTF.StringOperators tests.  I'm not sure why, but I'm reverting for now.
https://build.webkit.org/builders/Apple%20Yosemite%20Release%20WK1%20%28Tests%29/builds/5975/steps/run-api-tests/logs/stdio
Comment 11 Alex Christensen 2015-06-10 21:15:58 PDT
Comment on attachment 254694 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=254694&action=review

> Tools/ChangeLog:3
> +        [Content Extensions] Make max NFA size and max rule count user defaults.

And I need to update this ChangeLog entry.
Comment 12 Alex Christensen 2015-06-15 09:35:08 PDT
Created attachment 254878 [details]
Patch
Comment 13 WebKit Commit Bot 2015-06-15 10:23:39 PDT
Comment on attachment 254878 [details]
Patch

Clearing flags on attachment: 254878

Committed r185555: <http://trac.webkit.org/changeset/185555>
Comment 14 WebKit Commit Bot 2015-06-15 10:23:42 PDT
All reviewed patches have been landed.  Closing bug.