Change RefPtr<DOMWrapperWorld> that causes reference cycles to DOMWrapperWorld*

Change RefPtr<DOMWrapperWorld> that causes reference cycles to DOMWrapperWorld*

Currently a lot of V8-proxy objects hold RefPtr<DOMWrapperWorld>s, but these RefPtrs can cause reference cycles. For example, V8MutationCallback holds RefPtr<DOMWrapperWorld> like this:

  class V8MutationCallback {
    RefPtr<DOMWrapperWorld> m_world;
  };

This causes the following cycle: V8MutationCallback --(RefPtr)--> DOMWrapperWorld --(OwnPtr)--> DOMDataStore --(UnsafePersistent)--> some wrapper --> V8MutationCallback's wrapper --(RefPtr)--> V8MutationCallback.

Previously these cycles were not a problem, because only the main thread had DOMWrapperWorld and it was OK to leak memory when shutting down the main thread. However, now that we support DOMWrapperWorlds for worker threads, it's not OK to leak memory when shutting down the worker threads. Thus this CL fixes the cycles.

From the perspective of the ownership of DOMWrapperWorlds, we just need to use RefPtr<DOMWrapperWorld> in V8WindowShell, WorkerScriptController and ScriptPreprocessor. Other places should use DOMWrapperWorld*. Note that it's OK to use a raw pointer for V8MutationCallback::m_world, because DOMWrapperWorld is destroyed at the very end of the main/worker thread executions. V8MutationCallback should not outlive the DOMWrapperWorld.

BUG=341032

[haraken, 2014-03-04 15:25:05]

PTAL (though I need to fix the compile error in linux bots.)

[haraken, 2014-03-05 02:13:35]

The CQ bit was checked by haraken@chromium.org

[haraken, 2014-03-05 02:13:40]

The CQ bit was unchecked by haraken@chromium.org

[haraken, 2014-03-05 05:18:20]

The CQ bit was checked by haraken@chromium.org

[haraken, 2014-03-05 05:19:08]

The CQ bit was unchecked by haraken@chromium.org

[haraken, 2014-03-05 05:54:47]

Fixed the compile errors. PTAL.

[haraken, 2014-03-06 00:59:40]

Dan: Would you take a look at this one? This CL is necessary for landing
https://codereview.chromium.org/182903003/.

[dcarney, 2014-03-07 09:48:31]

On 2014/03/06 00:59:40, haraken wrote:
> Dan: Would you take a look at this one? This CL is necessary for landing
> https://codereview.chromium.org/182903003/.

I don't like having all these raw pointers running around...  Also,
DomWrapperMaps are weak, so i don't understand the reference cycle issue. Is the
underlying problem that we don't clear the map correctly before shutting down V8
on a worker thread?

[haraken, 2014-03-10 04:05:33]

On 2014/03/07 09:48:31, dcarney wrote:
> On 2014/03/06 00:59:40, haraken wrote:
> > Dan: Would you take a look at this one? This CL is necessary for landing
> > https://codereview.chromium.org/182903003/.
> 
> I don't like having all these raw pointers running around...  Also,
> DomWrapperMaps are weak, so i don't understand the reference cycle issue. Is
the
> underlying problem that we don't clear the map correctly before shutting down
V8
> on a worker thread?

ah, you're completely right. I was misunderstanding the issue.

- RefPtr<DOMWrapperMap> shouldn't cause cycles.
- The real issue is that DOMWrapperMap is not cleared before shutting down V8 on
worker threads.
- So what we should do is just to clear DOMWrapperMap before shutting down V8 on
worker threads.

I'll upload another CL to fix it.

[haraken, 2014-03-10 04:41:28]

On 2014/03/10 04:05:33, haraken wrote:
> On 2014/03/07 09:48:31, dcarney wrote:
> > On 2014/03/06 00:59:40, haraken wrote:
> > > Dan: Would you take a look at this one? This CL is necessary for landing
> > > https://codereview.chromium.org/182903003/.
> > 
> > I don't like having all these raw pointers running around...  Also,
> > DomWrapperMaps are weak, so i don't understand the reference cycle issue. Is
> the
> > underlying problem that we don't clear the map correctly before shutting
down
> V8
> > on a worker thread?
> 
> ah, you're completely right. I was misunderstanding the issue.
> 
> - RefPtr<DOMWrapperMap> shouldn't cause cycles.
> - The real issue is that DOMWrapperMap is not cleared before shutting down V8
on
> worker threads.
> - So what we should do is just to clear DOMWrapperMap before shutting down V8
on
> worker threads.
> 
> I'll upload another CL to fix it.

Uploaded a new CL here: https://codereview.chromium.org/189513012/

Thanks for review!