Bug 73851 - [MutationObservers] Enable in Chromium trunk
Summary: [MutationObservers] Enable in Chromium trunk
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rafael Weinstein
URL:
Keywords:
Depends on: 73919 73937
Blocks: 68729
  Show dependency treegraph
 
Reported: 2011-12-05 12:21 PST by Rafael Weinstein
Modified: 2011-12-13 08:40 PST (History)
9 users (show)

See Also:


Attachments
Patch (1.23 KB, patch)
2011-12-05 17:47 PST, Rafael Weinstein
no flags Details | Formatted Diff | Diff
Patch (2.67 KB, patch)
2011-12-07 14:11 PST, Adam Klein
ojan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rafael Weinstein 2011-12-05 12:21:53 PST
Mutation observers is now functionally complete on the chromium port (some work in JSC remains for completeness). We'd like to now parallelize getting developer feedback and completing the development work. 

In particular, we plan to invite chrome extension developers who are currently using DOM Mutation Events to experiment with re-implementing using Mutation Observers.

Note that there is currently no plan to release Mutation Observers to chrome beta or release channel, and doing so is predicated on (at minimum) completing all blocking work on the webkit Mutation Observers meta bug (68729).
Comment 1 Rafael Weinstein 2011-12-05 17:47:24 PST
Created attachment 117967 [details]
Patch
Comment 2 WebKit Review Bot 2011-12-05 23:50:04 PST
Comment on attachment 117967 [details]
Patch

Clearing flags on attachment: 117967

Committed r102102: <http://trac.webkit.org/changeset/102102>
Comment 3 WebKit Review Bot 2011-12-05 23:50:10 PST
All reviewed patches have been landed.  Closing bug.
Comment 4 Vsevolod Vlasov 2011-12-06 08:28:30 PST
This patch caused a lot of failures.

1)
https://bugs.webkit.org/show_bug.cgi?id=73919
Layout Test inspector/debugger/dom-breakpoints.html is failing on chromium linux debug

2)
This patch caused compile failures on all chromium.webkit win builder bots once another patch triggered recompilation of WebKit.cpp.
Compile failures were fixed by clobbering builder bots.

3)
https://bugs.webkit.org/show_bug.cgi?id=73925
REGRESSION(r102102): Causes segmentation fault on chromium unit_tests, content_unittests, sync_unit_tests (Requested by vsevik on #webkit).

-----

Please run chromium try bots before landing this patch next time. 
http://www.chromium.org/developers/testing/try-server-usage#TOC-Simultaneously-patch-code-in-the-We describes how to do that using --sub_rep.

I think you should also notify chromium/webkit sheriffs before landing this patch because rolling webkit DEPS with this patch can potentially break chromium tree (win builders)
Comment 5 Adam Klein 2011-12-06 09:17:09 PST
(In reply to comment #4)
> This patch caused a lot of failures.

Sorry the trouble.

> 1)
> https://bugs.webkit.org/show_bug.cgi?id=73919
> Layout Test inspector/debugger/dom-breakpoints.html is failing on chromium linux debug
> 
> 2)
> This patch caused compile failures on all chromium.webkit win builder bots once another patch triggered recompilation of WebKit.cpp.
> Compile failures were fixed by clobbering builder bots.

Here's the failure:

http://build.chromium.org/p/chromium.webkit/builders/Win%20Builder/builds/13012/steps/compile/logs/stdio

It's not clear to me whether there's anything we can do here besides clobbering (haven't seen these errors on Linux or Mac builds).

> 3)
> https://bugs.webkit.org/show_bug.cgi?id=73925
> REGRESSION(r102102): Causes segmentation fault on chromium unit_tests, content_unittests, sync_unit_tests (Requested by vsevik on #webkit).

Do you have a stack trace for these segfaults? I don't see any output from the bots.

> 
> -----
> 
> Please run chromium try bots before landing this patch next time. 
> http://www.chromium.org/developers/testing/try-server-usage#TOC-Simultaneously-patch-code-in-the-We describes how to do that using --sub_rep.
> 
> I think you should also notify chromium/webkit sheriffs before landing this patch because rolling webkit DEPS with this patch can potentially break chromium tree (win builders)
Comment 6 Vsevolod Vlasov 2011-12-06 09:29:14 PST
> > 3)
> > https://bugs.webkit.org/show_bug.cgi?id=73925
> > REGRESSION(r102102): Causes segmentation fault on chromium unit_tests, content_unittests, sync_unit_tests (Requested by vsevik on #webkit).
> 
> Do you have a stack trace for these segfaults? I don't see any output from the bots.
No. The bots didn't have anything. I've just built everything release configuration locally and bisected to find the failure reason, so I don't have a stack trace.
It reproduces locally easily (I did it on linux).
Comment 7 Adam Klein 2011-12-06 11:08:12 PST
(In reply to comment #6)
> > > 3)
> > > https://bugs.webkit.org/show_bug.cgi?id=73925
> > > REGRESSION(r102102): Causes segmentation fault on chromium unit_tests, content_unittests, sync_unit_tests (Requested by vsevik on #webkit).
> > 
> > Do you have a stack trace for these segfaults? I don't see any output from the bots.
> No. The bots didn't have anything. I've just built everything release configuration locally and bisected to find the failure reason, so I don't have a stack trace.
> It reproduces locally easily (I did it on linux).

Reproduces locally, the problem is WebKit::initialize() is called with WebKitPlatformSupport::currentThread() NULL.  Patch on the way.
Comment 8 Adam Klein 2011-12-07 14:11:18 PST
Created attachment 118276 [details]
Patch
Comment 9 Adam Klein 2011-12-07 14:46:32 PST
Committed r102275: <http://trac.webkit.org/changeset/102275>
Comment 10 Jarek Foksa 2011-12-13 05:48:24 PST
I'm interested in testing this feature, but window.MutationObserver object is not present in current Chromium snapashot (http://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?path=Linux_x64/114205/)

Do I need some additional flag to enable it?
Comment 11 Adam Klein 2011-12-13 08:40:21 PST
(In reply to comment #10)
> I'm interested in testing this feature, but window.MutationObserver object is not present in current Chromium snapashot (http://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?path=Linux_x64/114205/)
> 
> Do I need some additional flag to enable it?

It's vendor-prefixed: look for window.WebKitMutationObserver.