Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1871)

Issue 29436567: Issue 3200 - Delay initialization for prerendered documents (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months, 1 week ago by Manish Jethani
Modified:
1 month, 3 weeks ago
Reviewers:
kzar, Sebastian Noack
Base URL:
https://hg.adblockplus.org/adblockpluschrome/
Visibility:
Public.

Description

Issue 3200 - Delay initialization for prerendered documents Lazily instantiate the message proxy for a Page object in the background page; wait until a prerendered document is made visible in the content script.

Patch Set 1 #

Patch Set 2 : Delay composer.ready until document is visible #

Total comments: 4

Patch Set 3 : Address comments to Patch Set 2 #

Total comments: 6

Patch Set 4 : Move common logic into separate function #

Unified diffs Side-by-side diffs Delta from patch set Stats (+61 lines, -17 lines) Patch
M composer.postload.js View 1 2 3 1 chunk +6 lines, -1 line 0 comments Download
M include.preload.js View 1 2 3 1 chunk +24 lines, -1 line 0 comments Download
M safari/ext/background.js View 1 2 2 chunks +31 lines, -15 lines 0 comments Download

Messages

Total messages: 13
Manish Jethani
5 months, 1 week ago (2017-05-11 19:11:53 UTC) #1
Manish Jethani
Patch Set 1
5 months, 1 week ago (2017-05-11 19:15:33 UTC) #2
Manish Jethani
Patch Set 2: Delay composer.ready until document is visible This fixes the issue of the ...
5 months, 1 week ago (2017-05-12 13:33:52 UTC) #3
kzar
Otherwise LGTM https://codereview.adblockplus.org/29436567/diff/29437573/composer.postload.js File composer.postload.js (right): https://codereview.adblockplus.org/29436567/diff/29437573/composer.postload.js#newcode588 composer.postload.js:588: document.addEventListener("visibilitychange", function onVisibilitychange() Nit: Long line. Also ...
5 months ago (2017-05-19 12:33:05 UTC) #4
Manish Jethani
Patch Set 3: Address comments to Patch Set 2 https://codereview.adblockplus.org/29436567/diff/29437573/composer.postload.js File composer.postload.js (right): https://codereview.adblockplus.org/29436567/diff/29437573/composer.postload.js#newcode588 composer.postload.js:588: ...
5 months ago (2017-05-19 23:58:48 UTC) #5
kzar
LGTM
5 months ago (2017-05-20 10:24:00 UTC) #6
Sebastian Noack
Just to clarify, this patch has been tested against the "safari" bookmark, and is supposed ...
5 months ago (2017-05-20 11:19:06 UTC) #7
Manish Jethani
On 2017/05/20 11:19:06, Sebastian Noack wrote: > Just to clarify, this patch has been tested ...
5 months ago (2017-05-20 19:00:58 UTC) #8
Manish Jethani
https://codereview.adblockplus.org/29436567/diff/29442761/composer.postload.js File composer.postload.js (right): https://codereview.adblockplus.org/29436567/diff/29442761/composer.postload.js#newcode586 composer.postload.js:586: if (document.visibilityState == "prerender") On 2017/05/20 11:19:06, Sebastian Noack ...
5 months ago (2017-05-20 19:01:06 UTC) #9
Sebastian Noack
https://codereview.adblockplus.org/29436567/diff/29442761/composer.postload.js File composer.postload.js (right): https://codereview.adblockplus.org/29436567/diff/29442761/composer.postload.js#newcode586 composer.postload.js:586: if (document.visibilityState == "prerender") On 2017/05/20 19:01:06, Manish Jethani ...
5 months ago (2017-05-21 20:31:01 UTC) #10
Manish Jethani
Patch Set 4: Move common logic into separate function Also rebased. https://codereview.adblockplus.org/29436567/diff/29442761/composer.postload.js File composer.postload.js (right): ...
1 month, 3 weeks ago (2017-08-24 14:07:58 UTC) #11
Sebastian Noack
LGTM. kzar, can you have a look as well?
1 month, 3 weeks ago (2017-08-24 14:09:29 UTC) #12
kzar
1 month, 3 weeks ago (2017-08-29 13:07:00 UTC) #13
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5