Blink-in-JS: Implement internal APIs exposed only to private scripts

Blink-in-JS: Implement internal APIs exposed only to private scripts

This is the final part of the core infrastructure of Blink-in-JS. This CL implements internal APIs exposed only to private scripts. They are not visible from user's JavaScript. They are needed for moving XSLT and editing/ to Blink-in-JS.

Here is a summary of the IDL extended attributes around private scripts:

interface X {
  aaa();
  [ImplementedInPrivateScript] bbb();
  [OnlyExposedToPrivateScript] ccc();
  [ImplementedInPrivateScript, OnlyExposedToPrivateScript] ddd();
};

- aaa() is a normal DOM method implemented in C++. aaa() is exposed to both user's JavaScript and private scripts.

- bbb() is implemented in a private script. bbb() is exposed to both user's JavaScript and private scripts.

- ccc() is implemented in C++. ccc() is exposed only to private scripts.

- ddd() is implemented in a private script. ddd() is not exposed to user's JavaScript nor private scripts. V8X::ddd() is exposed to C++ so that Blink can invoke ddd() from C++.

In other words, we essentially need the following four kinds of APIs, and the two IDL attributes control the kind of each API.

- user's JS & private scripts => C++ (e.g., aaa())
- user's JS & private scripts => private scripts (e.g., bbb())
- private scripts => C++ (e.g., ccc())
- C++ => private scripts (e.g., ddd())

This design might be a bit confusing but this is the least confusing design we can come up with at the moment.

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

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

[haraken, 2014-07-25 07:11:37]

bashi@: PTAL at the IDL compiler.

tasak@: PTAL at the overall design. abarth@ is on vacation and jl@ is supposed
to be on vacation.

abarth@, arv@, adamk@, jl@: FYI.

[bashi, 2014-07-25 07:57:42]

IDL compiler LGTM.

https://codereview.chromium.org/413393003/diff/1/Source/bindings/templates/in...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/413393003/diff/1/Source/bindings/templates/in...
Source/bindings/templates/interface.cpp:734: {% if not
attribute.is_implemented_in_private_script or not
attribute.only_exposed_to_private_script %}
nit: We check the same condition four times. We may want to introduce a new flag
to |attribute| for these check.

https://codereview.chromium.org/413393003/diff/1/Source/bindings/templates/in...
Source/bindings/templates/interface.cpp:754: {% if not
attribute.is_implemented_in_private_script or not
attribute.only_exposed_to_private_script %}
nit: Can we merge this to the above if?

https://codereview.chromium.org/413393003/diff/1/Source/bindings/templates/in...
Source/bindings/templates/interface.cpp:786: {% if not
method.is_implemented_in_private_script or not
method.only_exposed_to_private_script %}
nit: We check this condition two times.

[haraken, 2014-07-25 08:26:59]

Thanks for review.

https://codereview.chromium.org/413393003/diff/1/Source/bindings/templates/in...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/413393003/diff/1/Source/bindings/templates/in...
Source/bindings/templates/interface.cpp:734: {% if not
attribute.is_implemented_in_private_script or not
attribute.only_exposed_to_private_script %}
On 2014/07/25 07:57:42, bashi1 wrote:
> nit: We check the same condition four times. We may want to introduce a new
flag
> to |attribute| for these check.

Done.

https://codereview.chromium.org/413393003/diff/1/Source/bindings/templates/in...
Source/bindings/templates/interface.cpp:754: {% if not
attribute.is_implemented_in_private_script or not
attribute.only_exposed_to_private_script %}
On 2014/07/25 07:57:42, bashi1 wrote:
> nit: Can we merge this to the above if?

Done.

https://codereview.chromium.org/413393003/diff/1/Source/bindings/templates/in...
Source/bindings/templates/interface.cpp:786: {% if not
method.is_implemented_in_private_script or not
method.only_exposed_to_private_script %}
On 2014/07/25 07:57:42, bashi1 wrote:
> nit: We check this condition two times.

Done.

[Jens Widell, 2014-07-25 09:45:36]

https://codereview.chromium.org/413393003/diff/20001/Source/bindings/scripts/...
File Source/bindings/scripts/v8_attributes.py (right):

https://codereview.chromium.org/413393003/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_attributes.py:147: 'should_be_exposed_to_script': not
is_implemented_in_private_script or not is_only_exposed_to_private_script,
Might be a matter of taste, but I'd prefer "not(A and B)" for legibility.

https://codereview.chromium.org/413393003/diff/20001/Source/bindings/template...
File Source/bindings/templates/interface_base.cpp (right):

https://codereview.chromium.org/413393003/diff/20001/Source/bindings/template...
Source/bindings/templates/interface_base.cpp:65: {% if not
attribute.is_implemented_in_private_script or not
attribute.only_exposed_to_private_script %}
Use the new flag here too?

https://codereview.chromium.org/413393003/diff/20001/Source/bindings/template...
Source/bindings/templates/interface_base.cpp:94: {% if not
method.is_implemented_in_private_script or not
method.only_exposed_to_private_script %}
And here.

[haraken, 2014-07-25 09:58:19]

https://codereview.chromium.org/413393003/diff/20001/Source/bindings/scripts/...
File Source/bindings/scripts/v8_attributes.py (right):

https://codereview.chromium.org/413393003/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/v8_attributes.py:147: 'should_be_exposed_to_script': not
is_implemented_in_private_script or not is_only_exposed_to_private_script,
On 2014/07/25 09:45:36, Jens Lindström wrote:
> Might be a matter of taste, but I'd prefer "not(A and B)" for legibility.

