Remove threading primitives from ParseHTMLOnMainThread

Remove threading primitives from ParseHTMLOnMainThread

This patch removes locks from ParsedChunkQueue, and use
of crossThreadBind in HTMLDocumentParser/BackgroundHTMLParser,
which introduces unecessary copies if we're not actually crossing
threads.

BUG=627198
Committed: https://crrev.com/0b4376118260f28c64433db1002e23ed8c663cb0
Cr-Commit-Position: refs/heads/master@{#405139}

[Charlie Harrison, 2016-07-11 21:08:54]

Description was changed from

==========
Stop locking ParsedChunkQueue for ParseHTMLOnMainThread experiment

BUG=
==========

to

==========
Remove threading primitives from ParseHTMLOnMainThread

This patch removes locks from ParsedChunkQueue, and use
of crossThreadBind in HTMLDocumentParser/BackgroundHTMLParser,
which introduces unecessary copies if we're not actually crossing
threads.

BUG=627198
==========

[Charlie Harrison, 2016-07-11 21:09:26]

csharrison@chromium.org changed reviewers:
+ kouhei@chromium.org

[Charlie Harrison, 2016-07-11 21:09:29]

PTAL. There is a bit of ugliness to support both bind and crossthread bind...

[kouhei (in TOK), 2016-07-12 01:44:32]

kouhei@chromium.org changed reviewers:
+ tzik@chromium.org

[kouhei (in TOK), 2016-07-12 01:44:32]

Thanks! I like the patch but would like to delegate review to tzik. I'll stamp
once he is happy.

[kouhei (in TOK), 2016-07-12 03:11:14]

kouhei@chromium.org changed reviewers:
+ hiroshige@chromium.org
- tzik@chromium.org

[kouhei (in TOK), 2016-07-12 03:11:15]

-tzik: OOO today
kitsune-san: would you take a look?

[hiroshige, 2016-07-12 05:33:12]

lgtm, but please make sure the new code lines (commented below) work fine by
tests (on bots or at least locally).

https://codereview.chromium.org/2134323002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/BackgroundHTMLParser.cpp
(right):

https://codereview.chromium.org/2134323002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/BackgroundHTMLParser.cpp:335:
(*WTF::bind(function, std::forward<Ps>(parameters)...))();
Is this statement covered by tests?

https://codereview.chromium.org/2134323002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp (right):

https://codereview.chromium.org/2134323002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:1141:
(*WTF::bind(function, std::forward<Ps>(parameters)...))();
Is this statement covered by tests?

[kouhei (in TOK), 2016-07-12 05:34:21]

rubberstamp lgtm

[Charlie Harrison, 2016-07-12 12:18:36]

Thanks!

https://codereview.chromium.org/2134323002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/BackgroundHTMLParser.cpp
(right):

https://codereview.chromium.org/2134323002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/BackgroundHTMLParser.cpp:335:
(*WTF::bind(function, std::forward<Ps>(parameters)...))();
On 2016/07/12 05:33:12, hiroshige wrote:
> Is this statement covered by tests?

Yes, the ParseHTMLOnMainThread is set to status = test, so this runs on
LayoutTests.

https://codereview.chromium.org/2134323002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp (right):

https://codereview.chromium.org/2134323002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/parser/HTMLDocumentParser.cpp:1141:
(*WTF::bind(function, std::forward<Ps>(parameters)...))();
On 2016/07/12 05:33:12, hiroshige wrote:
> Is this statement covered by tests?

Yes, ditto.

[Charlie Harrison, 2016-07-12 13:22:27]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2016-07-12 13:22:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 14:40:11]

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

[commit-bot: I haz the power, 2016-07-12 14:40:14]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Charlie Harrison, 2016-07-12 19:04:00]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2016-07-12 19:05:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 22:55:48]

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

[commit-bot: I haz the power, 2016-07-12 22:55:49]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Charlie Harrison, 2016-07-13 12:23:21]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2016-07-13 12:23:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 12:27:17]

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

[commit-bot: I haz the power, 2016-07-13 12:27:19]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Charlie Harrison, 2016-07-13 12:53:43]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2016-07-13 12:53:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
hiroshige@chromium.org, kouhei@chromium.org
Link to the patchset: https://codereview.chromium.org/2134323002/#ps60001
(title: "rebase on 405128")

[commit-bot: I haz the power, 2016-07-13 12:53:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-13 14:10:33]

Description was changed from

==========
Remove threading primitives from ParseHTMLOnMainThread

This patch removes locks from ParsedChunkQueue, and use
of crossThreadBind in HTMLDocumentParser/BackgroundHTMLParser,
which introduces unecessary copies if we're not actually crossing
threads.

BUG=627198
==========

to

==========
Remove threading primitives from ParseHTMLOnMainThread

This patch removes locks from ParsedChunkQueue, and use
of crossThreadBind in HTMLDocumentParser/BackgroundHTMLParser,
which introduces unecessary copies if we're not actually crossing
threads.

BUG=627198
==========

[commit-bot: I haz the power, 2016-07-13 14:10:34]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-13 14:10:43]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-13 14:11:50]

Description was changed from

==========
Remove threading primitives from ParseHTMLOnMainThread

This patch removes locks from ParsedChunkQueue, and use
of crossThreadBind in HTMLDocumentParser/BackgroundHTMLParser,
which introduces unecessary copies if we're not actually crossing
threads.

BUG=627198
==========

to

==========
Remove threading primitives from ParseHTMLOnMainThread

This patch removes locks from ParsedChunkQueue, and use
of crossThreadBind in HTMLDocumentParser/BackgroundHTMLParser,
which introduces unecessary copies if we're not actually crossing
threads.

BUG=627198

Committed: https://crrev.com/0b4376118260f28c64433db1002e23ed8c663cb0
Cr-Commit-Position: refs/heads/master@{#405139}
==========

[commit-bot: I haz the power, 2016-07-13 14:11:51]

Patchset 4 (id:??) landed as
https://crrev.com/0b4376118260f28c64433db1002e23ed8c663cb0
Cr-Commit-Position: refs/heads/master@{#405139}