Bug 145663 - [Content Extensions] Make max rule count
Summary: [Content Extensions] Make max rule count
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on: 145871
Blocks:
  Show dependency treegraph
 
Reported: 2015-06-04 14:49 PDT by Alex Christensen
Modified: 2015-06-15 10:23 PDT (History)
1 user (show)

See Also:


Attachments
Patch (17.41 KB, patch)
2015-06-04 14:52 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (5.50 KB, patch)
2015-06-10 17:46 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (5.46 KB, patch)
2015-06-15 09:35 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.