Tighten & check the BookmarkScope API contract a bit.

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=rossberg@chromium.org
BUG=chromium:510825

[rossberg, 2015-08-20 15:45:12]

lgtm

https://codereview.chromium.org/1302893002/diff/1/src/scanner.h
File src/scanner.h (right):

https://codereview.chromium.org/1302893002/diff/1/src/scanner.h#newcode367
src/scanner.h:367: if (my_bookmark_) scanner_->DropBookmark();
Hm, this _does_ change the semantics, doesn't it? It looks more correct, though.

[vogelheim, 2015-08-20 16:26:31]

https://codereview.chromium.org/1302893002/diff/1/src/scanner.h
File src/scanner.h (right):

https://codereview.chromium.org/1302893002/diff/1/src/scanner.h#newcode367
src/scanner.h:367: if (my_bookmark_) scanner_->DropBookmark();
On 2015/08/20 15:45:12, rossberg wrote:
> Hm, this _does_ change the semantics, doesn't it? It looks more correct,
though.

Yes, good observation. But I think the case where this makes a difference
shouldn't happen in practice, so it shouldn't change behaviour overall.

And if the case does happen, this should fix a bug. :)  This is what I
originally meant to implement.

[rossberg, 2015-08-20 16:37:32]

On 2015/08/20 16:26:31, vogelheim wrote:
> https://codereview.chromium.org/1302893002/diff/1/src/scanner.h
> File src/scanner.h (right):
> 
> https://codereview.chromium.org/1302893002/diff/1/src/scanner.h#newcode367
> src/scanner.h:367: if (my_bookmark_) scanner_->DropBookmark();
> On 2015/08/20 15:45:12, rossberg wrote:
> > Hm, this _does_ change the semantics, doesn't it? It looks more correct,
> though.
> 
> Yes, good observation. But I think the case where this makes a difference
> shouldn't happen in practice, so it shouldn't change behaviour overall.
> 
> And if the case does happen, this should fix a bug. :)  This is what I
> originally meant to implement.

Would be good to know if it is supposed to happen. :) If not, this could perhaps
be a DCHECK.