Bug 88978 - Web Inspector: Refactor message loop instrumentation.
Summary: Web Inspector: Refactor message loop instrumentation.
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: 88325 89443
  Show dependency treegraph
 
Reported: 2012-06-13 02:40 PDT by eustas.bug
Modified: 2012-06-19 00:59 PDT (History)
18 users (show)

See Also:


Attachments
Patch (13.86 KB, patch)
2012-06-13 02:50 PDT, eustas.bug
no flags Details | Formatted Diff | Diff
The sum of changes: new methods/dependencies are marked green (21.78 KB, image/svg+xml)
2012-06-13 03:38 PDT, eustas.bug
no flags Details
The sum of changes: new methods/dependencies are marked green (15.55 KB, image/svg+xml)
2012-06-13 03:48 PDT, eustas.bug
no flags Details
Patch (13.78 KB, patch)
2012-06-13 22:27 PDT, eustas.bug
no flags Details | Formatted Diff | Diff
Patch (13.70 KB, patch)
2012-06-14 03:13 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-06-13 02:40:26 PDT
1) Remove "messageLoop" term
2) Reuse WebThread::TaskObserver interface
3) Move implementation (from embedder) to platform code.
Comment 1 eustas.bug 2012-06-13 02:50:16 PDT
Created attachment 147268 [details]
Patch
Comment 2 WebKit Review Bot 2012-06-13 02:53:19 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 eustas.bug 2012-06-13 03:38:48 PDT
Created attachment 147278 [details]
The sum of changes: new methods/dependencies are marked green
Comment 4 eustas.bug 2012-06-13 03:48:08 PDT
Created attachment 147282 [details]
The sum of changes: new methods/dependencies are marked green
Comment 5 Darin Fisher (:fishd, Google) 2012-06-13 11:45:51 PDT
Comment on attachment 147268 [details]
Patch

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

Thanks!!!

> Source/WebKit/chromium/src/WebDevToolsAgentImpl.h:119
> +

nit: extra new line here
Comment 6 eustas.bug 2012-06-13 22:25:53 PDT
Comment on attachment 147268 [details]
Patch

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

>> Source/WebKit/chromium/src/WebDevToolsAgentImpl.h:119
>> +
> 
> nit: extra new line here

fixed
Comment 7 eustas.bug 2012-06-13 22:27:59 PDT
Created attachment 147485 [details]
Patch
Comment 8 Pavel Feldman 2012-06-14 01:49:53 PDT
Comment on attachment 147485 [details]
Patch

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

> Source/WebKit/chromium/src/WebDevToolsAgentImpl.h:105
> +    virtual void startCurrentThreadMonitoring();

I would call this "startMainThreadMonitoring". We know that it is called on the main thread and that currentThread below returns what we need. But we don't want clients to think that it is possible to instrument other threads.
Comment 9 Pavel Feldman 2012-06-14 01:56:00 PDT
Comment on attachment 147485 [details]
Patch

(please apply the rename and we can land this).
Comment 10 eustas.bug 2012-06-14 03:11:49 PDT
Comment on attachment 147485 [details]
Patch

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

>> Source/WebKit/chromium/src/WebDevToolsAgentImpl.h:105
>> +    virtual void startCurrentThreadMonitoring();
> 
> I would call this "startMainThreadMonitoring". We know that it is called on the main thread and that currentThread below returns what we need. But we don't want clients to think that it is possible to instrument other threads.

done
Comment 11 eustas.bug 2012-06-14 03:13:10 PDT
Created attachment 147540 [details]
Patch
Comment 12 WebKit Review Bot 2012-06-14 06:27:08 PDT
Comment on attachment 147540 [details]
Patch

Clearing flags on attachment: 147540

Committed r120318: <http://trac.webkit.org/changeset/120318>
Comment 13 WebKit Review Bot 2012-06-14 06:27:14 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Yury Semikhatsky 2012-06-18 05:33:27 PDT
Comment on attachment 147540 [details]
Patch

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

> Source/WebKit/chromium/public/WebDevToolsAgentClient.h:63
> +    virtual void startMainThreadMonitoring() { }

These methods are never called, please remove them.
Comment 15 eustas.bug 2012-06-18 10:31:20 PDT
(In reply to comment #14)
> (From update of attachment 147540 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=147540&action=review
> 
> > Source/WebKit/chromium/public/WebDevToolsAgentClient.h:63
> > +    virtual void startMainThreadMonitoring() { }
> 
> These methods are never called, please remove them.

Of course.
Please see:\
https://bugs.webkit.org/show_bug.cgi?id=89361