Rename DatabaseTaskSynchronizer to TaskSynchronizer and move it to platform/

Rename DatabaseTaskSynchronizer to TaskSynchronizer and move it to platform/

- We want to use DatabaseTaskSynchronizer in the HTML parser thread in a follow-up CL. In preparation for that, this CL renames DatabaseTaskSynchronizer to TaskSynchronizer and moves it to platform/.

BUG=340522
R=ager@chromium.org, tkent@chromium.org, wibling@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170532

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170536

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170724

[haraken, 2014-03-13 06:49:38]

PTAL

[tkent, 2014-03-13 06:53:40]

Please add TaskSynchronizer.{cpp,h} to the CL.

[haraken, 2014-03-13 07:11:48]

On 2014/03/13 06:53:40, tkent wrote:
> Please add TaskSynchronizer.{cpp,h} to the CL.

Sorry, added. PTAL.

[tkent, 2014-03-13 07:43:51]

https://codereview.chromium.org/198673002/diff/20001/Source/platform/TaskSync...
File Source/platform/TaskSynchronizer.cpp (left):

https://codereview.chromium.org/198673002/diff/20001/Source/platform/TaskSync...
Source/platform/TaskSynchronizer.cpp:35: AXSVGRoot::AXSVGRoot(RenderObject*
renderer)
Git chose a wrong copy source.
We should land a CL to copy DatabaseTask.{cpp,h} to TaskSynchronizer.{cpp,h}
before this CL (TBR=tkent is ok), or
should try to "git cl upload --similarity=" with lower values.

[haraken, 2014-03-13 08:57:20]

On 2014/03/13 07:43:51, tkent wrote:
>
https://codereview.chromium.org/198673002/diff/20001/Source/platform/TaskSync...
> File Source/platform/TaskSynchronizer.cpp (left):
> 
>
https://codereview.chromium.org/198673002/diff/20001/Source/platform/TaskSync...
> Source/platform/TaskSynchronizer.cpp:35: AXSVGRoot::AXSVGRoot(RenderObject*
> renderer)
> Git chose a wrong copy source.
> We should land a CL to copy DatabaseTask.{cpp,h} to TaskSynchronizer.{cpp,h}
> before this CL (TBR=tkent is ok), or
> should try to "git cl upload --similarity=" with lower values.

Even --similarity=0 didn't work.

BTW, will it cause any issue if I land the patch set 4 as is? I guess it will
just confuse reviewers a bit.

[tkent, 2014-03-13 09:04:02]

> BTW, will it cause any issue if I land the patch set 4 as is? I guess it will
> just confuse reviewers a bit.

It will make finding an author or a CL of a specific line of TaskSynchronizer
harder.

[haraken, 2014-03-13 09:37:55]

Reuploaded a CL with nicer diff.

PTAL.

[tkent, 2014-03-13 09:53:29]

lgtm

https://codereview.chromium.org/198673002/diff/80001/Source/modules/webdataba...
File Source/modules/webdatabase/DatabaseThread.h (right):

https://codereview.chromium.org/198673002/diff/80001/Source/modules/webdataba...
Source/modules/webdatabase/DatabaseThread.h:47: class TaskSynchronizer;
alphabetical order

https://codereview.chromium.org/198673002/diff/80001/Source/platform/TaskSync...
File Source/platform/TaskSynchronizer.cpp (right):

https://codereview.chromium.org/198673002/diff/80001/Source/platform/TaskSync...
Source/platform/TaskSynchronizer.cpp:32: #include "heap/Handle.h"
ThreadState.h instead?

[haraken, 2014-03-13 10:24:33]

https://codereview.chromium.org/198673002/diff/80001/Source/modules/webdataba...
File Source/modules/webdatabase/DatabaseThread.h (right):

https://codereview.chromium.org/198673002/diff/80001/Source/modules/webdataba...
Source/modules/webdatabase/DatabaseThread.h:47: class TaskSynchronizer;
On 2014/03/13 09:53:30, tkent wrote:
> alphabetical order

Done.

https://codereview.chromium.org/198673002/diff/80001/Source/platform/TaskSync...
File Source/platform/TaskSynchronizer.cpp (right):

https://codereview.chromium.org/198673002/diff/80001/Source/platform/TaskSync...
Source/platform/TaskSynchronizer.cpp:32: #include "heap/Handle.h"
On 2014/03/13 09:53:30, tkent wrote:
> ThreadState.h instead?

