This repository has been archived by the owner on Jan 4, 2019. It is now read-only.
Fix/tabs discard dead tab #504
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Added that this fixes #487. |
petemill
previously approved these changes
Feb 22, 2018
There was a problem hiding this comment.
lgtm. Tested with brave/browser-laptop#13120 which handles the new tab-replaced-at function and it works great. Found an additional issue related to persisting scroll position and some kind of tab storage (not sessionStorage, which is persisted for the tab as expected), and will open a new issue (it should not block this PR IMO.
petemill
added a commit
to brave/browser-laptop
that referenced
this pull request
Feb 22, 2018
Requires brave/muon#504 Ensure that tab/WebContents objects are added to memory as soon as they are created. Use muon's new `tab-replaced-at` event (instead of <webview>.tab-id-changed) to ensure state gets updated correctly when a tab is discarded, as well as communicating temporary contents replacement to a window's frame (such as when detaching a tab to a new window, the tab in the old window gets a temporary WebContents that then gets destroyed). In the case of tabs which get discarded, they are replaced with a fresh, clean WebContents. We were not receiving this new object, so when the renderer asked for this new object's tabId to be made active, the cache did not have that object. We must also handle updating references to the old TabId to the new TabId in state, in the module which remembers opener tab IDs and the module which handles historical tab 'active' history for a window. The guestInstanceId for a tab also changes when a discarded tab is reloaded, so make sure we pass that new value in the frame options when a discarded tab is detached to a new window. Adds more relevant log entries under the `--debug-tab-events` flag.
petemill
added a commit
to brave/browser-laptop
that referenced
this pull request
Feb 22, 2018
Requires brave/muon#504 Ensure that tab/WebContents objects are added to memory as soon as they are created. Use muon's new `tab-replaced-at` event (instead of <webview>.tab-id-changed) to ensure state gets updated correctly when a tab is discarded, as well as communicating temporary contents replacement to a window's frame (such as when detaching a tab to a new window, the tab in the old window gets a temporary WebContents that then gets destroyed). In the case of tabs which get discarded, they are replaced with a fresh, clean WebContents. We were not receiving this new object, so when the renderer asked for this new object's tabId to be made active, the cache did not have that object. We must also handle updating references to the old TabId to the new TabId in state, in the module which remembers opener tab IDs and the module which handles historical tab 'active' history for a window. The guestInstanceId for a tab also changes when a discarded tab is reloaded, so make sure we pass that new value in the frame options when a discarded tab is detached to a new window. Adds more relevant log entries under the `--debug-tab-events` flag.
petemill
added a commit
to brave/browser-laptop
that referenced
this pull request
Feb 22, 2018
Requires brave/muon#504 Ensure that tab/WebContents objects are added to memory as soon as they are created. Use muon's new `tab-replaced-at` event (instead of <webview>.tab-id-changed) to ensure state gets updated correctly when a tab is discarded, as well as communicating temporary contents replacement to a window's frame (such as when detaching a tab to a new window, the tab in the old window gets a temporary WebContents that then gets destroyed). In the case of tabs which get discarded, they are replaced with a fresh, clean WebContents. We were not receiving this new object, so when the renderer asked for this new object's tabId to be made active, the cache did not have that object. We must also handle updating references to the old TabId to the new TabId in state, in the module which remembers opener tab IDs and the module which handles historical tab 'active' history for a window. The guestInstanceId for a tab also changes when a discarded tab is reloaded, so make sure we pass that new value in the frame options when a discarded tab is detached to a new window. Adds more relevant log entries under the `--debug-tab-events` flag.
4 tasks
bridiver
force-pushed
the
fix/tabs-discard-dead-tab
branch
from
422810f
to
b31d829
Compare
bsclifton
approved these changes
Feb 22, 2018
|
@bsclifton I approved but new commits were pushed marking it 'stale'. FWIW all 3 issues are resolved as far as I'm concerned, and I've found no new issues. |
bsclifton
pushed a commit
to brave/browser-laptop
that referenced
this pull request
Feb 23, 2018
Requires brave/muon#504 Ensure that tab/WebContents objects are added to memory as soon as they are created. Use muon's new `tab-replaced-at` event (instead of <webview>.tab-id-changed) to ensure state gets updated correctly when a tab is discarded, as well as communicating temporary contents replacement to a window's frame (such as when detaching a tab to a new window, the tab in the old window gets a temporary WebContents that then gets destroyed). In the case of tabs which get discarded, they are replaced with a fresh, clean WebContents. We were not receiving this new object, so when the renderer asked for this new object's tabId to be made active, the cache did not have that object. We must also handle updating references to the old TabId to the new TabId in state, in the module which remembers opener tab IDs and the module which handles historical tab 'active' history for a window. The guestInstanceId for a tab also changes when a discarded tab is reloaded, so make sure we pass that new value in the frame options when a discarded tab is detached to a new window. Adds more relevant log entries under the `--debug-tab-events` flag.
petemill
added a commit
to brave/browser-laptop
that referenced
this pull request
Feb 23, 2018
Requires brave/muon#504 Ensure that tab/WebContents objects are added to memory as soon as they are created. Use muon's new `tab-replaced-at` event (instead of <webview>.tab-id-changed) to ensure state gets updated correctly when a tab is discarded, as well as communicating temporary contents replacement to a window's frame (such as when detaching a tab to a new window, the tab in the old window gets a temporary WebContents that then gets destroyed). In the case of tabs which get discarded, they are replaced with a fresh, clean WebContents. We were not receiving this new object, so when the renderer asked for this new object's tabId to be made active, the cache did not have that object. We must also handle updating references to the old TabId to the new TabId in state, in the module which remembers opener tab IDs and the module which handles historical tab 'active' history for a window. The guestInstanceId for a tab also changes when a discarded tab is reloaded, so make sure we pass that new value in the frame options when a discarded tab is detached to a new window. Adds more relevant log entries under the `--debug-tab-events` flag.
darkdh
reviewed
Feb 23, 2018
| @@ -318,6 +318,11 @@ void TabViewGuest::ApplyAttributes(const base::DictionaryValue& params) { | |||
|
|
|||
| if (attached() && | |||
| web_contents()->GetController().IsInitialNavigation()) { | |||
| // don't reload if we're already loading | |||
| if (web_contents()->GetController().GetPendingEntry() && | |||
| web_contents()->GetController().GetPendingEntry()->GetURL() == src_) { | |||
ryanml
pushed a commit
to ryanml/browser-laptop
that referenced
this pull request
Feb 27, 2018
Requires brave/muon#504 Ensure that tab/WebContents objects are added to memory as soon as they are created. Use muon's new `tab-replaced-at` event (instead of <webview>.tab-id-changed) to ensure state gets updated correctly when a tab is discarded, as well as communicating temporary contents replacement to a window's frame (such as when detaching a tab to a new window, the tab in the old window gets a temporary WebContents that then gets destroyed). In the case of tabs which get discarded, they are replaced with a fresh, clean WebContents. We were not receiving this new object, so when the renderer asked for this new object's tabId to be made active, the cache did not have that object. We must also handle updating references to the old TabId to the new TabId in state, in the module which remembers opener tab IDs and the module which handles historical tab 'active' history for a window. The guestInstanceId for a tab also changes when a discarded tab is reloaded, so make sure we pass that new value in the frame options when a discarded tab is detached to a new window. Adds more relevant log entries under the `--debug-tab-events` flag.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #487
Fix #496
Fix #505