Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(103)

Issue 8586038: Implement WebThread::{add,remove}TaskObserver. (Closed)

Created:
9 years, 1 month ago by adamk
Modified:
9 years ago
CC:
chromium-reviews, darin-cc_chromium.org, rafaelw
Visibility:
Public.

Description

Implement WebThread::{add,remove}TaskObserver. Depends on http://trac.webkit.org/changeset/101418 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113010

Patch Set 1 #

Patch Set 2 : Remove willProcessTask #

Patch Set 3 : Move impl to WebThreadImpl #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -4 lines) Patch
M webkit/glue/webthread_impl.h View 1 2 2 chunks +26 lines, -4 lines 0 comments Download
M webkit/glue/webthread_impl.cc View 1 2 4 chunks +45 lines, -0 lines 2 comments Download

Messages

Total messages: 5 (0 generated)
adamk
9 years ago (2011-11-29 21:18:03 UTC) #1
darin (slow to review)
LGTM http://codereview.chromium.org/8586038/diff/3001/webkit/glue/webthread_impl.cc File webkit/glue/webthread_impl.cc (right): http://codereview.chromium.org/8586038/diff/3001/webkit/glue/webthread_impl.cc#newcode71 webkit/glue/webthread_impl.cc:71: return thread_->thread_id() == base::PlatformThread::CurrentId(); it feels like this ...
9 years ago (2011-11-30 06:45:38 UTC) #2
adamk
http://codereview.chromium.org/8586038/diff/3001/webkit/glue/webthread_impl.cc File webkit/glue/webthread_impl.cc (right): http://codereview.chromium.org/8586038/diff/3001/webkit/glue/webthread_impl.cc#newcode71 webkit/glue/webthread_impl.cc:71: return thread_->thread_id() == base::PlatformThread::CurrentId(); On 2011/11/30 06:45:39, darin wrote: ...
9 years ago (2011-11-30 21:58:24 UTC) #3
Avi (use Gerrit)
Reverted in r113014. ./webkit/glue/webthread_impl.h:16:1:error: [chromium-style] Complex class/struct needs an explicit out-of-line constructor. class WebThreadBase : ...
9 years ago (2011-12-05 20:49:10 UTC) #4
adamk
9 years ago (2011-12-05 21:19:57 UTC) #5
On 2011/12/05 20:49:10, Avi wrote:
> Reverted in r113014.
> 
> ./webkit/glue/webthread_impl.h:16:1:error: [chromium-style] Complex
class/struct
> needs an explicit out-of-line constructor.
> class WebThreadBase : public WebKit::WebThread {
> ^
> ./webkit/glue/webthread_impl.h:16:1:error: [chromium-style] Complex
class/struct
> needs an explicit out-of-line destructor.
> ./webkit/glue/webthread_impl.h:41:11:error: [chromium-style] Overriding method
> must be marked with OVERRIDE.
>   virtual bool IsCurrentThread() const;
>           ^
> ./webkit/glue/webthread_impl.h:56:11:error: [chromium-style] Overriding method
> must be marked with OVERRIDE.
>   virtual bool IsCurrentThread() const;
>           ^
> 
> Use trybots. That's what they're there for.

For the record: I thought I'd trybot'd this, but lost state due to the >2week
delay in landing it. It'd be nice if the presubmit would warn if it sees no try
jobs (rather than just failed tryjobs)...

Powered by Google App Engine
This is Rietveld 408576698