Bug 139276

Summary: Web Inspector: Uncaught Exceptions when attempting to show Content Flow
Product: WebKit Reporter: Brian Burg <burg>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix
none
[PATCH] Proposed Fix
none
[PATCH] Proposed Fix - Better ChangeLog none

Description Brian Burg 2014-12-04 16:13:10 PST
Test case:

<!DOCTYPE html>
<html>
<body>
<script>
function test()
{
    setTimeout(function() {}, 100);
}

eval(test.toString() + "\n//@ sourceURL= foo.js");
test();
</script>
</body>
</html>


1. Open the inspector
2. Resources
3. See test.html in the content view (Source Code or DOM)
4. Click "test.html" in the navigation area, and select "Extra Scripts"


file:///Users/burg/repos/webkit/WebKitBuild/Release/WebInspectorUI.framework/Resources/Views/ContentView.js:130:38: CONSOLE ERROR TypeError: undefined is not a valid argument for 'instanceof' (evaluating 'representedObject instanceof WebInspector.LegacyJavaScriptProfileObject')
Comment 1 Radar WebKit Bug Importer 2014-12-04 16:13:27 PST
<rdar://problem/19150581>
Comment 2 Joseph Pecoraro 2014-12-04 17:18:03 PST
That doesn't sound like an assertion, that sounds like an uncaught exception!

WebInspector.LegacyJavaScriptProfileObject doesn't exist. This is blocking creating content views for anything later on (like ContentFlow).

Should be easy to fix.
Comment 3 Joseph Pecoraro 2014-12-04 17:23:29 PST
Hmm, I don't know how you hit the exception with those steps to reproduce, but this patch should fix the issue.
Comment 4 Joseph Pecoraro 2014-12-04 17:23:46 PST
Created attachment 242602 [details]
[PATCH] Proposed Fix
Comment 5 Joseph Pecoraro 2014-12-04 17:27:08 PST
* STEPS TO REPRODUCE
1. Inspect <http://umaar.github.io/css-region-demo/>
2. Select the "example-1" Content Flow in the sidebar
  => nothing happens, uncaught exception
Comment 6 Joseph Pecoraro 2014-12-04 17:29:08 PST
Comment on attachment 242602 [details]
[PATCH] Proposed Fix

Better fix coming that will actually allow us to show content flows.
Comment 7 Joseph Pecoraro 2014-12-04 17:37:07 PST
Created attachment 242603 [details]
[PATCH] Proposed Fix

Fixes all the uncaught exceptions. Unfortunately there are issues showing the DOM Tree of some of these Content Flows, so more bugs are in order.
Comment 8 Joseph Pecoraro 2014-12-04 17:43:28 PST
Created attachment 242604 [details]
[PATCH] Proposed Fix - Better ChangeLog
Comment 9 Brian Burg 2014-12-05 10:06:18 PST
Comment on attachment 242604 [details]
[PATCH] Proposed Fix - Better ChangeLog

View in context: https://bugs.webkit.org/attachment.cgi?id=242604&action=review

> Source/WebInspectorUI/UserInterface/Views/FolderizedTreeElement.js:240
> +        parentTreeElement.insertChild(childTreeElement, insertionIndexForObjectInListSortedByFunction(childTreeElement, parentTreeElement.children, this.compareChildTreeElements.bind(this)));

Is there an eslint rule that can figure out this was missing, if the callback is not defined inline? (In this case it seems like it might be harder, and certainly not possible for arbitrary property bindings. But we don't do too much in the way of dynamically binding functions to properties)
Comment 10 WebKit Commit Bot 2014-12-05 10:47:37 PST
Comment on attachment 242604 [details]
[PATCH] Proposed Fix - Better ChangeLog

Clearing flags on attachment: 242604

Committed r176853: <http://trac.webkit.org/changeset/176853>
Comment 11 WebKit Commit Bot 2014-12-05 10:47:41 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Joseph Pecoraro 2014-12-05 10:57:38 PST
(In reply to comment #9)
> Comment on attachment 242604 [details]
> [PATCH] Proposed Fix - Better ChangeLog
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=242604&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/FolderizedTreeElement.js:240
> > +        parentTreeElement.insertChild(childTreeElement, insertionIndexForObjectInListSortedByFunction(childTreeElement, parentTreeElement.children, this.compareChildTreeElements.bind(this)));
> 
> Is there an eslint rule that can figure out this was missing, if the
> callback is not defined inline? (In this case it seems like it might be
> harder, and certainly not possible for arbitrary property bindings. But we
> don't do too much in the way of dynamically binding functions to properties)

Hmm, I'm not sure. If ESLint can jump from the token "compareChildTreeElements" to the definition of it then yes it could. But I don't think ESLint can do that right now. The rules I see it having "no-extra-bind" work on FunctionExpressions, it doesn't check for Properties and jump to analyzing the AST where that property was assigned / declared.