Bug 187050 – Remove quarantine for Webex plugin

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

Remove quarantine for Webex plugin

[youenn fablet]

Created attachment 343613 [details]
Patch

[Brent Fulgham]

Comment on attachment 343613 [details]
Patch

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

r=me

> Source/WebKit/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=187050

Radar, please! :-)

[Radar WebKit Bug Importer]

<rdar://problem/41478189>

[youenn fablet]

Created attachment 343662 [details]
Patch for landing

[youenn fablet]

(In reply to Brent Fulgham from comment #2)
> Comment on attachment 343613 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=343613&action=review
> 
> r=me
> 
> > Source/WebKit/ChangeLog:4
> > +        https://bugs.webkit.org/show_bug.cgi?id=187050
> 
> Radar, please! :-)

Thanks for the review, added radar link.

[WebKit Commit Bot]

Comment on attachment 343662 [details]
Patch for landing

Clearing flags on attachment: 343662

Committed r233232: <https://trac.webkit.org/changeset/233232>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 187081

[youenn fablet]

Created attachment 344237 [details]
Using qtn_proc_init if qtn_proc_init_with_self fails

[EWS Watchlist]

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


ERROR: Source/WebKit/Platform/spi/mac/QuarantineSPI.h:54:  qtn_proc_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 7 files


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

[Brent Fulgham]

Comment on attachment 344237 [details]
Using qtn_proc_init if qtn_proc_init_with_self fails

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

> Source/WebKit/PluginProcess/PluginProcess.h:78
> +    bool shouldOverrideQuarantine() final;

I feel like this would be better named "shouldEnableQuarantine()" rather than override. This really is the default case (we only "Don't Override" for WebEx).

> Source/WebKit/PluginProcess/mac/PluginProcessMac.mm:697
> +    return m_pluginBundleIdentifier != "com.cisco.webex.plugin.gpc64";

Is there a 32-bit version we need to worry about?

[youenn fablet]

(In reply to Brent Fulgham from comment #11)
> Comment on attachment 344237 [details]
> Using qtn_proc_init if qtn_proc_init_with_self fails
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=344237&action=review
> 
> > Source/WebKit/PluginProcess/PluginProcess.h:78
> > +    bool shouldOverrideQuarantine() final;
> 
> I feel like this would be better named "shouldEnableQuarantine()" rather
> than override. This really is the default case (we only "Don't Override" for
> WebEx).

I was initially ok with the renaming but looking further in the code, overriding seems the right term.
There is a default value in Info.plist for WebContent/Network/Storage/Plugin processes and we sometimes override it.
Given the current Info.plist, we do not really need to 'override' the value fo WebContent/Network/Storage processes since LSFileQuarantineEnabled is already set to true for all of them.

> > Source/WebKit/PluginProcess/mac/PluginProcessMac.mm:697
> > +    return m_pluginBundleIdentifier != "com.cisco.webex.plugin.gpc64";
> 
> Is there a 32-bit version we need to worry about?

I only encountered this one so we should be good there.

[WebKit Commit Bot]

Comment on attachment 344237 [details]
Using qtn_proc_init if qtn_proc_init_with_self fails

Clearing flags on attachment: 344237

Committed r233497: <https://trac.webkit.org/changeset/233497>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.