Bug 117522 - Make sure we aren't throttling plugin timers during initialisation and destruction
Summary: Make sure we aren't throttling plugin timers during initialisation and destru...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-06-11 14:56 PDT by Oliver Hunt
Modified: 2013-06-11 18:12 PDT (History)
11 users (show)

See Also:


Attachments
Patch (15.44 KB, patch)
2013-06-11 14:59 PDT, Oliver Hunt
no flags Details | Formatted Diff | Diff
Patch (14.43 KB, patch)
2013-06-11 18:01 PDT, Oliver Hunt
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2013-06-11 14:56:34 PDT
Make sure we aren't throttling plugin timers during initialisation and destruction
Comment 1 Oliver Hunt 2013-06-11 14:59:12 PDT
Created attachment 204362 [details]
Patch
Comment 2 kov's GTK+ EWS bot 2013-06-11 16:03:23 PDT
Comment on attachment 204362 [details]
Patch

Attachment 204362 [details] did not pass gtk-wk2-ews (gtk-wk2):
Output: http://webkit-queues.appspot.com/results/789464
Comment 3 Oliver Hunt 2013-06-11 17:01:24 PDT
radar://14103814
Comment 4 Sam Weinig 2013-06-11 17:52:45 PDT
Comment on attachment 204362 [details]
Patch

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

> Source/WebKit2/Shared/ActivityAssertion.cpp:28
> +#include "config.h"
> +
> +#include "ActivityAssertion.h"

You should not have newline here.

> Source/WebKit2/Shared/ActivityAssertion.h:36
> +class ChildProcess;
> +class ActivityAssertion {

You should have a newline here.

> Source/WebKit2/Shared/ActivityAssertion.h:39
> +    WTF_MAKE_NONCOPYABLE(ActivityAssertion);
> +
> +public:

You should not have a newline here.

> Source/WebKit2/Shared/ActivityAssertion.h:43
> +    static PassOwnPtr<ActivityAssertion> create(ChildProcess* process)
> +    {
> +        return adoptPtr(new ActivityAssertion(process));
> +    }

If this is only ever used on the stack, is there any reason to make this use OwnPtr/PassOwnPtr?

> Source/WebKit2/Shared/ActivityAssertion.h:50
> +    ActivityAssertion(ChildProcess*);
> +    ChildProcess* m_process;
> +};

You should have a newline here.

> Source/WebKit2/Shared/ChildProcess.h:82
> +    PassOwnPtr<ActivityAssertion> takeActivityAssertion()
> +    {
> +        return ActivityAssertion::create(this);
> +    }
> +    
> +

Extra newline.
Comment 5 Oliver Hunt 2013-06-11 18:01:23 PDT
Created attachment 204374 [details]
Patch
Comment 6 Darin Adler 2013-06-11 18:03:22 PDT
Comment on attachment 204374 [details]
Patch

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

> Source/WebKit2/Shared/ActivityAssertion.h:31
> +#include <wtf/OwnPtr.h>
> +#include <wtf/PassOwnPtr.h>

Don’t need these any more.
Comment 7 Sam Weinig 2013-06-11 18:04:19 PDT
Comment on attachment 204374 [details]
Patch

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

> Source/WebKit2/PluginProcess/WebProcessConnection.cpp:232
> +    // Ensure we don't clamp any timers during initialisation

"initialisation" is spelled "initialization" in WebKit.

> Source/WebKit2/Shared/ActivityAssertion.h:46
> +private:
> +    ChildProcess& m_process;
> +
> +};

Extra newline.
Comment 8 Oliver Hunt 2013-06-11 18:12:31 PDT
Committed r151480: <http://trac.webkit.org/changeset/151480>