Blink-in-JS: Support private scripts in partial interfaces (PrivateScriptRunner part)

Blink-in-JS: Support private scripts in partial interfaces (PrivateScriptRunner part)

This CL is a split from https://codereview.chromium.org/457483002/.
This CL supports private scripts in partial interfaces.

Assume that XPartial-1 and XPartial-2 are partial interfaces of X.
Then we have XPartial-1.js, XPartial-2.js and X.js.
When a DOM attribute/method in any of the JS files is requested
at the first time, all of the JS files are compiled at the same time.

This is redundant because when a DOM attribute/method in XPartial-1.js
is requested, we just need to compile XPartial-1.js and X.js.
I'll address the issue in a follow-up CL.

BUG=341031
TEST=fast/dom/private-script-unittest.html

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

[haraken, 2014-08-08 06:11:48]

abarth@ or arv@: PTAL.

jl@, tasak@, vivekg@: FYI.

[Jens Widell, 2014-08-09 14:52:25]

https://codereview.chromium.org/454773002/diff/20001/Source/bindings/core/v8/...
File Source/bindings/core/v8/PrivateScriptRunner.cpp (right):

https://codereview.chromium.org/454773002/diff/20001/Source/bindings/core/v8/...
Source/bindings/core/v8/PrivateScriptRunner.cpp:33: v8::TryCatch block;
We could unduplicate some code here; this block is essentially duplicated three
times.

[haraken, 2014-08-11 01:18:31]

https://codereview.chromium.org/454773002/diff/20001/Source/bindings/core/v8/...
File Source/bindings/core/v8/PrivateScriptRunner.cpp (right):

https://codereview.chromium.org/454773002/diff/20001/Source/bindings/core/v8/...
Source/bindings/core/v8/PrivateScriptRunner.cpp:33: v8::TryCatch block;
On 2014/08/09 14:52:25, Jens Lindström wrote:
> We could unduplicate some code here; this block is essentially duplicated
three
> times.

Removed the duplication.

[haraken, 2014-08-11 12:00:54]

jl@: Would you mind taking a look at this when you have a cycle?

[Jens Widell, 2014-08-11 12:27:57]

LGTM with a few comments.

https://codereview.chromium.org/454773002/diff/60001/Source/bindings/core/v8/...
File Source/bindings/core/v8/PrivateScriptRunner.cpp (right):

https://codereview.chromium.org/454773002/diff/60001/Source/bindings/core/v8/...
Source/bindings/core/v8/PrivateScriptRunner.cpp:29: WTF_LOG_ERROR("Private
script error: Compile failed. (Class name = %s)\n", className.utf8().data());
Intentional to not use LOG_ERROR_ALWAYS() here?

https://codereview.chromium.org/454773002/diff/60001/Source/bindings/core/v8/...
File Source/bindings/core/v8/PrivateScriptRunner.js (right):

https://codereview.chromium.org/454773002/diff/60001/Source/bindings/core/v8/...
Source/bindings/core/v8/PrivateScriptRunner.js:85: if
(installedClasses[className] === undefined)
"!(className in installedClasses)" would be a more direct way to check if we've
stored a class already.

https://codereview.chromium.org/454773002/diff/60001/Source/core/core_generat...
File Source/core/core_generated.gyp (right):

https://codereview.chromium.org/454773002/diff/60001/Source/core/core_generat...
Source/core/core_generated.gyp:158: ],
Since you were fixing indentation already: deindent this and following lines
too?

[haraken, 2014-08-11 12:41:40]

Thanks for review!

https://codereview.chromium.org/454773002/diff/60001/Source/bindings/core/v8/...
File Source/bindings/core/v8/PrivateScriptRunner.cpp (right):

https://codereview.chromium.org/454773002/diff/60001/Source/bindings/core/v8/...
Source/bindings/core/v8/PrivateScriptRunner.cpp:29: WTF_LOG_ERROR("Private
script error: Compile failed. (Class name = %s)\n", className.utf8().data());
On 2014/08/11 12:27:57, Jens Lindström wrote:
> Intentional to not use LOG_ERROR_ALWAYS() here?

Done.

https://codereview.chromium.org/454773002/diff/60001/Source/bindings/core/v8/...
File Source/bindings/core/v8/PrivateScriptRunner.js (right):

https://codereview.chromium.org/454773002/diff/60001/Source/bindings/core/v8/...
Source/bindings/core/v8/PrivateScriptRunner.js:85: if
(installedClasses[className] === undefined)
On 2014/08/11 12:27:57, Jens Lindström wrote:
> "!(className in installedClasses)" would be a more direct way to check if
we've
> stored a class already.

Done.

https://codereview.chromium.org/454773002/diff/60001/Source/core/core_generat...
File Source/core/core_generated.gyp (right):

https://codereview.chromium.org/454773002/diff/60001/Source/core/core_generat...
Source/core/core_generated.gyp:158: ],
On 2014/08/11 12:27:57, Jens Lindström wrote:
> Since you were fixing indentation already: deindent this and following lines
> too?

Done.

[haraken, 2014-08-11 12:41:43]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-08-11 12:42:23]

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

[commit-bot: I haz the power, 2014-08-11 12:56:15]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_db...)
  android_blink_compile_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/android_blink_compile_re...)
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/2...)
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)
  linux_chromium_gn_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_chromium_gn_rel/bu...)
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19281)
  win_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/bu...)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/22049)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/39948)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/45193)

[commit-bot: I haz the power, 2014-08-11 12:58:51]

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

[commit-bot: I haz the power, 2014-08-11 12:58:52]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_chromium_gn_rel/bu...)
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/19284)

[haraken, 2014-08-11 12:59:51]

jl@: Sorry, I forgot to git add PartialPrivateScriptTest.{h,js,idl} to the CL.
Would you take another quick look?

[Jens Widell, 2014-08-11 13:12:10]

On 2014/08/11 12:59:51, haraken wrote:
> jl@: Sorry, I forgot to git add PartialPrivateScriptTest.{h,js,idl} to the CL.
> Would you take another quick look?

LGTM

Didn't notice they had gone missing. Sorry about that.

[haraken, 2014-08-11 13:32:39]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-08-11 13:33:48]

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

[commit-bot: I haz the power, 2014-08-11 14:37:01]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/bu...)

[commit-bot: I haz the power, 2014-08-11 14:55:30]

Change committed as 179960