Bug 61040

Summary: Web Inspector: initial showing of heap snapshot doesn't provide any feedback on waiting time
Product: WebKit Reporter: Ilya Tikhonovsky <loislo>
Component: Web Inspector (Deprecated)Assignee: Mikhail Naganov <mnaganov>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
patch
yurys: review-, mnaganov: commit-queue-
comment addressed yurys: review+, mnaganov: commit-queue-

Description Ilya Tikhonovsky 2011-05-18 04:52:29 PDT 
there are 4 stages of making detailed heap snapshot.
1) snapshoting
2) loading...
3) parsing...
4) a stage between parsing and the moment when snapshot items appear in the list.

it'd be nice to have a text for the fourth stage and a kind of progress indicator for all the stages.
Comment 1 Mikhail Naganov 2011-05-20 08:34:20 PDT 
Created attachment 94224 [details]
patch
Comment 2 Yury Semikhatsky 2011-05-20 08:55:30 PDT 
Comment on attachment 94224 [details]
patch

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

> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:93
> +        if (action)

The action will not be called if the message is already visible. r- for this.
Comment 3 Mikhail Naganov 2011-05-20 12:19:45 PDT 
Created attachment 94258 [details]
comment addressed
Comment 4 Mikhail Naganov 2011-05-20 12:20:10 PDT 
(In reply to comment #2)
> (From update of attachment 94224 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=94224&action=review
> 
> > Source/WebCore/inspector/front-end/PleaseWaitMessage.js:93
> > +        if (action)
> 
> The action will not be called if the message is already visible. r- for this.

Really good catch! Fixed.
Comment 5 Yury Semikhatsky 2011-05-23 00:21:41 PDT 
Comment on attachment 94258 [details]
comment addressed

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

> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:90
> +            listenObject.addEventListener(listenEvent, WebInspector.PleaseWaitMessage.prototype.unlistenAndHide.bind(instance, listenObject, listenEvent), instance);

Why no use a shorter notation: instance.unlistenAndHide.bind(instance, listenObject, listenEvent)? Also, you don't need to pass instance as the third parameter since you pass it in bind.
Comment 6 Mikhail Naganov 2011-05-23 00:46:40 PDT 
Comment on attachment 94258 [details]
comment addressed

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

>> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:90
>> +            listenObject.addEventListener(listenEvent, WebInspector.PleaseWaitMessage.prototype.unlistenAndHide.bind(instance, listenObject, listenEvent), instance);
> 
> Why no use a shorter notation: instance.unlistenAndHide.bind(instance, listenObject, listenEvent)? Also, you don't need to pass instance as the third parameter since you pass it in bind.

Fixed.
The third parameter is used by Object.addEventListener / removeEventListener pair of functions to identify the listener, as they can't extract the event handler from the closure passed.
Comment 7 Mikhail Naganov 2011-05-23 00:52:14 PDT 
Manually committed http://trac.webkit.org/changeset/87056


2011-05-23  Mikhail Naganov  <mnaganov@chromium.org>

        Reviewed by Yury Semikhatsky.

        Web Inspector: [Chromium] Show "Please wait" message during initial
        heap snapshot processing.
        https://bugs.webkit.org/show_bug.cgi?id=61040

        * inspector/front-end/DetailedHeapshotView.js:
        (WebInspector.HeapSnapshotSortableDataGrid.prototype.sortingChanged.sort):
        (WebInspector.HeapSnapshotSortableDataGrid.prototype.sortingChanged):
        (WebInspector.DetailedHeapshotView.profileCallback):
        (WebInspector.DetailedHeapshotView.setDataSource):
        (WebInspector.DetailedHeapshotView):
        (WebInspector.DetailedHeapshotView.prototype._changeBase.baseProfileLoaded):
        (WebInspector.DetailedHeapshotView.prototype._changeBase.setDataSource):
        (WebInspector.DetailedHeapshotView.prototype._changeBase):
        (WebInspector.DetailedHeapshotView.prototype._changeView.setDataSource):
        (WebInspector.DetailedHeapshotView.prototype._changeView):
        * inspector/front-end/PleaseWaitMessage.js:
        (WebInspector.PleaseWaitMessage.prototype.showAndWaitFor):