Bug 66751

Summary: [Chromium]WebWorkerClientImpl should always invoke InspectorIntrumentation on main thread
Product: WebKit Reporter: Dmitry Lomov <dslomov>
Component: WebCore Misc.Assignee: Dmitry Lomov <dslomov>
Severity: Normal CC: ap, dimich, levin, pfeldman, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
This patch moves calls to InspectorInstrumentation from WebWorkerClientImpl to WorkerMessagingProxy
Indentation fixed
pfeldman: review+
CR feedback
One more indentation fix.. none

Description Dmitry Lomov 2011-08-23 01:00:28 PDT
InspectorInstrumentation access Document, so the callbacks should always be invoked on main thread (http://code.google.com/p/chromium/issues/detail?id=93777).
Comment 1 Dmitry Lomov 2011-08-23 01:19:18 PDT
Created attachment 104802 [details]
This patch moves calls to InspectorInstrumentation from WebWorkerClientImpl to WorkerMessagingProxy
Comment 2 WebKit Review Bot 2011-08-23 01:21:03 PDT
Attachment 104802 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit/chromium/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/chromium/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/chromium/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 6 in 3 files

If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Dmitry Lomov 2011-08-23 01:24:35 PDT
Created attachment 104803 [details]
Indentation fixed
Comment 4 Pavel Feldman 2011-08-23 01:50:26 PDT
Comment on attachment 104803 [details]
Indentation fixed

Looks good. Could you please add ASSERT(isMainThread()) into instrumentationForPage(Page* page) (InstrumentingAgents.cpp) as a drive by? That way it'll hit us prior to thread sanitizer next time.
Comment 5 Dmitry Lomov 2011-08-23 13:41:10 PDT
Created attachment 104905 [details]
CR feedback
Comment 6 Dmitry Lomov 2011-08-23 13:43:14 PDT
Created attachment 104907 [details]
One more indentation fix..
Comment 7 WebKit Review Bot 2011-08-23 16:22:32 PDT
Comment on attachment 104907 [details]
One more indentation fix..

Clearing flags on attachment: 104907

Committed r93654: <http://trac.webkit.org/changeset/93654>
Comment 8 WebKit Review Bot 2011-08-23 16:22:37 PDT
All reviewed patches have been landed.  Closing bug.