Skip to content

Ensure that viewer handshake always happens quickly.#6765

Merged
cramforce merged 5 commits intoampproject:masterfrom
cramforce:early-chunks
Dec 21, 2016
Merged

Ensure that viewer handshake always happens quickly.#6765
cramforce merged 5 commits intoampproject:masterfrom
cramforce:early-chunks

Conversation

@cramforce
Copy link
Copy Markdown
Member

This fixes #6728. THe problem was that chunking can delay the handshake with the viewer for a very long time. During this time the viewer cannot tell the doc that it is now visible–and visibility triggers chunking to go into fast mode. With this change the aggressive chunking mode is only triggered when the doc has gotten into a state where it can communicate with the viewer; and the viewer communication code itself is never delayed in a chunk.

@cramforce cramforce requested a review from dvoytenko December 20, 2016 23:45
@cramforce cramforce changed the title Ensure that viewer handshake always happens quicklu. Ensure that viewer handshake always happens quickly. Dec 20, 2016
}
});
};
if (typeof fnOrStruct == 'function' || fnOrStruct.p == 'high') {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you leave a comment about the the .p and how it is set

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is in the typedef, but will enhance the comment.

This fixes ampproject#6728. THe problem was that chunking can delay the handshake with the viewer for a very long time. During this time the viewer cannot tell the doc that it is now visible–and visibility triggers chunking to go into fast mode. With this change the aggressive chunking mode is only triggered when the doc has gotten into a state where it can communicate with the viewer; and the viewer communication code itself is never delayed in a chunk.
var e = extensions[key];
buildExtension(e.name, e.version, e.hasCss, options, e.extraGlobs);
var o = Object.assign({}, options);
o = Object.assign(o, e);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the double assign? var o = Object.assign({}, options, e)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly though nesting assigns is kind of unreadable. Happy to change.

Copy link
Copy Markdown
Contributor

@dvoytenko dvoytenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good on my side. Just one small request.

});
};
if (typeof fnOrStruct == 'function' || fnOrStruct.p == 'high') {
// "High priority" extensions do not go through chunking.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make a note why function is considered "high" priority.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@cramforce cramforce merged commit 03ee950 into ampproject:master Dec 21, 2016
@cramforce cramforce deleted the early-chunks branch December 21, 2016 20:15
Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
This fixes ampproject#6728. THe problem was that chunking can delay the handshake with the viewer for a very long time. During this time the viewer cannot tell the doc that it is now visible–and visibility triggers chunking to go into fast mode. With this change the aggressive chunking mode is only triggered when the doc has gotten into a state where it can communicate with the viewer; and the viewer communication code itself is never delayed in a chunk.
Lith pushed a commit to Lith/amphtml that referenced this pull request Dec 22, 2016
This fixes ampproject#6728. THe problem was that chunking can delay the handshake with the viewer for a very long time. During this time the viewer cannot tell the doc that it is now visible–and visibility triggers chunking to go into fast mode. With this change the aggressive chunking mode is only triggered when the doc has gotten into a state where it can communicate with the viewer; and the viewer communication code itself is never delayed in a chunk.
torch2424 pushed a commit to torch2424/amphtml that referenced this pull request Jan 3, 2017
This fixes ampproject#6728. THe problem was that chunking can delay the handshake with the viewer for a very long time. During this time the viewer cannot tell the doc that it is now visible–and visibility triggers chunking to go into fast mode. With this change the aggressive chunking mode is only triggered when the doc has gotten into a state where it can communicate with the viewer; and the viewer communication code itself is never delayed in a chunk.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chunking must be less aggressive until we get to a state where we can be transitioned out of pre-render

4 participants