Reviewers: rossberg,

Description:
Tighten & check the BookmarkScope API contract a bit.

The real fix for crbug.com/510825 is in Blink (crrev.com/1286883004).
When debugging, I first suspected abuse of the BookmarkScope and added
additional assertions to check proper use. It turned out that didn't
actually have anything to do with the bug in question, but IMHO it's
still a useful cleanup. This should not actually change any behaviour.

R=rossb...@chromium.org
BUG=chromium:510825

Please review this at https://codereview.chromium.org/1302893002/

Base URL: https://chromium.googlesource.com/v8/v8.git@master

Affected files (+24, -7 lines):
  M src/parser.cc
  M src/scanner.h


Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index 301bda94c4011987aff1a704a5f2e6f84d79c137..aeb9a3de0589439060bf134d21ec5831819021a6 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -4110,7 +4110,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
                            &expected_property_count, /*CHECK_OK*/ ok,
                            maybe_bookmark);

-      if (bookmark.HasBeenReset()) {
+      if (maybe_bookmark && maybe_bookmark->HasBeenReset()) {
         // Trigger eager (re-)parsing, just below this block.
         is_lazily_parsed = false;

Index: src/scanner.h
diff --git a/src/scanner.h b/src/scanner.h
index 92588905ad609b7ea64d8c5c7c2007d3e55d409b..2afa5b27c1a61315ddab75a5c64e35929c1d929b 100644
--- a/src/scanner.h
+++ b/src/scanner.h
@@ -359,18 +359,35 @@ class Scanner {
   // Scoped helper for a re-settable bookmark.
   class BookmarkScope {
    public:
-    explicit BookmarkScope(Scanner* scanner) : scanner_(scanner) {
+    explicit BookmarkScope(Scanner* scanner)
+        : scanner_(scanner), my_bookmark_(false) {
       DCHECK_NOT_NULL(scanner_);
     }
-    ~BookmarkScope() { scanner_->DropBookmark(); }
+    ~BookmarkScope() {
+      if (my_bookmark_) scanner_->DropBookmark();
+    }

-    bool Set() { return scanner_->SetBookmark(); }
-    void Reset() { scanner_->ResetToBookmark(); }
-    bool HasBeenSet() { return scanner_->BookmarkHasBeenSet(); }
-    bool HasBeenReset() { return scanner_->BookmarkHasBeenReset(); }
+    bool Set() {
+      DCHECK(!my_bookmark_);
+      my_bookmark_ = scanner_->SetBookmark();
+      return my_bookmark_;
+    }
+    void Reset() {
+      DCHECK(my_bookmark_);
+      scanner_->ResetToBookmark();
+    }
+    bool HasBeenSet() {
+      DCHECK(my_bookmark_);
+      return scanner_->BookmarkHasBeenSet();
+    }
+    bool HasBeenReset() {
+      DCHECK(my_bookmark_);
+      return scanner_->BookmarkHasBeenReset();
+    }

    private:
     Scanner* scanner_;
+    bool my_bookmark_;  // Did this BookmarkScope set the current bookmark?

     DISALLOW_COPY_AND_ASSIGN(BookmarkScope);
   };


--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to