Bug 19225 – Web Inspector: create automated profiler tests

• Status: REOPENED
• Component: Web Inspector

[Kevin McCullough]

In WebCore/manual-tests/inspector there are many manual tests for the new profiler.  It would be great if we could automate them.

[Mark Rowe (bdash)]

*** Bug 19226 has been marked as a duplicate of this bug. ***

[Kevin McCullough]

Created attachment 24566 [details]
Proposed patch

[Timothy Hatcher]

Comment on attachment 24566 [details]
Proposed patch

+    if(iter == end)
+        return jsNull();

I think it would be better to return an empty array instead of null.

+const ProfilesArray& Console::profiles() const
+{
+    Page* page = this->page();
+    ASSERT_ARG(page, page);
+    return page->inspectorController()->profiles();
+}

We need to have the Console object hold the profiles, since there is a Console object per Frame. Asking the InspectorController for the Profiles would return all profiles for all frames, and that is a security issue.

The InspectorController could still have it's own list since it does want all Profiles for all Frames.

[Kevin McCullough]

Created attachment 24574 [details]
Proposed patch

[Oliver Hunt]

Comment on attachment 24574 [details]
Proposed patch

I don't like
 46     if(iter == end)
 47         return constructArray(exec, list);
 48 
 49     do {
 50         list.append(toJS(exec, iter->get()));
 51         ++iter;
 52     } while (iter != end);

i think
ArgList list;
ProfilesArray::const_iterator end = profiles.end();
for (ProfilesArray::const_iterator iter = profiles.begin(); iter != end; ++iter)
    list.append(toJS(exec, iter->get()));

return constructArray(exec, list);

is better as it's simpler, smaller, and more in keeping with the style of similar code elsewhere.

[Kevin McCullough]

Created attachment 24578 [details]
Proposed patch

[Kevin McCullough]

Committed revision 37791.

[Kevin McCullough]

We need to do this better.  Exposing the profiles via JS is not necessarily the best way to go right now.  We don't know the correct format or API of a profile.  A new idea is to let DRT use SPI to get the profiles and we can return to the idea of exposing profiles via JS later.

[Timothy Hatcher]

Comment on attachment 24578 [details]
Proposed patch

Clearing the review flag since this landed. Keeping open based on Kevin's last comment.