Done.

[haraken, 2014-03-13 10:24:37]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-03-13 10:24:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/198673002/100001

[commit-bot: I haz the power, 2014-03-13 10:33:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-13 10:33:05]

Try jobs failed on following builders:
  tryserver.chromium on mac_blink_compile

[haraken, 2014-03-17 09:43:18]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-03-17 09:43:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/198673002/120001

[commit-bot: I haz the power, 2014-03-17 09:49:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-17 09:49:34]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_compile_dbg

[haraken, 2014-03-19 10:19:30]

Fixed compilation issues. Landing.

[haraken, 2014-03-19 10:19:36]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-03-19 10:20:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/198673002/140001

[commit-bot: I haz the power, 2014-03-19 10:26:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-19 10:26:42]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_compile_dbg

[haraken, 2014-03-19 11:49:29]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-03-19 11:49:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/198673002/150001

[commit-bot: I haz the power, 2014-03-19 12:03:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-19 12:03:37]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_compile_dbg

[haraken, 2014-03-25 12:34:18]

Closing this CL.

I'm sorry about the confusion. We don't need to support the HTML parser thread
to move core/animations to oilpan's heap (I was confused). Thus we don't need to
move TaskSynchronizer to platform/ either.

[haraken, 2014-03-31 01:14:24]

tkent@: PTAL again.

- It turned out that we need to support the HTML parser thread in oilpan because
it allocates on-heap objects (e.g., MediaQueryParser). So I'm going to land this
CL.

- When I tried to land this CL last time, we copied DatabaseTask to
TaskSynchronizer and then tried to land this CL to make the diff saner. However,
it took me a long time to land this CL after doing the copy, which caused
confusion in the code base (mads and abarth reverted the CL). I don't think it's
a good idea to leave unused files in the code base, the latest patch set is
going to land everything in one go (although the diff is not really nice).

[Mads Ager (chromium), 2014-03-31 05:43:52]

LGTM

The reason this was reverted the last time was both that it was unused, but also
that the TaskSynchronizer files included stuff from webdatabase which is not
allowed. This changelist does not have any of those issues. :)

[tkent, 2014-03-31 06:20:14]

I still recommend to do copy-then-modification process to keep blame list
correctly.

The last try failed because haraken@ stopped proceeding the process. Having
database things in platform is harmless if it is not compiled and the process is
done in a few hours.

[Mads Ager (chromium), 2014-03-31 06:41:40]

On 2014/03/31 06:20:14, tkent wrote:
> I still recommend to do copy-then-modification process to keep blame list
> correctly.
> 
> The last try failed because haraken@ stopped proceeding the process. Having
> database things in platform is harmless if it is not compiled and the process
is
> done in a few hours.

I don't mind the two step approach. Make it so! :)

[wibling-chromium, 2014-03-31 07:23:07]

lgtm

[haraken, 2014-03-31 08:10:01]

On 2014/03/31 06:20:14, tkent wrote:
> I still recommend to do copy-then-modification process to keep blame list
> correctly.
> 
> The last try failed because haraken@ stopped proceeding the process. Having
> database things in platform is harmless if it is not compiled and the process
is
> done in a few hours.

OK, but it looks like try bots are not working today... So I cannot confirm bot
results without trying to commit this CL. Is there any good idea? (If it takes
time to land this CL, we need to revert CLs that allocated objects in the HTML
parser thread.)

[tkent, 2014-03-31 08:22:41]

On 2014/03/31 08:10:01, haraken wrote:
> OK, but it looks like try bots are not working today... So I cannot confirm
bot
> results without trying to commit this CL. Is there any good idea? (If it takes
> time to land this CL, we need to revert CLs that allocated objects in the HTML
> parser thread.)

Looks like try bots work now.

TBR is ok for a copy CL, and this CL already has owner LGTM. So, when you
confirmed green results of try bots in this CL, you may land the copy CL with
TBR, rebase this CL, then land this CL with/without CQ.

[haraken, 2014-03-31 08:46:55]

On 2014/03/31 08:22:41, tkent wrote:
> On 2014/03/31 08:10:01, haraken wrote:
> > OK, but it looks like try bots are not working today... So I cannot confirm
> bot
> > results without trying to commit this CL. Is there any good idea? (If it
takes
> > time to land this CL, we need to revert CLs that allocated objects in the
HTML
> > parser thread.)
> 
> Looks like try bots work now.
> 
> TBR is ok for a copy CL, and this CL already has owner LGTM. So, when you
> confirmed green results of try bots in this CL, you may land the copy CL with
> TBR, rebase this CL, then land this CL with/without CQ.

OK, will do that. Thanks!

[tkent, 2014-04-01 05:39:36]

https://codereview.chromium.org/198673002/diff/330001/Source/platform/TaskSyn...
File Source/platform/TaskSynchronizer.h (right):

https://codereview.chromium.org/198673002/diff/330001/Source/platform/TaskSyn...
Source/platform/TaskSynchronizer.h:39: class TaskSynchronizer {
Need PLATFORM_EXPORT

[haraken, 2014-04-01 05:40:47]

I'm hitting the following errors in try bots (This does not happen in my local
build). I think I need to add some dependency to gyp files, but do you have any
idea?

Linux:
/mnt/scratch0/b_used/build/slave/linux_layout/build/src/out/Debug/webkit_unit_tests
/mnt/scratch0/b_used/build/slave/linux_layout/build/src/out/Debug/webkit_unit_tests:
symbol lookup error:
/mnt/scratch0/b_used/build/slave/linux_layout/build/src/out/Debug/lib/libblink_web.so:
undefined symbol: _ZN7WebCore16TaskSynchronizerC1Ev


Mac:
FAILED: if [ ! -e libblink_web.dylib -o ! -e libblink_web.dylib.TOC ] || otool
-l libblink_web.dylib | grep -q LC_REEXPORT_DYLIB ; then
/Volumes/data/b/build/goma/gomacc
../../third_party/llvm-build/Release+Asserts/bin/clang++ -shared
-Wl,-search_paths_first -mmacosx-version-min=10.6 -isysroot
/Developer/SDKs/MacOSX10.6.sdk -arch i386 -L. -install_name
@rpath/libblink_web.dylib -Wl,-rpath,@loader_path/.
-Wl,-rpath,@loader_path/../../.. -o libblink_web.dylib @libblink_web.dylib.rsp
&& { otool -l libblink_web.dylib | grep LC_ID_DYLIB -A 5; nm -gP
libblink_web.dylib | cut -f1-2 -d' ' | grep -v U$; true; } >
libblink_web.dylib.TOC; else /Volumes/data/b/build/goma/gomacc
../../third_party/llvm-build/Release+Asserts/bin/clang++ -shared
-Wl,-search_paths_first -mmacosx-version-min=10.6 -isysroot
/Developer/SDKs/MacOSX10.6.sdk -arch i386 -L. -install_name
@rpath/libblink_web.dylib -Wl,-rpath,@loader_path/.
-Wl,-rpath,@loader_path/../../.. -o libblink_web.dylib @libblink_web.dylib.rsp
&& { otool -l libblink_web.dylib | grep LC_ID_DYLIB -A 5; nm -gP
libblink_web.dylib | cut -f1-2 -d' ' | grep -v U$; true; } >
libblink_web.dylib.tmp && if ! cmp -s libblink_web.dylib.tmp
libblink_web.dylib.TOC; then mv libblink_web.dylib.tmp libblink_web.dylib.TOC ;
fi; fi
Undefined symbols for architecture i386:
  "WebCore::TaskSynchronizer::TaskSynchronizer()", referenced from:
      WebCore::DatabaseContext::stopDatabases() in
libmodules.a(modules.DatabaseContext.o)
      WebCore::Database::tableNames() in libmodules.a(modules.Database.o)
      WebCore::DatabaseBackend::openAndVerifyVersion(bool,
WebCore::DatabaseError&, WTF::String&) in
libmodules.a(modules.DatabaseBackend.o)
  "WebCore::TaskSynchronizer::waitForTaskCompletion()", referenced from:
      WebCore::DatabaseContext::stopDatabases() in
libmodules.a(modules.DatabaseContext.o)
      WebCore::Database::tableNames() in libmodules.a(modules.Database.o)
      WebCore::DatabaseBackend::openAndVerifyVersion(bool,
WebCore::DatabaseError&, WTF::String&) in
libmodules.a(modules.DatabaseBackend.o)
  "WebCore::TaskSynchronizer::taskCompleted()", referenced from:
      WebCore::DatabaseThread::cleanupDatabaseThreadCompleted() in
