Bug 102239

Summary: Web Inspector: keep track of mutation observers and disconnect them upon unload
Product: WebKit Reporter: Pavel Feldman <pfeldman>
Component: Web Inspector (Deprecated)Assignee: Pavel Feldman <pfeldman>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, dglazkov, eustas.bug, keishi, loislo, ossy, pfeldman, pmuellr, vsevik, web-inspector-bugs, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch vsevik: review+, webkit.review.bot: commit-queue-

Description Pavel Feldman 2012-11-14 08:22:59 PST
Otherwise we hit memory leaks.
Comment 1 Pavel Feldman 2012-11-14 08:28:12 PST
Created attachment 174167 [details]
Patch
Comment 2 WebKit Review Bot 2012-11-14 09:09:02 PST
Comment on attachment 174167 [details]
Patch

Attachment 174167 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14834373

New failing tests:
http/tests/inspector/appcache/appcache-iframe-manifests.html
http/tests/inspector/injected-script-for-origin.html
http/tests/inspector/extensions-network-redirect.html
http/tests/inspector/change-iframe-src.html
http/tests/inspector/modify-cross-domain-rule.html
http/tests/inspector/resource-har-conversion.html
http/tests/inspector/console-resource-errors.html
http/tests/inspector/filesystem/delete-entry.html
http/tests/inspector/extensions-headers.html
http/tests/inspector-enabled/injected-script-discard.html
http/tests/inspector/console-cross-origin-iframe-logging.html
http/tests/inspector/appcache/appcache-swap.html
http/tests/inspector/extensions-useragent.html
http/tests/inspector-enabled/dynamic-scripts.html
http/tests/inspector/console-cd-completions.html
http/tests/inspector/compiler-script-mapping.html
http/tests/inspector/compiler-source-mapping-debug.html
http/tests/inspector/filesystem/directory-tree.html
http/tests/inspector/web-socket-frame-error.html
http/tests/inspector/console-xhr-logging.html
http/tests/inspector/console-xhr-logging-async.html
http/tests/inspector/console-cd.html
http/tests/inspector/resource-har-headers.html
http/tests/inspector/network-preflight-options.html
http/tests/inspector/inspect-iframe-from-different-domain.html
http/tests/inspector-enabled/console-log-before-frame-navigation.html
http/tests/inspector-enabled/dom-storage-open.html
http/tests/inspector/resource-parameters.html
http/tests/inspector-enabled/database-open.html
http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
Comment 3 Pavel Feldman 2012-11-14 09:09:18 PST
Created attachment 174175 [details]
Patch
Comment 4 WebKit Review Bot 2012-11-14 09:30:26 PST
Comment on attachment 174175 [details]
Patch

Attachment 174175 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14833351

New failing tests:
http/tests/inspector/appcache/appcache-iframe-manifests.html
http/tests/inspector/injected-script-for-origin.html
http/tests/inspector/extensions-network-redirect.html
http/tests/inspector/change-iframe-src.html
http/tests/inspector/modify-cross-domain-rule.html
http/tests/inspector/resource-har-conversion.html
http/tests/inspector/console-resource-errors.html
http/tests/inspector/filesystem/delete-entry.html
http/tests/inspector/extensions-headers.html
http/tests/inspector-enabled/injected-script-discard.html
http/tests/inspector/console-cross-origin-iframe-logging.html
http/tests/inspector/appcache/appcache-swap.html
http/tests/inspector/extensions-useragent.html
http/tests/inspector-enabled/dynamic-scripts.html
http/tests/inspector/console-cd-completions.html
http/tests/inspector/compiler-script-mapping.html
http/tests/inspector/compiler-source-mapping-debug.html
http/tests/inspector/filesystem/directory-tree.html
http/tests/inspector/web-socket-frame-error.html
http/tests/inspector/console-xhr-logging.html
http/tests/inspector/console-xhr-logging-async.html
http/tests/inspector/console-cd.html
http/tests/inspector/resource-har-headers.html
http/tests/inspector/network-preflight-options.html
http/tests/inspector/inspect-iframe-from-different-domain.html
http/tests/inspector-enabled/console-log-before-frame-navigation.html
http/tests/inspector-enabled/dom-storage-open.html
http/tests/inspector/resource-parameters.html
http/tests/inspector-enabled/database-open.html
http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
Comment 5 Pavel Feldman 2012-11-14 10:49:34 PST
Committed r134627: <http://trac.webkit.org/changeset/134627>
Comment 6 Csaba Osztrogonác 2012-11-14 13:08:50 PST
(In reply to comment #5)
> Committed r134627: <http://trac.webkit.org/changeset/134627>

It broke all inspector test on Qt - http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Release/r134629%20%2845085%29/results.html

+main frame - has 1 onunload handler(s)

Could you check it please?
Comment 7 Pavel Feldman 2012-11-14 21:43:08 PST
> +main frame - has 1 onunload handler(s)
> 
> Could you check it please?

Looking.
Comment 8 eustas.bug 2012-11-14 22:37:22 PST
(In reply to comment #7)
> > +main frame - has 1 onunload handler(s)
> > 
> > Could you check it please?
> 
> Looking.

CODE: if (!window.testRunner) {

Causes new compiler warning: "testRunner" is not defined.
Comment 9 Csaba Osztrogonác 2012-11-15 06:11:52 PST
Fix landed in http://trac.webkit.org/changeset/134745