Bug 33857 – [Qt] Need a public API for suspending/resuming DOM Objects on a page

• Component: WebKit Qt
• Status: RESOLVED WONTFIX

[Jesus Sanchez-Palencia]

Qt need an API for suspending/resuming DOM Objects on a QWebPage.
We are defining DOM Objects as JS timers, animated GIFs, SVG animations, CSS animations and plugins.

One example of use-case is when we want to save battery consumption if a mobile browser is running on background, but other use-cases were raised during previous discussion.

This is an extension of https://bugs.webkit.org/show_bug.cgi?id=31673 , and is related to http://bugreports.qt.nokia.com/browse/QTWEBKIT-6 and http://bugreports.qt.nokia.com/browse/QTWEBKIT-5 .

[Jesus Sanchez-Palencia]

Created attachment 46941 [details]
Proposed API for QWebFrame and QWebPage, and manual-tests

[Jesus Sanchez-Palencia]

Created attachment 46942 [details]
Add an option to QtLauncher to be easy to suspend/resume DOM Objects on the current page

[Eric Seidel (no email)]

Comment on attachment 46941 [details]
Proposed API for QWebFrame and QWebPage, and manual-tests

Do we have license to distribute Elvis_with_parachute?

[WebKit Review Bot]

Attachment 46942 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/198987

[Jesus Sanchez-Palencia]

(In reply to comment #3)
>Do we have license to distribute Elvis_with_parachute?

The place I took it said that it was "free for using" and that you should contact the author for any specific questions about license. I just can't find the author, so it's probably better to use another GIF from our resources. I'll fix this.


(In reply to comment #4)
> Attachment 46942 [details] did not build on qt:
> Build output: http://webkit-commit-queue.appspot.com/results/198987

Attachment 46942 [details] depends on attachment 46941 [details].

[Kenneth Rohde Christiansen]

First of all it is called Qt and not QT. QT == QuickTime

[Kenneth Rohde Christiansen]

Comment on attachment 46941 [details]
Proposed API for QWebFrame and QWebPage, and manual-tests


> 
> The API from QWebPage will call suspend/resume DOM Objects of the main frame
> and of all its children.


Add API to QWebPage so that it is possible to suspend/resume...


> * manual-tests/frames-multiple-active-dom-objects.html: Added.
> * manual-tests/resources/Elvis_with_parachute.gif: Added.

What is the license of this file?


> +// FIXME: add support to suspending/resuming of animated GIFs and plugins.
> +// Suspend DOM objects (JS timers, CSS animation, SVG animation) in this frame.
> +bool QWebFramePrivate::suspendDOMObjects()
> +{
> +    if (!frame)
> +        return false;
> +
> +    AnimationController* controller = frame->animation();
> +    if (!controller)
> +        return false;

This seems wrong... what if you dont have a controller, can't you still suspend the active DOM objects included by
doc->suspendActiveDOMObjects(); ?

> +
> +    Document* doc = frame->document();
> +    if (!doc)
> +        return false;
> +
> +    // JavaScript
> +    doc->suspendActiveDOMObjects();

> +bool QWebFramePrivate::resumeDOMObjects()

same thing here

> +    QList<QWebFrame *> subFrames = frame->childFrames();

Should be  QList<QWebFrame*>. childFrames is probably a better name than subFrames :-) and it is already used

> +    QList<QWebFrame *>::const_iterator i;
> +    for (i = subFrames.constBegin(); i != subFrames.constEnd() ; ++i) {
> +        b = (b && suspendChildFrames((*i)));
> +    }
> +

Don't use b, use a descriptive name.

> +bool QWebPagePrivate::resumeChildFrames(QWebFrame *frame)

Wrong coding style, QWebFrame* frame

> +{
> +    if (!frame)
> +        return false;
> +
> +    bool b = frame->resumeDOMObjects();
> +

Don't use b

> +    QList<QWebFrame *> subFrames = frame->childFrames();
> +    QList<QWebFrame *>::const_iterator i;

Coding style

> +/*!
> +    Resume all frames, starting at the main frame.
> +    DOM Objects (JS timers, CSS and SVG animation) will be resumed.

DOM objects that were suspended, will be resumed.

> +    bool suspendChildFrames(QWebFrame *frame);
> +    bool resumeChildFrames(QWebFrame *frame);

Coding style

[Kenneth Rohde Christiansen]

Comment on attachment 46942 [details]
Add an option to QtLauncher to be easy to suspend/resume DOM Objects on the current page

> From debee88b204b8faf48edca5bca37b6b25e5a26cc Mon Sep 17 00:00:00 2001
> From: Jesus Sanchez-Palencia <jesferna@darjeeling.(none)>
> Date: Tue, 19 Jan 2010 17:17:45 -0300
> Subject: [PATCH 2/2] Add suspend/resume page option to QtLauncher
> 
> Reviewed by NOBODY (OOPS!).
> 
> [QT] Need a public API for suspending/resuming DOM Objects on a page
> https://bugs.webkit.org/show_bug.cgi?id=33857

It is called Qt


> +        suspendDOMObjects = false;

Doesn't the launcher uses m_ for member fields?

> +            QWebPage* page = view->page();
> +            page->suspend();

Why not call it directly?

> +    void toggleSuspendResumeObjects(bool on)

That doesnt seem much like a toggle if it takes a bool argument.

[Jesus Sanchez-Palencia]

Created attachment 47026 [details]
[v2] Proposed API and manual-tests

[Jesus Sanchez-Palencia]

Created attachment 47027 [details]
[v2] Add a suspend/resume option to QtLauncher

This patch depends on the other attachment (47026).

[WebKit Review Bot]

Attachment 47027 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/204430

[Simon Hausmann]

Comment on attachment 47026 [details]
[v2] Proposed API and manual-tests

Clearing review for the time being, given that we're waiting for more feedback on the requirement at http://bugreports.qt.nokia.com/browse/QTWEBKIT-5

[Simon Hausmann]

Comment on attachment 47027 [details]
[v2] Add a suspend/resume option to QtLauncher

Clearning review here, too

[Tor Arne Vestbø]

Please follow the QtWebKit bug reporting guidelines when reporting bugs.

See http://trac.webkit.org/wiki/QtWebKitBugs

Specifically:

  - The 'QtWebKit' component should only be used for bugs/features in the
    public QtWebKit API layer, not to signify that the bug is specific to
    the Qt port of WebKit

      http://trac.webkit.org/wiki/QtWebKitBugs#Component

  - Add the keyword 'Qt' to signal that it's a Qt-related bug

      http://trac.webkit.org/wiki/QtWebKitBugs#Keywords

[Kent Hansen]

I think Jesus needs a reply at http://bugreports.qt.nokia.com/browse/QTWEBKIT-5 in order for this to proceed.

[Jesus Sanchez-Palencia]

Closing this bug as a long time ago it was decided this API wasn't needed.