|
|
DescriptionRefactor TabModel to use WebStateList to store WebStates.
Refactor TabModel to use WebStateList to store WebStates to share
code with the new architecture. Keep a NSSet<Tab*> to retain the
Tabs (as TabModel owns them and WebStateList cannot).
BUG=687207
Review-Url: https://codereview.chromium.org/2683393003
Cr-Commit-Position: refs/heads/master@{#451319}
Committed: https://chromium.googlesource.com/chromium/src/+/32c0c38623575d0441969a8a11b23ee540bf28cb
Patch Set 1 #Patch Set 2 : Rebase to get dependent CL fix of "gn check". #
Total comments: 6
Patch Set 3 : Rebase with WebStateList API using int and address comments. #Patch Set 4 : Rebase. #Patch Set 5 : Fix TabModelTest.MoveTabs. #
Dependent Patchsets: Messages
Total messages: 32 (23 generated)
The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run
sdefresne@chromium.org changed reviewers: + marq@chromium.org, rohitrao@chromium.org
Please take a look.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
lgtm
lgtm https://codereview.chromium.org/2683393003/diff/20001/ios/chrome/browser/tabs... File ios/chrome/browser/tabs/tab_model.mm (right): https://codereview.chromium.org/2683393003/diff/20001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab_model.mm:127: // in the WebStateList (as Tabs currently owns their WebState). Remove once "own" https://codereview.chromium.org/2683393003/diff/20001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab_model.mm:129: base::scoped_nsobject<NSMutableSet<Tab*>> _tabs; Another naming option is to call this something like _tabRetainer, to make it less likely that someone will use it accidentally. https://codereview.chromium.org/2683393003/diff/20001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab_model.mm:565: - (void)moveTab:(Tab*)tab toIndex:(NSUInteger)toIndex { DCHECK([_tabs containsObject:tab]); https://codereview.chromium.org/2683393003/diff/20001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab_model.mm:816: - (id)proxyForWebState:(web::WebState*)webState { What is this for?
The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL. https://codereview.chromium.org/2683393003/diff/20001/ios/chrome/browser/tabs... File ios/chrome/browser/tabs/tab_model.mm (right): https://codereview.chromium.org/2683393003/diff/20001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab_model.mm:127: // in the WebStateList (as Tabs currently owns their WebState). Remove once On 2017/02/14 14:37:12, rohitrao wrote: > "own" Done. https://codereview.chromium.org/2683393003/diff/20001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab_model.mm:129: base::scoped_nsobject<NSMutableSet<Tab*>> _tabs; On 2017/02/14 14:37:12, rohitrao wrote: > Another naming option is to call this something like _tabRetainer, to make it > less likely that someone will use it accidentally. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
lgtm
The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by sdefresne@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from marq@chromium.org, rohitrao@chromium.org Link to the patchset: https://codereview.chromium.org/2683393003/#ps80001 (title: "Fix TabModelTest.MoveTabs.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by sdefresne@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487345604175700, "parent_rev": "96112f4c073dee6ff36623faf30e4f14bd898ac0", "commit_rev": "32c0c38623575d0441969a8a11b23ee540bf28cb"}
Message was sent while issue was closed.
Description was changed from ========== Refactor TabModel to use WebStateList to store WebStates. Refactor TabModel to use WebStateList to store WebStates to share code with the new architecture. Keep a NSSet<Tab*> to retain the Tabs (as TabModel owns them and WebStateList cannot). BUG=687207 ========== to ========== Refactor TabModel to use WebStateList to store WebStates. Refactor TabModel to use WebStateList to store WebStates to share code with the new architecture. Keep a NSSet<Tab*> to retain the Tabs (as TabModel owns them and WebStateList cannot). BUG=687207 Review-Url: https://codereview.chromium.org/2683393003 Cr-Commit-Position: refs/heads/master@{#451319} Committed: https://chromium.googlesource.com/chromium/src/+/32c0c38623575d0441969a8a11b2... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/32c0c38623575d0441969a8a11b2... |