Issue 4483 - Update page structure when sitekey is received

Issue 4483 - Update page structure when sitekey is received

[kzar, 2016-10-17 10:19:46]

Patch Set 1

I'm not too happy with this solution, it means that the page structure will be
updated twice when a valid sitekey header is received. It also means that the
page structure could be left in a wrong state if a pre-rendered tab which had a
valid sitekey header doesn't end up being committed to. Suggestions welcome, I
can't see how else to do this.

[kzar, 2016-10-17 11:04:46]

Patch Set 2 : Avoid updating the page structure twice

[Sebastian Noack, 2016-10-18 13:20:11]

From the top of my head, I don't have any better idea. I put Wladimir CC since
he recently touched the code, discovering the frame hierarchy. Perhaps he should
have a go on this review as well.

https://codereview.adblockplus.org/29357576/diff/29357584/chrome/ext/backgrou...
File chrome/ext/background.js (right):

https://codereview.adblockplus.org/29357576/diff/29357584/chrome/ext/backgrou...
chrome/ext/background.js:139: let page = new Page({
Nit: This doesn't need to be wrapped.

[kzar, 2016-10-19 12:36:58]

Patch Set 3 : Create page on one line

https://codereview.adblockplus.org/29357576/diff/29357584/chrome/ext/backgrou...
File chrome/ext/background.js (right):

https://codereview.adblockplus.org/29357576/diff/29357584/chrome/ext/backgrou...
chrome/ext/background.js:139: let page = new Page({
On 2016/10/18 13:20:10, Sebastian Noack wrote:
> Nit: This doesn't need to be wrapped.

Done.

[Wladimir Palant, 2016-10-19 15:04:52]

On 2016/10/18 13:20:11, Sebastian Noack wrote:
> From the top of my head, I don't have any better idea. I put Wladimir CC since
> he recently touched the code, discovering the frame hierarchy.

I don't have any bright idea either - this patch won't produce the correct state
reliably but neither will anything else we do. The APIs just don't provide
enough information when prerendered tabs are involved...

[Sebastian Noack, 2016-10-19 15:12:40]

On 2016/10/19 15:04:52, Wladimir Palant wrote:
> On 2016/10/18 13:20:11, Sebastian Noack wrote:
> > From the top of my head, I don't have any better idea. I put Wladimir CC
since
> > he recently touched the code, discovering the frame hierarchy.
> 
> I don't have any bright idea either - this patch won't produce the correct
state
> reliably but neither will anything else we do. The APIs just don't provide
> enough information when prerendered tabs are involved...

So far this issue effects only one domain parking provider, and since it is a
timing issue, it doesn't even happen all the time. If there is no nice and
reliable solution, I could check whether we can just ignore this issue.

[Sebastian Noack, 2016-10-19 16:02:32]

I have a suspicion that this issue might be caused by the fact that we don't
keep track of the whitelisted frame's IDs but of their URL at the time the
sitekey was seen. This was necessary for compatibility with Safari, but once we
stop supporting Safari with the upstream code, we should cleanup this code and
use frame IDs again.

So I guess this workaround is good enough for the time being. LGTM.