Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: ext/background.js

Issue 29755575: Issue 6595 - Make sure frame structure gets always updated (Closed)
Patch Set: Created April 18, 2018, 11:16 a.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ext/background.js
===================================================================
--- a/ext/background.js
+++ b/ext/background.js
@@ -259,20 +259,17 @@
{types: ["main_frame", "sub_frame"], urls: ["http://*/*", "https://*/*"]},
["responseHeaders"]);
- browser.webNavigation.onBeforeNavigate.addListener(details =>
+ browser.webNavigation.onCommitted.addListener(details =>
{
- // Since we can only listen for HTTP(S) responses using
- // webRequest.onHeadersReceived we must update the page structure here for
- // other navigations.
- let {url} = details;
- if (!(url.startsWith("http:") ||
- url.startsWith("https:") &&
Sebastian Noack 2018/04/18 11:25:36 The assumption that every document with an HTTP(S)
kzar 2018/05/02 12:06:52 Initially I worried you removed the whole onHeader
Sebastian Noack 2018/05/02 14:32:15 During onBeforeNavigate we don't know yet whether
kzar 2018/05/02 15:26:52 OK, yea that makes sense. I can't think of a bette
Sebastian Noack 2018/05/02 15:34:49 Well, if blob: URLs are opaque to the webRequest A
kzar 2018/05/02 15:46:35 Well if you paste this snippet from my Chromium is
Sebastian Noack 2018/05/02 16:27:55 I tested it, and the onCommitted listener is emitt
- // Chrome doesn't dispatch webRequest.onHeadersReceived
- // for Web Store URLs.
- // https://crrev.com/76882bf/extensions/common/extension_urls.cc#33
- !url.startsWith("https://chrome.google.com/webstore/")))
Sebastian Noack 2018/04/18 11:25:36 This special case is redundant now, since onCommit
kzar 2018/05/02 12:06:52 Acknowledged.
+ // We have to update the frame structure for documents that weren't
+ // loaded over HTTP (including documents cached by Service Workers),
+ // when the navigation occurs. However, we must be careful to not
+ // update the state of the same document twice, otherewise the number
+ // of any ads blocked already and any recorded sitekey could get lost.
+ let frame = ext.getFrame(details.tabId, details.frameId);
+ if (!frame || frame.url.href != details.url)
{
- updatePageFrameStructure(details.frameId, details.tabId, url,
+ updatePageFrameStructure(details.frameId, details.tabId, details.url,
details.parentFrameId);
}
});
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld