Bug 34530 – [v8] Remove clear method from DOM object maps

• Component: WebCore Misc.
• Status: RESOLVED WONTFIX

[anton muhin]

[v8] Remove clear method from DOM object maps

[anton muhin]

Created attachment 48034 [details]
Patch

[anton muhin]

Currently the only client of clear method is WorkerScriptController (this method is called in dtor.)

I believe that as worker most probably would own a separate V8 instance, VM destroy should take care of clearing (if it's not the case, I'd be glad to fix it on V8-side.)

If this cleanup is crucial, I'd provide a separate method to iterate through created wrappers.

This patch is just one in (planned) series of patches which could lead to elimination of indirect reference from the Node to its wrapper.  The next move would be to get rid of visit which is somewhat more involved.

[Adam Barth]

Comment on attachment 48034 [details]
Patch

Amazing!  I take it this functionality had no clients.  :)

[anton muhin]

Thanks a lot for review, Adam.

just in case, if I manage to get rid of indirection, ChunkedTable would go away as well---I remember about my promise :)

[WebKit Commit Bot]

Comment on attachment 48034 [details]
Patch

Clearing flags on attachment: 48034

Committed r54341: <http://trac.webkit.org/changeset/54341>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Stephen White]

Sorry, I had to revert this, since it broke the ui_tests fpr Workers.  See 

http://build.chromium.org/buildbot/waterfall/builders/XP%20Tests%20(dbg)(2)/builds/13955/steps/ui_tests/logs/stdio

---

[ RUN      ] WorkerTest.MultipleWorkers
C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167): error: Value of: actual_crashes
  Actual: 1
Expected: expected_crashes_
Which is: 0
Encountered an unexpected crash in the program during this test.
[  FAILED  ] WorkerTest.MultipleWorkers (3609 ms)
[ RUN      ] WorkerTest.IncognitoSharedWorkers
C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167): error: Value of: actual_crashes
  Actual: 1
Expected: expected_crashes_
Which is: 0
Encountered an unexpected crash in the program during this test.
[  FAILED  ] WorkerTest.IncognitoSharedWorkers (3985 ms)
[ RUN      ] WorkerTest.WorkerFastLayoutTests2
Test: worker-call.html
Test: worker-constructor.html
Test: worker-event-listener.html
Test: worker-messageport.html
Test: worker-replace-global-constructor.html
Test: worker-terminate.html
C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167): error: Value of: actual_crashes
  Actual: 6
Expected: expected_crashes_
Which is: 0
Encountered an unexpected crash in the program during this test.
[  FAILED  ] WorkerTest.WorkerFastLayoutTests2 (10297 ms)

[anton muhin]

Sorry about that---I ran ui_tests on my box and they were fine.  Will have a closer look.

(In reply to comment #7)
> Sorry, I had to revert this, since it broke the ui_tests fpr Workers.  See 
> 
> http://build.chromium.org/buildbot/waterfall/builders/XP%20Tests%20(dbg)(2)/builds/13955/steps/ui_tests/logs/stdio
> 
> ---
> 
> [ RUN      ] WorkerTest.MultipleWorkers
> C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167):
> error: Value of: actual_crashes
>   Actual: 1
> Expected: expected_crashes_
> Which is: 0
> Encountered an unexpected crash in the program during this test.
> [  FAILED  ] WorkerTest.MultipleWorkers (3609 ms)
> [ RUN      ] WorkerTest.IncognitoSharedWorkers
> C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167):
> error: Value of: actual_crashes
>   Actual: 1
> Expected: expected_crashes_
> Which is: 0
> Encountered an unexpected crash in the program during this test.
> [  FAILED  ] WorkerTest.IncognitoSharedWorkers (3985 ms)
> [ RUN      ] WorkerTest.WorkerFastLayoutTests2
> Test: worker-call.html
> Test: worker-constructor.html
> Test: worker-event-listener.html
> Test: worker-messageport.html
> Test: worker-replace-global-constructor.html
> Test: worker-terminate.html
> C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167):
> error: Value of: actual_crashes
>   Actual: 6
> Expected: expected_crashes_
> Which is: 0
> Encountered an unexpected crash in the program during this test.
> [  FAILED  ] WorkerTest.WorkerFastLayoutTests2 (10297 ms)

[anton muhin]

