Bug 125871 – Allow partial application of PlugInAutoStart tables based on timestamp

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Ricky Mondello]

Allow clients of the PlugInAutoStart functionality to partially apply a table based on the timestamp of entries. More specifically, clients may find it useful to only apply policies created before a certain date to “roll back” changes to the table.

[Ricky Mondello]

Created attachment 219465 [details]
First take

[WebKit Commit Bot]

Attachment 219465 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/C/WKContext.cpp', u'Source/WebKit2/UIProcess/API/C/WKContext.h', u'Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.cpp', u'Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.h', u'Source/WebKit2/UIProcess/WebContext.cpp', u'Source/WebKit2/UIProcess/WebContext.h', '--commit-queue']" exit_code: 1
ERROR: Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.cpp:109:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.cpp:116:  Missing space before {  [whitespace/braces] [5]
ERROR: Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.cpp:121:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.h:59:  The parameter name "table" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.h:68:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/UIProcess/WebContext.h:270:  The parameter name "dictionary" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 6 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ricky Mondello]

<rdar://problem/15684849>

[Ricky Mondello]

Created attachment 219466 [details]
Take 2, with style corrections

[WebKit Commit Bot]

Attachment 219466 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/C/WKContext.cpp', u'Source/WebKit2/UIProcess/API/C/WKContext.h', u'Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.cpp', u'Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.h', u'Source/WebKit2/UIProcess/WebContext.cpp', u'Source/WebKit2/UIProcess/WebContext.h', '--commit-queue']" exit_code: 1
ERROR: Source/WebKit2/UIProcess/WebContext.h:270:  The parameter name "dictionary" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ricky Mondello]

Created attachment 219469 [details]
More style corrections

[Jon Lee]

Comment on attachment 219469 [details]
More style corrections

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

> Source/WebKit2/ChangeLog:18
> +            with a lambda that lets policies created after a certain time pass.

errrr-- "before time" in the function name, but let policies pass "after a certain time"?

> Source/WebKit2/UIProcess/API/C/WKContext.cpp:394
> +void WKContextApplyPlugInAutoStartOriginsAddedBeforeTime(WKContextRef contextRef, WKDictionaryRef dictionaryRef, double time)

"Apply" seems like the wrong verb to use here. The method isn't taking the provided dictionary and amended the existing one with it, which is what I think of when I see "apply". Maybe something more along the lines of "SetPlugInAutoStartOrigins{BeforeTime,UsingTimeCutoff}"?

> Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.cpp:52
> +static double timeExpirationTimeWasGenerated(double expirationTime)

awkward naming. "originationTimeFromExpiration"?

> Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.cpp:121
> +void PlugInAutoStartProvider::setAutoStartOriginsTableWithItemsPassingTest(ImmutableDictionary const& table, std::function<bool(double timestamp)> predicate)

Wouldn't it be better for the predicate to be named properly, for readability in the code? Like "isExpirationTimeAcceptable"?
Also seems that proper style is "const ImmutableDictionary&".

[Ricky Mondello]

Created attachment 219664 [details]
Addressing Jon Lee’s feedback

[WebKit Commit Bot]

Attachment 219664 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/C/WKContext.cpp', u'Source/WebKit2/UIProcess/API/C/WKContext.h', u'Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.cpp', u'Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.h', u'Source/WebKit2/UIProcess/WebContext.cpp', u'Source/WebKit2/UIProcess/WebContext.h', '--commit-queue']" exit_code: 1
ERROR: Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.h:68:  The parameter name "table" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ricky Mondello]

Created attachment 219665 [details]
Addressing Jon Lee’s feedback (2)

[Ricky Mondello]

Created attachment 219779 [details]
Naming cleanup

[WebKit Commit Bot]

Comment on attachment 219779 [details]
Naming cleanup

Clearing flags on attachment: 219779

Committed r160922: <http://trac.webkit.org/changeset/160922>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Jon Lee]

Comment on attachment 219779 [details]
Naming cleanup

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

> Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.cpp:109
> +void PlugInAutoStartProvider::setAutoStartOriginsFilteringOutEntriesAddedBeforeTime(ImmutableDictionary& table, double time)

Isn't it FilteringInEntriesAddedBeforeTime, or FilteringOutEntriesAddedAfterTime?

[Ricky Mondello]

Comment on attachment 219779 [details]
Naming cleanup

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

>> Source/WebKit2/UIProcess/Plugins/PlugInAutoStartProvider.cpp:109
>> +void PlugInAutoStartProvider::setAutoStartOriginsFilteringOutEntriesAddedBeforeTime(ImmutableDictionary& table, double time)
> 
> Isn't it FilteringInEntriesAddedBeforeTime, or FilteringOutEntriesAddedAfterTime?

Yes. New bug/patch coming. Thank you for catching this.

[Ricky Mondello]

Rename PlugInAutoStartProvider’s ...EntriesAddedBeforeTime facility to ...EntriesAddedAfterTime
https://bugs.webkit.org/show_bug.cgi?id=126078