HTML API: Stop counting no-op seek operations against the quota.

[dmsnell]

The HTML Tag Processor limits how many times seek() may be called. This is in place to guard against accidental infinite loops introduced by code which might bounce between bookmarks in an unintentional cycle.

Some code might call seek() on a bookmark where the internal cursor already is. Since a seek to the current locations involves essentially no processing, and is less likely to be the result of an unseen cycle, the HTML Tag Processor should ignore these calls against the quota.

[dmsnell]

After proposing this, I'm now wondering if it's wrong to open up the quote because I think it's also similarly likely that an accidental infinite loop could involve no-op seeks. Perhaps we want a different getter to report if the processor has moved from a bookmark?

<?php
if ( ! $processor->is_at_bookmark( 'here' ) ) {
        $processor->seek( 'here' );
}

cc: @westonruter @jonsurrell

Trac ticket: Core-62085

[westonruter]

I don't know. It seems like this may be doing too much to attempt to prevent a developer from committing a basic logic error in their code.

[davidbaumwald]

With 6.7 Beta 1 releasing in a few hours, this is being moved to 6.8 given recent momentum towards a resolution.

[audrasjb]

As per today's bug scrub:
Hello @dmsnell @westonruter, how can we move this towards a resolution? Or should we close it as wontfix/maybelater for now?
Moving to Future Release pending a decision.

[westonruter]

I think this is still relevant. To avoid reaching the seek quota, I'm currently having to keep track of whether the cursor is moved:

​https://github.com/WordPress/performance/blob/6459571471b26aee4f63f00e2ba9dfe6f5ce2f39/plugins/optimization-detective/class-od-html-tag-processor.php#L237-L245
​https://github.com/WordPress/performance/blob/6459571471b26aee4f63f00e2ba9dfe6f5ce2f39/plugins/optimization-detective/class-od-html-tag-processor.php#L324

This is so that the code only calls seek() if it needs to move:

​https://github.com/WordPress/performance/blob/6459571471b26aee4f63f00e2ba9dfe6f5ce2f39/plugins/optimization-detective/optimization.php#L301-L305

If, however, the cursor was already at the same location as the given bookmark, then seek() should just be a no-op.

[westonruter]

I can confirm that with the patch applied that I can remove all cursor move counting that I had to do in Optimization Detective: ​https://github.com/WordPress/performance/pull/1861

[westonruter]

I move to commit this.

[westonruter]

In 59812:

HTML API: Stop counting no-op seek operations against the max seek count.

This allows seek() to be freely called when the current cursor at the provided bookmark.

Props dmsnell, jonsurrell, westonruter.
Fixes #62085.

Committed in r59812.