That's strange.  I ran ui_tests couple of times on both Ubuntu and Windows 7 and didn't see anything like that.  Could it have been some flakiness?  Do we have any trybots for ui_tests?

(In reply to comment #8)
> Sorry about that---I ran ui_tests on my box and they were fine.  Will have a
> closer look.
> 
> (In reply to comment #7)
> > Sorry, I had to revert this, since it broke the ui_tests fpr Workers.  See 
> > 
> > http://build.chromium.org/buildbot/waterfall/builders/XP%20Tests%20(dbg)(2)/builds/13955/steps/ui_tests/logs/stdio
> > 
> > ---
> > 
> > [ RUN      ] WorkerTest.MultipleWorkers
> > C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167):
> > error: Value of: actual_crashes
> >   Actual: 1
> > Expected: expected_crashes_
> > Which is: 0
> > Encountered an unexpected crash in the program during this test.
> > [  FAILED  ] WorkerTest.MultipleWorkers (3609 ms)
> > [ RUN      ] WorkerTest.IncognitoSharedWorkers
> > C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167):
> > error: Value of: actual_crashes
> >   Actual: 1
> > Expected: expected_crashes_
> > Which is: 0
> > Encountered an unexpected crash in the program during this test.
> > [  FAILED  ] WorkerTest.IncognitoSharedWorkers (3985 ms)
> > [ RUN      ] WorkerTest.WorkerFastLayoutTests2
> > Test: worker-call.html
> > Test: worker-constructor.html
> > Test: worker-event-listener.html
> > Test: worker-messageport.html
> > Test: worker-replace-global-constructor.html
> > Test: worker-terminate.html
> > C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167):
> > error: Value of: actual_crashes
> >   Actual: 6
> > Expected: expected_crashes_
> > Which is: 0
> > Encountered an unexpected crash in the program during this test.
> > [  FAILED  ] WorkerTest.WorkerFastLayoutTests2 (10297 ms)

[Stephen White]

(In reply to comment #9)
> That's strange.  I ran ui_tests couple of times on both Ubuntu and Windows 7
> and didn't see anything like that.  Could it have been some flakiness?

It's possible it was flake, although the problems all went away with the revert.  I don't know enough about these tests to say.

> Do we have any trybots for ui_tests?

The regular trybots (win/mac/linux) run ui_tests, I believe.

It's possible that i
> 
> (In reply to comment #8)
> > Sorry about that---I ran ui_tests on my box and they were fine.  Will have a
> > closer look.
> > 
> > (In reply to comment #7)
> > > Sorry, I had to revert this, since it broke the ui_tests fpr Workers.  See 
> > > 
> > > http://build.chromium.org/buildbot/waterfall/builders/XP%20Tests%20(dbg)(2)/builds/13955/steps/ui_tests/logs/stdio
> > > 
> > > ---
> > > 
> > > [ RUN      ] WorkerTest.MultipleWorkers
> > > C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167):
> > > error: Value of: actual_crashes
> > >   Actual: 1
> > > Expected: expected_crashes_
> > > Which is: 0
> > > Encountered an unexpected crash in the program during this test.
> > > [  FAILED  ] WorkerTest.MultipleWorkers (3609 ms)
> > > [ RUN      ] WorkerTest.IncognitoSharedWorkers
> > > C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167):
> > > error: Value of: actual_crashes
> > >   Actual: 1
> > > Expected: expected_crashes_
> > > Which is: 0
> > > Encountered an unexpected crash in the program during this test.
> > > [  FAILED  ] WorkerTest.IncognitoSharedWorkers (3985 ms)
> > > [ RUN      ] WorkerTest.WorkerFastLayoutTests2
> > > Test: worker-call.html
> > > Test: worker-constructor.html
> > > Test: worker-event-listener.html
> > > Test: worker-messageport.html
> > > Test: worker-replace-global-constructor.html
> > > Test: worker-terminate.html
> > > C:\b\slave\chromium-dbg-builder\build\src\chrome\test\ui\ui_test.cc(167):
> > > error: Value of: actual_crashes
> > >   Actual: 6
> > > Expected: expected_crashes_
> > > Which is: 0
> > > Encountered an unexpected crash in the program during this test.
> > > [  FAILED  ] WorkerTest.WorkerFastLayoutTests2 (10297 ms)

[Anders Carlsson]

V8 is gone.