Chromium Code Reviews
Help | Chromium Project | Sign in
(31)

Issue 2697193004: Add opener-opened relationship between WebState in WebStateList. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 1 week ago by sdefresne
Modified:
1 month ago
Reviewers:
marq, rohitrao
CC:
chromium-reviews, marq+watch_chromium.org, pklpkl, noyau+watch_chromium.org, sdefresne+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add opener-opened relationship between WebState in WebStateList. In preparation of refactoring TabModelOrderController to be shared with the new architecture, move the opener-opened relationship to WebStateList. The relationship can be implemented without any reference to tabId, openerId or openerNavigationIndex. Those notions will be needed for serialisation (and will be possible to implement once serialisation refactoring is complete, until then leave the serialisation in the TabModel class). BUG=687207 Review-Url: https://codereview.chromium.org/2697193004 Cr-Commit-Position: refs/heads/master@{#452439} Committed: https://chromium.googlesource.com/chromium/src/+/e0a9ba6076a8522976835cafac5e389aab66a43e

Patch Set 1 #

Total comments: 8

Patch Set 2 : Address comments. #

Patch Set 3 : Rebase. #

Patch Set 4 : Fix opener relationship when restoring a session. #

Total comments: 8

Patch Set 5 : Address comments. #

Patch Set 6 : Fix copy-n-paste error. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+482 lines, -111 lines) Patch
M ios/chrome/browser/tabs/tab_model.h View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/browser/tabs/tab_model.mm View 1 2 3 4 9 chunks +89 lines, -62 lines 0 comments Download
M ios/chrome/browser/tabs/tab_model_unittest.mm View 1 2 3 4 2 chunks +23 lines, -4 lines 0 comments Download
M ios/shared/chrome/browser/tabs/web_state_list.h View 1 2 3 4 2 chunks +47 lines, -4 lines 0 comments Download
M ios/shared/chrome/browser/tabs/web_state_list.mm View 1 2 3 4 5 7 chunks +127 lines, -6 lines 0 comments Download
M ios/shared/chrome/browser/tabs/web_state_list_fast_enumeration_helper_unittest.mm View 1 chunk +1 line, -1 line 0 comments Download
M ios/shared/chrome/browser/tabs/web_state_list_unittest.mm View 1 2 3 4 18 chunks +194 lines, -34 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 40 (29 generated)
sdefresne
Please take a look.
1 month, 1 week ago (2017-02-16 12:40:27 UTC) #2
marq
LGTM modulo comment and naming nits. https://codereview.chromium.org/2697193004/diff/20001/ios/shared/chrome/browser/tabs/web_state_list.h File ios/shared/chrome/browser/tabs/web_state_list.h (right): https://codereview.chromium.org/2697193004/diff/20001/ios/shared/chrome/browser/tabs/web_state_list.h#newcode112 ios/shared/chrome/browser/tabs/web_state_list.h:112: int skip_up_to, Can ...
1 month, 1 week ago (2017-02-16 15:19:17 UTC) #4
sdefresne
Thank you for the review. https://codereview.chromium.org/2697193004/diff/20001/ios/shared/chrome/browser/tabs/web_state_list.h File ios/shared/chrome/browser/tabs/web_state_list.h (right): https://codereview.chromium.org/2697193004/diff/20001/ios/shared/chrome/browser/tabs/web_state_list.h#newcode112 ios/shared/chrome/browser/tabs/web_state_list.h:112: int skip_up_to, On 2017/02/16 ...
1 month, 1 week ago (2017-02-16 15:42:01 UTC) #5
marq
lgtm
1 month, 1 week ago (2017-02-16 15:54:48 UTC) #6
sdefresne
rohitrao: ping
1 month ago (2017-02-20 15:17:52 UTC) #17
rohitrao
lgtm https://codereview.chromium.org/2697193004/diff/140001/ios/chrome/browser/tabs/tab_model.mm File ios/chrome/browser/tabs/tab_model.mm (right): https://codereview.chromium.org/2697193004/diff/140001/ios/chrome/browser/tabs/tab_model.mm#newcode996 ios/chrome/browser/tabs/tab_model.mm:996: // Recreate all the restored tabs but do ...
1 month ago (2017-02-22 14:57:53 UTC) #27
sdefresne
Thank you for the review. https://codereview.chromium.org/2697193004/diff/140001/ios/chrome/browser/tabs/tab_model.mm File ios/chrome/browser/tabs/tab_model.mm (right): https://codereview.chromium.org/2697193004/diff/140001/ios/chrome/browser/tabs/tab_model.mm#newcode996 ios/chrome/browser/tabs/tab_model.mm:996: // Recreate all the ...
1 month ago (2017-02-22 18:23:47 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697193004/180001
1 month ago (2017-02-22 18:24:52 UTC) #32
commit-bot: I haz the power
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds/158046) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, ...
1 month ago (2017-02-22 18:34:16 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2697193004/200001
1 month ago (2017-02-23 09:01:49 UTC) #37
commit-bot: I haz the power
1 month ago (2017-02-23 09:14:52 UTC) #40
Message was sent while issue was closed.
Committed patchset #6 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/e0a9ba6076a8522976835cafac5e...
Sign in to reply to this message.

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