Issue 4804 - Avoid trashing pagemaps prematurely

chrome/ext/background.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 111 matching lines @@
 
     return frame;
   }
 
   function updatePageFrameStructure(frameId, tabId, url)
   {
     if (frameId == 0)
     {
       let page = new Page({id: tabId, url: url});
 
-      ext._removeFromAllPageMaps(tabId);

[kzar, 2017/01/18 08:12:09]

I can't see why to remove the tabId from the pageMaps at this point.
- New tabs including prerendered ones get a new tab ID.
- Closed tabs will be removed by our chrome.tabs.onRemoved listener
- Replaced tabs (prerendered tab committed to) will be removed by our
chrome.tabs.onReplaced listener.

Do I miss something?

[Sebastian Noack, 2017/01/18 11:58:31]

The Page and PageMap objects abstract the concept of pages/documents, not tabs.
So if an existing tab loads a new page/document, this is meant to be a different
Page object.

For some of our logic, the distinction between pages and tabs is irrelevant.
However, for the recorded frame hierarchy it is crucial to reset the state when
a tab navigates to a new page/document.

Once we got rid of ext.*, and ext.PageMap in particular, we can just use regular
Map objects with the tabId as key, and clear the frame hierarchy manually. While
the icon animation can use its own data structure which persists the tabs.

Addressing this issue while we still use ext.* is probably not trivial, and
would require adding more complexity to the abtraction layer. So I suggest to
revist this issue later.

-
       chrome.tabs.get(tabId, function()
       {
         // If the tab is prerendered, chrome.tabs.get() sets
         // chrome.runtime.lastError and we have to dispatch the onLoading event,
         // since the onUpdated event isn't dispatched for prerendered tabs.
         // However, we have to keep relying on the unUpdated event for tabs that
         // are already visible. Otherwise browser action changes get overridden
         // when Chrome automatically resets them on navigation.
         if (chrome.runtime.lastError)
           ext.pages.onLoading._dispatch(page);
@@ skipping 447 matching lines @@
   ext.windows = {
     create: function(createData, callback)
     {
       chrome.windows.create(createData, function(createdWindow)
       {
         afterTabLoaded(callback)(createdWindow.tabs[0]);
       });
     }
   };
 })();