Explore the possibility of implementing the CSS Region interface.

Source/bindings/v8/custom/V8WebKitNamedFlowCustom.cpp

+/*
+ * Copyright (C) 2013 Adobe Systems Incorporated. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer.
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDER "AS IS" AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
+ * OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
+ * TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "V8WebKitNamedFlow.h"
+
+#include "V8HTMLElement.h"
+#include "V8PseudoElement.h"
+

[abarth-chromium, 2013/11/14 07:55:15]

no need for this blank line

+#include "bindings/v8/V8Binding.h"
+#include "core/dom/NamedFlow.h"
+#include "core/rendering/RenderRegion.h"
+

[abarth-chromium, 2013/11/14 07:55:15]

ditto

+#include "wtf/RefPtr.h"
+#include "wtf/Vector.h"
+
+namespace WebCore {
+
+static v8::Handle<v8::Value> toV8Array(Vector<const RenderRegion*>& regionsList, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)

[abarth-chromium, 2013/11/14 07:55:15]

regionsList -> regions

+{
+    v8::Local<v8::Array> result = v8::Array::New(regionsList.size());

[abarth-chromium, 2013/11/14 07:55:15]

If script has replaced the global Array constructor, will this line of code
cause JavaScript to execute?

If it's possible to enter the site's script at any time during this function,
we're in deep trouble because nothing is keeping the RenderRegion* in the Vector
alive.  Other than this line, I don't see anything else that triggers JS
execution, but this approach is very fragile.  Instead, you probably want the
C++ DOM API to return a DOM concept (not a RenderTree concept) because DOM
concepts can be usually be retained by RefPtrs whereas RenderTree concepts
cannot.  In your earlier iteration, you wrapped the RenderRegion* in a
RefCounted container, but that's just window dressing around the same problem. 
You can solve this problem, it's just some ugly code.

[abucur, 2013/11/15 07:15:14]

Good point, I agree it's a problem if this code execute scripts, though I
suspect it should be possible for the engine to build a "default" type of array.
It doesn't sound very safe (and neither spec compliant) for the engine to return
user defined data structures when a Web API is called. It somehow exposes the
browser internals.

I'll break this dependency and write a test for it in the final patch.

+
+    int index = 0;
+    Vector<const RenderRegion*>::const_iterator end = regionsList.end();
+    for (Vector<const RenderRegion*>::const_iterator iter = regionsList.begin(); iter != end; ++iter) {
+        const RenderRegion* region = *iter;
+        Element* generator = region->element();
+        if (!generator)
+            continue;

[abarth-chromium, 2013/11/14 07:55:15]

When can this happen?

[abucur, 2013/11/15 07:15:14]

At this moment, never. It's better to make it an ASSERT.

+        if (generator->isHTMLElement())
+            result->Set(v8::Integer::New(index++, isolate), toV8(toHTMLElement(generator), creationContext, isolate));
+        else if (generator->isPseudoElement())
+            result->Set(v8::Integer::New(index++, isolate), toV8(toPseudoElement(generator), creationContext, isolate));
+        else
+            ASSERT_NOT_REACHED();

[abarth-chromium, 2013/11/14 07:55:15]

Why can't it be a non-HTML element?

[abucur, 2013/11/15 07:15:14]

At this moment only blocks can become regions (I suspected this implies
HTMLElement, please correct me if I'm wrong). The ASSERT_NOT_REACHED is there to
help implementors add a new toV8 case for future region types that may appear.
If they forgot to edit this file, running the OM tests will cause this to
trigger.

+    }
+
+    return result;
+}
+
+void V8WebKitNamedFlow::getRegionsMethodCustom(const v8::FunctionCallbackInfo<v8::Value>& args)
+{
+    NamedFlow* imp = V8WebKitNamedFlow::toNative(args.Holder());
+    Vector<const RenderRegion*> regionsList = imp->getRegions();
+    
+    v8SetReturnValue(args, toV8Array(regionsList, args.Holder(), args.GetIsolate()));
+}
+
+void V8WebKitNamedFlow::getRegionsByContentMethodCustom(const v8::FunctionCallbackInfo<v8::Value>& args)
+{
+    NamedFlow* imp = V8WebKitNamedFlow::toNative(args.Holder());
+
+    V8TRYCATCH_VOID(Node*, node, V8Node::HasInstance(args[1], args.GetIsolate(), worldType(args.GetIsolate())) ? V8Node::toNative(v8::Handle<v8::Object>::Cast(args[1])) : 0);

[abarth-chromium, 2013/11/14 07:55:15]

What about args[0]?  What if there are zero or one argument?

[abucur, 2013/11/15 07:15:14]

Oh, I don't know why I had the feeling args[0] was the Function object. I'll
change it in the real implementation.

+    Vector<const RenderRegion*> regionsList = imp->getRegionsByContent(node);
+
+    v8SetReturnValue(args, toV8Array(regionsList, args.Holder(), args.GetIsolate()));
+}
+
+} // namespace WebCore