[Webkit-unassigned] [Bug 33243] [v8] Let ScriptController have more than one windowShell

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=33243


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #45948|review?                     |review+
               Flag|                            |




--- Comment #2 from Eric Seidel <eric at webkit.org>  2010-01-06 09:14:57 PST ---
(From update of attachment 45948)
ShellMap seems like a slightly lame name.  WindowShellMap might be better.  or
WorldToShellMap?

 142         // JSC triggers a GC here, but we haven't historically.

Do we know why JSC does?  Maybe to make leak detection easier? (which wouldn't
be a good reason, but it's all I can think of atm)

I might have put this:
// Force the mainWindowShell to exist.
 237     windowShell(mainThreadNormalWorld());
in a function called "ensureMainWindowShell()" (then you wouldn't need a
comment).

Oh.  Why not just call mainWorldWindowShell()?

C++ is really lame about constructs like these:
for (ShellMap::iterator iter = m_windowShells.begin(); iter !=
m_windowShells.end(); ++iter)
 486         iter->second->clearForNavigation();
I guess the best way would be to use a MACRO to save the copy/paste iteration
code.  WE could use function pointers, but those are complicated.  I think for
now the copy/paste might be OK, but it's still lame. :(  Especially since we
have 4 copy/pastes of that code!

Does the JS version destroy all shells in:
464495 void ScriptController::destroyWindowShell()
?
Seems that method is poorly named if that's the case.

I'm not sure why:
 67     V8DOMWindowShell* windowShell(DOMWrapperWorld* world)
or
 72     V8DOMWindowShell* existingWindowShell(DOMWrapperWorld* world) const
would be in the header like that.

I wonder if "if (iter)" would function the same as "iter !=
m_windowShells.end())"

"world" is superfluous:
 197     V8DOMWindowShell* initScript(DOMWrapperWorld* world);

 48     // For some reason JSDOMWindowShell uses the DOMWindow as the first
arg...
Sam Weinig would know why.

This seems OK.  See comments above.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.