bindings: Refactors BindingSecurity::shouldAllowAccessToXXX.

third_party/WebKit/Source/bindings/core/v8/BindingSecurity.h

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 13 matching lines @@
  * 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.
  */
 
 #ifndef BindingSecurity_h
 #define BindingSecurity_h
 
-// FIXME: The LocalFrame include should not be necessary, clients should be including it where they use it.
 #include "core/CoreExport.h"
-#include "core/frame/LocalFrame.h"
 #include "wtf/Allocator.h"
 #include <v8.h>
 
 namespace blink {
 
+class DOMWindow;
+class EventTarget;
+class ExceptionState;
+class Frame;
 class LocalDOMWindow;
-class ExceptionState;
+class Location;
 class Node;
 
 enum SecurityReportingOption {
     DoNotReportSecurityError,
     ReportSecurityError,
 };
 
-class BindingSecurity {
+class CORE_EXPORT BindingSecurity {
     STATIC_ONLY(BindingSecurity);
 public:
-    // Check the access to the return value.
-    static bool shouldAllowAccessToNode(v8::Isolate*, LocalDOMWindow* accessingWindow, Node*, SecurityReportingOption);
-    static bool shouldAllowAccessToNode(v8::Isolate*, LocalDOMWindow* accessingWindow, Node*, ExceptionState&);
+    // Check if the caller (|accessingWindow|) is allowed to access to the JS

[haraken, 2015/11/22 14:55:00]

to access the JS receiver object

[Yuki, 2015/11/24 08:44:12]

Done.

+    // receiver object (|target|), where the receiver object is the JS object
+    // for which the DOM attribute or DOM operation is being invoked (in the
+    // form of receiver.domAttr or receiver.domOp()).
+    // Note that only Window and Location objects are cross-origin accessible
+    // and that EventTarget interface is the parent interface of Window
+    // interface.  So the receiver object must be of type DOMWindow,
+    // EventTarget, or Location.
+    // DOMWindow
+    static bool shouldAllowAccessTo(v8::Isolate*, const LocalDOMWindow* accessingWindow, const DOMWindow* target, ExceptionState&);
+    static bool shouldAllowAccessTo(v8::Isolate*, const LocalDOMWindow* accessingWindow, const DOMWindow* target, SecurityReportingOption);
+    // EventTarget (as the parent of DOMWindow)
+    static bool shouldAllowAccessTo(v8::Isolate*, const LocalDOMWindow* accessingWindow, const EventTarget* target, ExceptionState&);  // NOLINT(readability/parameter_name)

[haraken, 2015/11/22 14:55:00]

Remove the NOLINT.

[Yuki, 2015/11/24 08:44:12]

I need this NOLINT, otherwise the lint tool warns that "EventTarget* target" is
redundant and we should remove "target" argument identifier.

+    // Location
+    static bool shouldAllowAccessTo(v8::Isolate*, const LocalDOMWindow* accessingWindow, const Location* target, ExceptionState&);
+    static bool shouldAllowAccessTo(v8::Isolate*, const LocalDOMWindow* accessingWindow, const Location* target, SecurityReportingOption);
+    // Check if the caller (|accessingWindow|) is allowed to access to the frame
+    // (|target|) rather than to a DOMWindow/EventTarget/Location.

[haraken, 2015/11/22 14:55:00]

Isn't this comment redundant? This is the same one as line 56.

[Yuki, 2015/11/24 08:44:12]

Removed.

+    // Prefer to use the previous overloads instead of falling back to using
+    // Frame*.
+    static bool shouldAllowAccessToFrame(v8::Isolate*, const LocalDOMWindow* accessingWindow, const Frame* target, SecurityReportingOption); // OBSOLETE
 
-    // Check the access to the receiver.
-    CORE_EXPORT static bool shouldAllowAccessToFrame(v8::Isolate*, LocalDOMWindow* accessingWindow, Frame*, SecurityReportingOption = ReportSecurityError);
-    CORE_EXPORT static bool shouldAllowAccessToFrame(v8::Isolate*, LocalDOMWindow* accessingWindow, Frame*, ExceptionState&);
+    // Check if the caller (|accessingWindow|) is allowed to access to the JS
+    // returned object (|target|), where the returned object is the JS object
+    // which is returned as a result of invoking a DOM attribute or DOM
+    // operation (in the form of
+    //   var x = receiver.domAttr // or receiver.domOp()
+    // where |x| is the returned object).

[haraken, 2015/11/22 14:55:00]

Ditto.

[Yuki, 2015/11/24 08:44:12]

Sorry, ditto for what?

[haraken, 2015/11/24 08:47:48]

The comment in line 78 - 83 is similar to the one in line 56 - 59.

[Yuki, 2015/11/24 08:51:13]

Yes, similar, but it seems better to clarify the difference between "receiver"
and "return value" and/or what they are.  I'd prefer leaving the comment as is.

+    // See window.frameElement for example, which may return a frame object.
+    // The object returned from window.frameElement must be the same origin if
+    // it's not null.
+    // Node
+    static bool shouldAllowAccessTo(v8::Isolate*, const LocalDOMWindow* accessingWindow, const Node* target, ExceptionState&);
+    static bool shouldAllowAccessTo(v8::Isolate*, const LocalDOMWindow* accessingWindow, const Node* target, SecurityReportingOption);
 };
 
-}
+} // namespace blink
 
 #endif