Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(30)

Issue 2687303003: Convert some of TabModel logic to WebStateListObservers. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 months, 1 week ago by sdefresne
Modified:
7 months ago
CC:
chromium-reviews, marq+watch_chromium.org, pkl (ping after 24h if needed), noyau+watch_chromium.org, sdefresne+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Convert some of TabModel logic to WebStateListObservers. BUG=687207 Review-Url: https://codereview.chromium.org/2687303003 Cr-Commit-Position: refs/heads/master@{#451593} Committed: https://chromium.googlesource.com/chromium/src/+/39bfaaba5dc0d78349ccb2a1a6c2eb42d16dca69

Patch Set 1 #

Total comments: 10

Patch Set 2 : Rebase. #

Patch Set 3 : Rename classes, convert them to C++, fix index types mismatches. #

Patch Set 4 : Rename TabModelMetricsObserver to WebStateListMetricsObserver and move it to ios/shared/chrome/brow… #

Patch Set 5 : Rebase. #

Patch Set 6 : Rebase. #

Patch Set 7 : Rebase. #

Patch Set 8 : Rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+293 lines, -24 lines) Patch
M ios/chrome/browser/tabs/BUILD.gn View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M ios/chrome/browser/tabs/tab_model.mm View 1 2 3 4 5 6 9 chunks +21 lines, -24 lines 0 comments Download
A ios/chrome/browser/tabs/tab_model_observers_bridge.h View 1 2 1 chunk +24 lines, -0 lines 0 comments Download
A ios/chrome/browser/tabs/tab_model_observers_bridge.mm View 1 2 1 chunk +78 lines, -0 lines 0 comments Download
A ios/chrome/browser/tabs/tab_parenting_observer.h View 1 2 1 chunk +36 lines, -0 lines 0 comments Download
A ios/chrome/browser/tabs/tab_parenting_observer.mm View 1 2 1 chunk +47 lines, -0 lines 0 comments Download
M ios/shared/chrome/browser/tabs/BUILD.gn View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
A ios/shared/chrome/browser/tabs/web_state_list_metrics_observer.h View 1 2 3 1 chunk +36 lines, -0 lines 0 comments Download
A ios/shared/chrome/browser/tabs/web_state_list_metrics_observer.mm View 1 2 3 1 chunk +45 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Dependent Patchsets:

Messages

Total messages: 37 (28 generated)
sdefresne
Please take a look.
7 months, 1 week ago (2017-02-10 16:40:12 UTC) #3
marq (ping after 24h)
https://codereview.chromium.org/2687303003/diff/1/ios/chrome/browser/tabs/tab_model.mm File ios/chrome/browser/tabs/tab_model.mm (left): https://codereview.chromium.org/2687303003/diff/1/ios/chrome/browser/tabs/tab_model.mm#oldcode547 ios/chrome/browser/tabs/tab_model.mm:547: [tab fetchFavicon]; Any idea why [tab setParentTabModel:self] isn't called ...
7 months, 1 week ago (2017-02-13 17:06:54 UTC) #7
sdefresne
https://codereview.chromium.org/2687303003/diff/1/ios/chrome/browser/tabs/tab_model.mm File ios/chrome/browser/tabs/tab_model.mm (left): https://codereview.chromium.org/2687303003/diff/1/ios/chrome/browser/tabs/tab_model.mm#oldcode547 ios/chrome/browser/tabs/tab_model.mm:547: [tab fetchFavicon]; On 2017/02/13 17:06:54, marq wrote: > Any ...
7 months, 1 week ago (2017-02-13 17:13:39 UTC) #8
rohitrao (ping after 24h)
lgtm https://codereview.chromium.org/2687303003/diff/1/ios/chrome/browser/tabs/tab_model_metrics_observer.h File ios/chrome/browser/tabs/tab_model_metrics_observer.h (right): https://codereview.chromium.org/2687303003/diff/1/ios/chrome/browser/tabs/tab_model_metrics_observer.h#newcode11 ios/chrome/browser/tabs/tab_model_metrics_observer.h:11: @interface TabModelMetricsObserver : NSObject<WebStateListObserver> Could this be a ...
7 months ago (2017-02-14 13:29:05 UTC) #10
marq (ping after 24h)
lgtm
7 months ago (2017-02-14 13:35:26 UTC) #11
sdefresne
PTAL https://codereview.chromium.org/2687303003/diff/1/ios/chrome/browser/tabs/tab_model_metrics_observer.h File ios/chrome/browser/tabs/tab_model_metrics_observer.h (right): https://codereview.chromium.org/2687303003/diff/1/ios/chrome/browser/tabs/tab_model_metrics_observer.h#newcode11 ios/chrome/browser/tabs/tab_model_metrics_observer.h:11: @interface TabModelMetricsObserver : NSObject<WebStateListObserver> On 2017/02/14 13:29:05, rohitrao ...
7 months ago (2017-02-14 17:19:39 UTC) #18
rohitrao (ping after 24h)
lgtm
7 months ago (2017-02-17 12:56:00 UTC) #27
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/2687303003/160001
7 months ago (2017-02-20 09:41:42 UTC) #34
commit-bot: I haz the power
7 months ago (2017-02-20 11:07:23 UTC) #37
Message was sent while issue was closed.
Committed patchset #8 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/39bfaaba5dc0d78349ccb2a1a6c2...
Sign in to reply to this message.

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