[Webkit-unassigned] [Bug 17134] Inspector should have an integrated JavaScript debugger

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=17134


darin at apple.com changed:

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




------- Comment #11 from darin at apple.com  2008-04-02 03:07 PDT -------
(From update of attachment 19836)
+    PageListenersMap::iterator it = m_pageListenersMap.find(page);
+    if (it == m_pageListenersMap.end()) {
+        pair<PageListenersMap::iterator, bool> result =
m_pageListenersMap.add(page, new ListenerSet);
+        ASSERT(result.second);
+        it = result.first;
+    }

The above is not a great idiom, because it unnecessarily does two hash table
lookups when adding a new entry. A better way is:

    pair<PageListenersMap::iterator, bool> result =
m_pageListenersMap.add(page, 0);
    if (result.second)
        result.first->second = new ListenerSet;
    ListenerSet* listeners = result.first->second;

Is it legal to add a listener when it's already in, or remove a listener if
it's not in? If not, then I suggest adding some assertions.

The use of "state" instead of "exec" for ExecState* arguments is quite
unconventional. I suggest you either use "exec" or push to get "state" used
elsewhere. Maybe we can switch using do-webcore-rename, because I bet "exec" is
not used for anything except ExecState*.

r=me


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