libmodules.a(modules.DatabaseThread.o)
      WebCore::DatabaseTask::run() in libmodules.a(modules.DatabaseTask.o)
ld: symbol(s) not found for architecture i386
clang: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.


Win:
[668/771] LINK_EMBED(DLL) blink_web.dll
FAILED: E:\b\depot_tools\python276_bin\python.exe gyp-win-tool
link-with-manifests environment.x86 True blink_web.dll
"E:\b\depot_tools\python276_bin\python.exe gyp-win-tool link-wrapper
environment.x86 False link.exe /nologo /IMPLIB:blink_web.dll.lib /DLL
/OUT:blink_web.dll @blink_web.dll.rsp" 2 mt.exe rc.exe
"obj\third_party\WebKit\Source\web\blink_web.blink_web.dll.intermediate.manifest"
obj\third_party\WebKit\Source\web\blink_web.blink_web.dll.generated.manifest
modules.Database.obj : error LNK2019: unresolved external symbol "public:
__thiscall WebCore::TaskSynchronizer::TaskSynchronizer(void)"
(??0TaskSynchronizer@WebCore@@QAE@XZ) referenced in function "public: class
WTF::Vector<class WTF::String,0,class WTF::DefaultAllocator> __thiscall
WebCore::Database::tableNames(void)"
(?tableNames@Database@WebCore@@QAE?AV?$Vector@VString@WTF@@$0A@VDefaultAllocator@2@@WTF@@XZ)

modules.DatabaseBackend.obj : error LNK2001: unresolved external symbol "public:
__thiscall WebCore::TaskSynchronizer::TaskSynchronizer(void)"
(??0TaskSynchronizer@WebCore@@QAE@XZ)

modules.DatabaseContext.obj : error LNK2001: unresolved external symbol "public:
__thiscall WebCore::TaskSynchronizer::TaskSynchronizer(void)"
(??0TaskSynchronizer@WebCore@@QAE@XZ)

modules.Database.obj : error LNK2019: unresolved external symbol "public: void
__thiscall WebCore::TaskSynchronizer::waitForTaskCompletion(void)"
(?waitForTaskCompletion@TaskSynchronizer@WebCore@@QAEXXZ) referenced in function
"public: class WTF::Vector<class WTF::String,0,class WTF::DefaultAllocator>
__thiscall WebCore::Database::tableNames(void)"
(?tableNames@Database@WebCore@@QAE?AV?$Vector@VString@WTF@@$0A@VDefaultAllocator@2@@WTF@@XZ)

modules.DatabaseBackend.obj : error LNK2001: unresolved external symbol "public:
void __thiscall WebCore::TaskSynchronizer::waitForTaskCompletion(void)"
(?waitForTaskCompletion@TaskSynchronizer@WebCore@@QAEXXZ)

modules.DatabaseContext.obj : error LNK2001: unresolved external symbol "public:
void __thiscall WebCore::TaskSynchronizer::waitForTaskCompletion(void)"
(?waitForTaskCompletion@TaskSynchronizer@WebCore@@QAEXXZ)

modules.DatabaseTask.obj : error LNK2019: unresolved external symbol "public:
void __thiscall WebCore::TaskSynchronizer::taskCompleted(void)"
(?taskCompleted@TaskSynchronizer@WebCore@@QAEXXZ) referenced in function
"public: virtual void __thiscall WebCore::DatabaseTask::run(void)"
(?run@DatabaseTask@WebCore@@UAEXXZ)

modules.DatabaseThread.obj : error LNK2001: unresolved external symbol "public:
void __thiscall WebCore::TaskSynchronizer::taskCompleted(void)"
(?taskCompleted@TaskSynchronizer@WebCore@@QAEXXZ)

blink_web.dll : fatal error LNK1120: 3 unresolved externals

[haraken, 2014-04-01 05:41:08]