Done.

https://codereview.chromium.org/413393003/diff/20001/Source/bindings/template...
File Source/bindings/templates/interface_base.cpp (right):

https://codereview.chromium.org/413393003/diff/20001/Source/bindings/template...
Source/bindings/templates/interface_base.cpp:65: {% if not
attribute.is_implemented_in_private_script or not
attribute.only_exposed_to_private_script %}
On 2014/07/25 09:45:36, Jens Lindström wrote:
> Use the new flag here too?

Done.

https://codereview.chromium.org/413393003/diff/20001/Source/bindings/template...
Source/bindings/templates/interface_base.cpp:94: {% if not
method.is_implemented_in_private_script or not
method.only_exposed_to_private_script %}
On 2014/07/25 09:45:36, Jens Lindström wrote:
> And here.

Done.

[Jens Widell, 2014-07-25 10:10:37]

Maybe add a method (and an attribute) corresponding to the ccc() variant to
TestObject.idl?

[haraken, 2014-07-25 10:45:31]

On 2014/07/25 10:10:37, Jens Lindström wrote:
> Maybe add a method (and an attribute) corresponding to the ccc() variant to
> TestObject.idl?

Done.

[Jens Widell, 2014-07-25 11:09:45]

(I don't have source code access currently, so haven't reviewed as thoroughly as
I would have otherwise, but aside from that) LGTM.

https://codereview.chromium.org/413393003/diff/60001/Source/bindings/template...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/413393003/diff/60001/Source/bindings/template...
Source/bindings/templates/interface.cpp:1070: "{{method.name}}",
{{method_callback}}, {{method_callback_for_main_world}}, {{method.length}},
V8DOMConfiguration::ExposedToAllScripts,
Should we assert somewhere that this code path isn't triggered for a method with
[OnlyExposeToPrivateScript]?

[haraken, 2014-07-25 13:22:16]

https://codereview.chromium.org/413393003/diff/60001/Source/bindings/template...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/413393003/diff/60001/Source/bindings/template...
Source/bindings/templates/interface.cpp:1070: "{{method.name}}",
{{method_callback}}, {{method_callback_for_main_world}}, {{method.length}},
V8DOMConfiguration::ExposedToAllScripts,
On 2014/07/25 11:09:45, Jens Lindström wrote:
> Should we assert somewhere that this code path isn't triggered for a method
with
> [OnlyExposeToPrivateScript]?

Instead of adding asserts, I just supported [OnlyExposedToPrivateScript] in
these structures as well.

[haraken, 2014-07-25 13:22:33]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-07-25 13:23:20]

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

[commit-bot: I haz the power, 2014-07-25 13:23:49]

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

[commit-bot: I haz the power, 2014-07-25 13:23:49]

Failed to apply patch for Source/core/testing/PrivateScriptTest.js:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/testing/PrivateScriptTest.js
  Hunk #1 FAILED at 129.
  1 out of 1 hunk FAILED -- saving rejects to file
Source/core/testing/PrivateScriptTest.js.rej

Patch:       Source/core/testing/PrivateScriptTest.js
Index: Source/core/testing/PrivateScriptTest.js
diff --git a/Source/core/testing/PrivateScriptTest.js
b/Source/core/testing/PrivateScriptTest.js
index
88050ca7b3f59b7759a76dfd6b016427f2edee13..c6bb4550b91f4efe7bbde912e0ca2fa552e122f2
100644
--- a/Source/core/testing/PrivateScriptTest.js
+++ b/Source/core/testing/PrivateScriptTest.js
@@ -129,5 +129,14 @@ installClass("PrivateScriptTest", function(global) {
         set: function(value) { this.m_stringAttributeForPrivateScriptOnly =
value; }
     });
 
+    InternalsPrototype.addIntegerImplementedInCPP = function(value1, value2) {
+        return this.addIntegerImplementedInCPPForPrivateScriptOnly(value1,
value2);
+    }
+
+    Object.defineProperty(InternalsPrototype,
"stringAttributeImplementedInCPP", {
+        get: function() { return
this.m_stringAttributeImplementedInCPPForPrivateScriptOnly; },
+        set: function(value) {
this.m_stringAttributeImplementedInCPPForPrivateScriptOnly = value; }
+    });
+
     return InternalsPrototype;
 });

[haraken, 2014-07-25 13:25:46]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-07-25 13:26:34]

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

[commit-bot: I haz the power, 2014-07-25 14:31:32]

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

[commit-bot: I haz the power, 2014-07-25 15:34:12]

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

[commit-bot: I haz the power, 2014-07-25 15:34:13]

Try jobs failed on following builders:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)

[haraken, 2014-07-25 15:35:58]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-07-25 15:36:48]

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

[haraken, 2014-07-25 16:29:02]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-07-25 16:29:14]

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

[haraken, 2014-07-25 16:34:10]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-07-25 16:34:21]

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

[commit-bot: I haz the power, 2014-07-25 17:37:18]

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

[commit-bot: I haz the power, 2014-07-25 18:22:29]

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

[commit-bot: I haz the power, 2014-07-25 18:22:30]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/18988)

[haraken, 2014-07-25 22:46:07]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-07-25 22:46:58]

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

[commit-bot: I haz the power, 2014-07-26 00:03:07]

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

[commit-bot: I haz the power, 2014-07-26 00:52:57]

Change committed as 178974