[hashtable] Don't add PropertyCell to GlobalDictionary too early

This is a follow-up fix for
  https://chromium-review.googlesource.com/c/v8/v8/+/2292230

In this CL fixes the case when the property cell is added to the
dictionary but the value is not actually stored which leaves
PropertyCell with the hole in the dictionary.

Now the logic for GlobalDictionary matches the logic for
NameDictionary - the property cell is added to the dictionary in
LookupIterator::ApplyTransitionToDataProperty().

Bug: chromium:1104711, chromium:1105383
Change-Id: I56da16d85d13288fbc41fd60dbce556fec5e7d18
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2297472Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68860}

src/objects/lookup.cc

@@ -550,24 +550,14 @@ void LookupIterator::PrepareTransitionToDataProperty(
   if (map->is_dictionary_map()) {
     state_ = TRANSITION;
     if (map->IsJSGlobalObjectMap()) {
-      // Install a property cell.
-      Handle<JSGlobalObject> global = Handle<JSGlobalObject>::cast(receiver);
-      DCHECK(!global->HasFastProperties());
-      Handle<GlobalDictionary> dictionary(global->global_dictionary(isolate_),
-                                          isolate_);
-
       Handle<PropertyCell> cell = isolate_->factory()->NewPropertyCell(name());
-
       DCHECK(cell->value(isolate_).IsTheHole(isolate_));
       DCHECK(!value->IsTheHole(isolate_));
+      // Don't set enumeration index (it will be set during value store).
       property_details_ = PropertyDetails(
           kData, attributes,
           PropertyCell::TypeForUninitializedCell(isolate_, value));
 
-      dictionary = GlobalDictionary::Add(isolate_, dictionary, name(), cell,
-                                         property_details_, &number_);
-      global->set_global_dictionary(*dictionary);
-
       transition_ = cell;
       has_property_ = true;
     } else {
@@ -603,6 +593,21 @@ void LookupIterator::ApplyTransitionToDataProperty(
   holder_ = receiver;
   if (receiver->IsJSGlobalObject(isolate_)) {
     JSObject::InvalidatePrototypeChains(receiver->map(isolate_));
+
+    // Install a property cell.
+    Handle<JSGlobalObject> global = Handle<JSGlobalObject>::cast(receiver);
+    DCHECK(!global->HasFastProperties());
+    Handle<GlobalDictionary> dictionary(global->global_dictionary(isolate_),
+                                        isolate_);
+
+    dictionary =
+        GlobalDictionary::Add(isolate_, dictionary, name(), transition_cell(),
+                              property_details_, &number_);
+    global->set_global_dictionary(*dictionary);
+
+    // Reload details containing proper enumeration index value.
+    property_details_ = transition_cell()->property_details();
+    has_property_ = true;
     state_ = DATA;
     return;
   }

test/mjsunit/regress/regress-crbug-1104711.js

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+__proto__ = {x:1};
+try {
+  foo();
+} catch (e) {}
+function foo() {
+  'use strict';
+  x = 42;
+}
+x = 42;
+foo();

test/mjsunit/regress/regress-crbug-1105383.js

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+'use strict';
+Object.defineProperty(
+    this.__proto__, "boom",
+    { value:153, writable:true, configurable: true, enumerable:false });
+
+var good = 1;
+try {
+  boom = 42;
+} catch (e) {}
+
+var properties = Object.getOwnPropertyNames(this);
+
+assertTrue(properties.findIndex(e => e == "good") >= 0);
+assertEquals(properties.findIndex(e => e == "boom"), -1);