Bug 92390 - Web Inspector: Profiles: cleanup HeapSnapshotReceiver interface
Summary: Web Inspector: Profiles: cleanup HeapSnapshotReceiver interface
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: eustas.bug
URL:
Keywords:
Depends on:
Blocks: 92348
  Show dependency treegraph
 
Reported: 2012-07-26 09:29 PDT by eustas.bug
Modified: 2012-08-01 00:54 PDT (History)
13 users (show)

See Also:


Attachments
Patch (14.36 KB, patch)
2012-07-26 09:37 PDT, eustas.bug
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description eustas.bug 2012-07-26 09:29:55 PDT
Remove callback parameter and return values from methods startLoading and finishLoading.
All implementations of HeapSnapshotReceiver honestly override interface methods now.
Comment 1 eustas.bug 2012-07-26 09:37:57 PDT
Created attachment 154672 [details]
Patch
Comment 2 Yury Semikhatsky 2012-07-31 06:32:40 PDT
Comment on attachment 154672 [details]
Patch

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

Is there a chance we have a test for this?

> Source/WebCore/inspector/front-end/HeapSnapshotLoader.js:75
> +        if (this._json)

What's the reason for splitting this into two methods?
Comment 3 eustas.bug 2012-07-31 07:42:09 PDT
Comment on attachment 154672 [details]
Patch

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

These changes are covered with tests: see changed test file - it tests both HeapSnapshotLoader and HeapSnapshotLoaderProxy ("loading" file with HeapProfileHeader).

>> Source/WebCore/inspector/front-end/HeapSnapshotLoader.js:75
>> +        if (this._json)
> 
> What's the reason for splitting this into two methods?

To make HeapSnapshotLoader implement receiver interface.

Practically we could remove @implements notation, leave this code
in peace and declare that "proxy" in not clearly "proxy", but
also "adapter"... But this way we would introduce one more entity
instead of reducing inconsistency.
Comment 4 WebKit Review Bot 2012-08-01 00:54:01 PDT
Comment on attachment 154672 [details]
Patch

Clearing flags on attachment: 154672

Committed r124308: <http://trac.webkit.org/changeset/124308>
Comment 5 WebKit Review Bot 2012-08-01 00:54:05 PDT
All reviewed patches have been landed.  Closing bug.