Make functions on the built-in object non-writable.

src/runtime.cc

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index 802fd6845d0204cabeba1de5cd070261ebbf3e66..b8885124d95951c1963795ddf3dc027b0660e03d 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -1149,16 +1149,19 @@ static Failure* ThrowRedeclarationError(Isolate* isolate,
 
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_DeclareGlobals) {
-  ASSERT(args.length() == 4);
+  ASSERT(args.length() == 3);
   HandleScope scope(isolate);
   Handle<GlobalObject> global = Handle<GlobalObject>(
       isolate->context()->global());
 
   Handle<Context> context = args.at<Context>(0);
   CONVERT_ARG_CHECKED(FixedArray, pairs, 1);
-  bool is_eval = args.smi_at(2) == 1;
-  StrictModeFlag strict_mode = static_cast<StrictModeFlag>(args.smi_at(3));
-  ASSERT(strict_mode == kStrictMode || strict_mode == kNonStrictMode);
+  CONVERT_SMI_ARG_CHECKED(flags, 2);
+  bool is_eval = (flags & kDeclareGlobalsEvalFlag) != 0;

[Kevin Millikin (Chromium), 2011/09/01 07:58:24]

These flags and 'base' below are declared a away from where they're used and
this is a big function.  You might consider moving them down toward their use.

If you keep the declarations up here, can you make them const so the reader
knows he doesn't have to look at all the intervening code between definition and
use?

I also think it's clearer to make strict_mode a bool and only convert it to the
enum at its single use.

[Lasse Reichstein, 2011/09/01 09:26:58]

Done.
The entire declaration of strict_mode is moved to just before it's used.

+  StrictModeFlag strict_mode =
+      ((flags & kDeclareGlobalsStrictModeFlag) != 0) ? kStrictMode
+                                                     : kNonStrictMode;
+  bool is_native = (flags & kDeclareGlobalsNativeFlag) != 0;
 
   // Compute the property attributes. According to ECMA-262, section
   // 13, page 71, the property must be read-only and
@@ -1177,7 +1180,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DeclareGlobals) {
     // assign to it when evaluating the assignment for "const x =
     // <expr>" the initial value is the hole.
     bool is_const_property = value->IsTheHole();
-
+    bool is_function_declaration = false;
     if (value->IsUndefined() || is_const_property) {
       // Lookup the property in the global object, and don't set the
       // value of the variable if the property is already there.
@@ -1226,6 +1229,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DeclareGlobals) {
         }
       }
     } else {
+      is_function_declaration = true;
       // Copy the function and update its context. Use it as value.
       Handle<SharedFunctionInfo> shared =
           Handle<SharedFunctionInfo>::cast(value);
@@ -1239,7 +1243,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DeclareGlobals) {
     LookupResult lookup;
     global->LocalLookup(*name, &lookup);
 
-    PropertyAttributes attributes = is_const_property
+    PropertyAttributes attributes =

[Kevin Millikin (Chromium), 2011/09/01 07:58:24]

Here, you could just get rid of base and have the more direct:

int attr = NONE;
if (!is_eval) attr |= DONT_DELETE;
if (is_const_property || (is_native && is_function_declaration)) attr |=
READ_ONLY;

You can also move it down below the next check that doesn't use the attributes.

[Lasse Reichstein, 2011/09/01 09:26:58]

Good idea. Done!

+        (is_const_property || (is_native && is_function_declaration))
         ? static_cast<PropertyAttributes>(base | READ_ONLY)
         : base;