Add DocumentWriteEvaluator unit tests

Add DocumentWriteEvaluator unit tests

This patch adds tests for the DocumentWriteEvaluator in isolation.

BUG=599115
Committed: https://crrev.com/732c4ab2db2056f451cb6ee13730a15276de0624
Cr-Commit-Position: refs/heads/master@{#385750}

[Charlie Harrison, 2016-04-05 18:12:28]

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

[Charlie Harrison, 2016-04-05 18:12:29]

haraken@, I noticed when writing these tests that if the TryCatch is removed
from DocumentWriteEvaluator, we crash in blink, not V8 due to lack of a window.
Here is a stack trace:

#3 0x7fe6e2189771 blink::enteredDOMWindow()
#4 0x7fe6e21a326a blink::messageHandlerInMainThread()
#5 0x7fe6e044ed9a v8::internal::MessageHandler::ReportMessage()
#6 0x7fe6e04288e9 v8::internal::Isolate::ReportPendingMessages()
#7 0x7fe6e02cefd8 v8::internal::(anonymous namespace)::Invoke()
#8 0x7fe6e02cec81 v8::internal::Execution::Call()
#9 0x7fe6dfdddb18 v8::Script::Run()
#10 0x7fe6e21bd588 blink::V8ScriptRunner::compileAndRunInternalScript()

The assert is in V8Binding.cpp on line 680. This seems problematic, WDYT? My gut
is that this is happening due to the change to use V8ScriptRunner to compile and
run the script.

[kouhei (in TOK), 2016-04-05 18:53:18]

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

[kouhei (in TOK), 2016-04-05 18:53:19]

drive by non-owner lgtm

https://codereview.chromium.org/1865543002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/DocumentWriteEvaluator.h
(right):

https://codereview.chromium.org/1865543002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/DocumentWriteEvaluator.h:42:
DocumentWriteEvaluator(const String& pathName, const String& hostName, const
String& protocol, const String& userAgent);
I think we can just make this public (keeping the comment) and use it from the
test.

[Charlie Harrison, 2016-04-05 20:02:50]

Thanks!

https://codereview.chromium.org/1865543002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/DocumentWriteEvaluator.h
(right):

https://codereview.chromium.org/1865543002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/DocumentWriteEvaluator.h:42:
DocumentWriteEvaluator(const String& pathName, const String& hostName, const
String& protocol, const String& userAgent);
On 2016/04/05 18:53:19, kouhei wrote:
> I think we can just make this public (keeping the comment) and use it from the
> test.

SGTM. Done.

[haraken, 2016-04-05 23:47:19]

LGTM

[Charlie Harrison, 2016-04-06 03:18:33]

On 2016/04/05 23:47:19, haraken wrote:
> LGTM

Sorry but can you clarify that the stack trace is not problematic? It still
feels wrong to me.

[haraken, 2016-04-06 04:18:39]

On 2016/04/06 03:18:33, csharrison wrote:
> On 2016/04/05 23:47:19, haraken wrote:
> > LGTM
> 
> Sorry but can you clarify that the stack trace is not problematic? It still
> feels wrong to me.

Sorry I missed your comment.

I think this is a bug of V8Initializer::messageHandlerInMainThread. Let me
upload a fix shortly.

[haraken, 2016-04-06 04:26:00]

On 2016/04/06 04:18:39, haraken wrote:
> On 2016/04/06 03:18:33, csharrison wrote:
> > On 2016/04/05 23:47:19, haraken wrote:
> > > LGTM
> > 
> > Sorry but can you clarify that the stack trace is not problematic? It still
> > feels wrong to me.
> 
> Sorry I missed your comment.
> 
> I think this is a bug of V8Initializer::messageHandlerInMainThread. Let me
> upload a fix shortly.

Uploaded a fix: https://codereview.chromium.org/1864623004/

[Charlie Harrison, 2016-04-06 19:41:45]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2016-04-06 19:41:45]

The patchset sent to the CQ was uploaded after l-g-t-m from kouhei@chromium.org
Link to the patchset: https://codereview.chromium.org/1865543002/#ps40001
(title: "Move anonymous namespace (trybots prev)")

[commit-bot: I haz the power, 2016-04-06 19:42:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865543002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865543002/40001

[commit-bot: I haz the power, 2016-04-06 23:48:48]

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

[commit-bot: I haz the power, 2016-04-06 23:48:49]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Charlie Harrison, 2016-04-07 13:17:51]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-07 13:17:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865543002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865543002/40001

[commit-bot: I haz the power, 2016-04-07 14:19:33]

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

[commit-bot: I haz the power, 2016-04-07 14:19:35]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2016-04-07 14:20:04]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2016-04-07 14:20:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865543002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865543002/40001

[commit-bot: I haz the power, 2016-04-07 14:25:30]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-07 14:26:16]

Description was changed from

==========
Add DocumentWriteEvaluator unit tests

This patch adds tests for the DocumentWriteEvaluator in isolation.

BUG=599115
==========

to

==========
Add DocumentWriteEvaluator unit tests

This patch adds tests for the DocumentWriteEvaluator in isolation.

BUG=599115

Committed: https://crrev.com/732c4ab2db2056f451cb6ee13730a15276de0624
Cr-Commit-Position: refs/heads/master@{#385750}
==========

[commit-bot: I haz the power, 2016-04-07 14:26:17]

Patchset 3 (id:??) landed as
https://crrev.com/732c4ab2db2056f451cb6ee13730a15276de0624
Cr-Commit-Position: refs/heads/master@{#385750}