Bug 139611 – Simplify tracking of process suppression disabled for PluginProcessManager

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

[Gavin Barraclough]

Shouldn't need to iterate contexts; just keep a count.

[Gavin Barraclough]

Created attachment 243239 [details]
Preliminary patch, needs Changelog

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/Plugins/PluginProcessManager.cpp:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 6 files


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

[Gavin Barraclough]

Created attachment 243265 [details]
Fix

[WebKit Commit Bot]

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


ERROR: Source/WebKit2/UIProcess/Plugins/PluginProcessManager.cpp:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 7 files


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

[Darin Adler]

Comment on attachment 243265 [details]
Fix

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

> Source/WebKit2/UIProcess/Plugins/PluginProcessManager.h:71
> +    {
> +        return m_processSuppressionDisabledForPageCounter.count();
> +    }

I suggest putting this inline function body outside the class definition. Keeps the class definition more readable.

> Source/WebKit2/UIProcess/Plugins/PluginProcessManager.h:78
> +    {
> +        return m_processSuppressionEnabled;
> +    };

Same suggestion here.

Also, the stray semicolon here is not needed.

> Source/WebKit2/UIProcess/Plugins/PluginProcessManager.h:93
> +    bool m_processSuppressionEnabled;

I suggest using modern initialization syntax here:

    bool m_processSuppressionEnabled { true };

Then this data member doesn’t have to be mentioned in the constructor.

[Gavin Barraclough]

http://trac.webkit.org/changeset/177291

[Carlos Alberto Lopez Perez]

(In reply to comment #6)
> http://trac.webkit.org/changeset/177291

This has broken both the GTK and EFL builds.

The EWS bots are green on the patch attached to this bug, so maybe you did some last-minute change before landing it that broke the build?

The build log are here:

 * GTK build failure: https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Build%29/builds/53626/steps/compile-webkit/logs/stdio
 * EFL build failure: https://build.webkit.org/builders/EFL%20Linux%20ARMv7%20Thumb2%20Release%20%28Build%29/builds/9261/steps/compile-webkit/logs/stdio

[Gavin Barraclough]

(In reply to comment #7)
> (In reply to comment #6)
> > http://trac.webkit.org/changeset/177291
> 
> This has broken both the GTK and EFL builds.
> 
> The EWS bots are green on the patch attached to this bug, so maybe you did
> some last-minute change before landing it that broke the build?
> 
> The build log are here:
> 
>  * GTK build failure:
> https://build.webkit.org/builders/GTK%20Linux%2064-
> bit%20Release%20%28Build%29/builds/53626/steps/compile-webkit/logs/stdio
>  * EFL build failure:
> https://build.webkit.org/builders/
> EFL%20Linux%20ARMv7%20Thumb2%20Release%20%28Build%29/builds/9261/steps/
> compile-webkit/logs/stdio

Ooops, yes – in addressing review comments moved inline functions out the the class declarations, I should have #ifdef'ed.

Fix incoming.

[Gavin Barraclough]

gtk/efl should be fixed in:

Transmitting file data ..
Committed revision 177307.

[Carlos Alberto Lopez Perez]

(In reply to comment #9)
> gtk/efl should be fixed in:
> 
> Transmitting file data ..
> Committed revision 177307.

That worked, thanks :)

(The EFL build is still broken, but don't seems related with this)