On 2014/04/01 05:39:36, tkent wrote:
>
https://codereview.chromium.org/198673002/diff/330001/Source/platform/TaskSyn...
> File Source/platform/TaskSynchronizer.h (right):
> 
>
https://codereview.chromium.org/198673002/diff/330001/Source/platform/TaskSyn...
> Source/platform/TaskSynchronizer.h:39: class TaskSynchronizer {
> Need PLATFORM_EXPORT

Thanks, this one!!

[haraken, 2014-04-01 06:20:30]

I think the patch set 22 will work fine. Trying try bots.

[haraken, 2014-04-01 08:03:20]

OK, the patch set 22 looks safe to land (The compile error in android bots is
not related to this CL). I'll land in the two step approach.

[haraken, 2014-04-01 08:16:28]

Committed patchset #22 manually as r170532 (presubmit successful).

[haraken, 2014-04-01 08:35:25]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-04-01 08:35:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/198673002/430001

[haraken, 2014-04-01 08:38:48]

Let me reland this CL through a commit queue. The two step approach + pending
CLs are too complicated to rebase correctly.

[commit-bot: I haz the power, 2014-04-01 09:39:21]

Change committed as 170536

[haraken, 2014-04-02 08:30:00]

This CL broke webkit_unit_tests and webkit_tests in the Android Nexus4 bot.
http://chromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Android%2...

Actually I have no idea about what's going on.

- This CL just renames TaskSynchronizer and moves it from modules/ to platform/.

- The failure happens only in the Android Nexus4 bot. 

- This is not a problem of whether we force a clobber build or not, because
reverting the CL fixed the failures without clobbering.

Any idea is welcome :)

[Mads Ager (chromium), 2014-04-02 08:45:22]

On 2014/04/02 08:30:00, haraken wrote:
> This CL broke webkit_unit_tests and webkit_tests in the Android Nexus4 bot.
>
http://chromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Android%2...
> 
> Actually I have no idea about what's going on.
> 
> - This CL just renames TaskSynchronizer and moves it from modules/ to
platform/.
> 
> - The failure happens only in the Android Nexus4 bot. 
> 
> - This is not a problem of whether we force a clobber build or not, because
> reverting the CL fixed the failures without clobbering.
> 
> Any idea is welcome :)

Beverloo pinged me on irc last night. There were crashes in the
SaveRegisters_arm stub. Looked like thumb interworking issues again. Didn't make
much sense to me. Slava was the last person to look at this. Slava, could you
have a look when you get in?

[haraken, 2014-04-02 14:06:31]

On 2014/04/02 08:45:22, Mads Ager (chromium) wrote:
> On 2014/04/02 08:30:00, haraken wrote:
> > This CL broke webkit_unit_tests and webkit_tests in the Android Nexus4 bot.
> >
>
http://chromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Android%2...
> > 
> > Actually I have no idea about what's going on.
> > 
> > - This CL just renames TaskSynchronizer and moves it from modules/ to
> platform/.
> > 
> > - The failure happens only in the Android Nexus4 bot. 
> > 
> > - This is not a problem of whether we force a clobber build or not, because
> > reverting the CL fixed the failures without clobbering.
> > 
> > Any idea is welcome :)
> 
> Beverloo pinged me on irc last night. There were crashes in the
> SaveRegisters_arm stub. Looked like thumb interworking issues again. Didn't
make
> much sense to me. Slava was the last person to look at this. Slava, could you
> have a look when you get in?

Slava is landing a fix for the Android issue in
https://codereview.chromium.org/222213002/

[haraken, 2014-04-03 00:35:16]

On 2014/04/02 14:06:31, haraken wrote:
> On 2014/04/02 08:45:22, Mads Ager (chromium) wrote:
> > On 2014/04/02 08:30:00, haraken wrote:
> > > This CL broke webkit_unit_tests and webkit_tests in the Android Nexus4
bot.
> > >
> >
>
http://chromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Android%2...
> > > 
> > > Actually I have no idea about what's going on.
> > > 
> > > - This CL just renames TaskSynchronizer and moves it from modules/ to
> > platform/.
> > > 
> > > - The failure happens only in the Android Nexus4 bot. 
> > > 
> > > - This is not a problem of whether we force a clobber build or not,
because
> > > reverting the CL fixed the failures without clobbering.
> > > 
> > > Any idea is welcome :)
> > 
> > Beverloo pinged me on irc last night. There were crashes in the
> > SaveRegisters_arm stub. Looked like thumb interworking issues again. Didn't
> make
> > much sense to me. Slava was the last person to look at this. Slava, could
you
> > have a look when you get in?
> 
> Slava is landing a fix for the Android issue in
> https://codereview.chromium.org/222213002/

The fix was landed, so I'm relanding this CL.

[haraken, 2014-04-03 00:35:57]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-04-03 00:36:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/haraken@chromium.org/198673002/430001

[commit-bot: I haz the power, 2014-04-03 00:36:53]

Change committed as 170724