Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2956483003:
[ios] TabIdTabHelper (Closed)
Description[ios] TabIdTabHelper
This CL creates a tab helper that provides a tab ID for WebStates
without a dependency on Tab. The tab ID is stable across cold starts.
This CL also refactors Tab to use this new tab helper, instead
of creating it's own.
BUG=none
Review-Url: https://codereview.chromium.org/2956483003
Cr-Commit-Position: refs/heads/master@{#482375}
Committed: https://chromium.googlesource.com/chromium/src/+/7dc10a6d12ef01c2b01800863fe517fd62d5bade
Patch Set 1 #Patch Set 2 : Refactor Tab to use TabIDTabHelper. #
Total comments: 8
Patch Set 3 : Address comments. #
Total comments: 8
Patch Set 4 : Rebase. #
Total comments: 14
Patch Set 5 : Additional unit tests and other edits. #
Messages
Total messages: 53 (39 generated)
The CQ bit was checked by edchin@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...
edchin@chromium.org changed reviewers: + eugenebut@chromium.org, marq@chromium.org, rohitrao@chromium.org, sdefresne@chromium.org
PTAL.
The CQ bit was checked by edchin@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...
Description was changed from ========== [ios] TabIDTabHelper This CL creates a tab helper that provides a TabID for WebStates without a dependency on Tab. TabID is stable across cold starts. This CL does not refactor Tab to use this new tab helper. BUG=none ========== to ========== [ios] TabIDTabHelper This CL creates a tab helper that provides a TabID for WebStates without a dependency on Tab. TabID is stable across cold starts. This CL also refactors Tab to use this new tab helper, instead of creating it's own TabID. BUG=none ==========
Patchset #2 (id:20001) has been deleted
The CQ bit was checked by edchin@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...)
Patchset #2 (id:40001) has been deleted
The CQ bit was checked by edchin@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: This issue passed the CQ dry run.
LGTM. https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/tabs... File ios/chrome/browser/tabs/tab.mm (right): https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab.mm:426: @dynamic tabId; You only need @dynamic when no implementation of the property is provided in this implementation; @dynamic is an assurance to the complier that some other code will handle accessing the property. For this change, just deleting the @synthesize line is sufficient.
https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/tabs... File ios/chrome/browser/tabs/tab.mm (right): https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab.mm:616: TabIDTabHelper::FromWebState(self.webState)->tab_id()); Is the TabID always used as an NSString? If so, should we just store that in the helper instead of a std::string? https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/web/... File ios/chrome/browser/web/tab_id_tab_helper.h (right): https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/web/... ios/chrome/browser/web/tab_id_tab_helper.h:12: typedef std::string TabID; Is there a reason to put this behind a TabID typedef? In theory, I like the idea of making the type opaque to callers, but in practice doesn't everyone perform string operations directly on the result? If callers always treat this as a string, is there value in having the TabID ifdef? https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/web/... File ios/chrome/browser/web/tab_id_tab_helper.mm (right): https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/web/... ios/chrome/browser/web/tab_id_tab_helper.mm:25: web::SerializableUserDataManager::FromWebState(web_state); Is this guaranteed to exist before the TabIDTabHelper is created?
PTAL. Comments addressed. https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/tabs... File ios/chrome/browser/tabs/tab.mm (right): https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab.mm:426: @dynamic tabId; On 2017/06/23 08:46:29, marq (ping after 24h) wrote: > You only need @dynamic when no implementation of the property is provided in > this implementation; @dynamic is an assurance to the complier that some other > code will handle accessing the property. > > For this change, just deleting the @synthesize line is sufficient. Done. https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab.mm:616: TabIDTabHelper::FromWebState(self.webState)->tab_id()); On 2017/06/23 11:10:27, rohitrao (ping after 24h) wrote: > Is the TabID always used as an NSString? If so, should we just store that in > the helper instead of a std::string? My original intent is to be consistent with ios/web (which prefers std::string over NSString) since this class derives from an ios/web class. However, looking at other recent subclasses of web::WebStateUserData, it is precedented to use NSString. I have updated this to use NSString. https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/web/... File ios/chrome/browser/web/tab_id_tab_helper.h (right): https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/web/... ios/chrome/browser/web/tab_id_tab_helper.h:12: typedef std::string TabID; On 2017/06/23 11:10:27, rohitrao (ping after 24h) wrote: > Is there a reason to put this behind a TabID typedef? > > In theory, I like the idea of making the type opaque to callers, but in practice > doesn't everyone perform string operations directly on the result? If callers > always treat this as a string, is there value in having the TabID ifdef? Agree that in practice it does not provide any benefit. I have updated to directly use NSString. https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/web/... File ios/chrome/browser/web/tab_id_tab_helper.mm (right): https://codereview.chromium.org/2956483003/diff/60001/ios/chrome/browser/web/... ios/chrome/browser/web/tab_id_tab_helper.mm:25: web::SerializableUserDataManager::FromWebState(web_state); On 2017/06/23 11:10:27, rohitrao (ping after 24h) wrote: > Is this guaranteed to exist before the TabIDTabHelper is created? The class comment says this: "// Returns the SerializableUserDataManager instance associated with // |web_state|, instantiating one if necessary." I interpret this to mean it will exist by the next line.
The CQ bit was checked by edchin@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-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... File ios/chrome/browser/web/tab_id_tab_helper.h (right): https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... ios/chrome/browser/web/tab_id_tab_helper.h:22: NSString* tab_id_; __strong NSString* tab_id_;
PTAL. Rebased and addressed sdefresne@'s comment. https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... File ios/chrome/browser/web/tab_id_tab_helper.h (right): https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... ios/chrome/browser/web/tab_id_tab_helper.h:22: NSString* tab_id_; On 2017/06/23 16:57:03, sdefresne wrote: > __strong NSString* tab_id_; Done.
The CQ bit was checked by edchin@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...
Sorry for the delay. lgtm https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/tabs... File ios/chrome/browser/tabs/tab_helper_util.mm (right): https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab_helper_util.mm:43: // TabIDTabHelper provides the TabID, which is necessary in other helpers. nit: Does this comment add anything? https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... File ios/chrome/browser/web/tab_id_tab_helper.mm (right): https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... ios/chrome/browser/web/tab_id_tab_helper.mm:28: if (!unique_id || ![unique_id length]) { No need for |!unique_id || | check. |[unique_id length]| will return nil if |unique_id| is nil https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... File ios/chrome/browser/web/tab_id_tab_helper_unittest.mm (right): https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... ios/chrome/browser/web/tab_id_tab_helper_unittest.mm:25: TEST_F(TabIDTabHelperTest, TabIDForWebState) { Would it be useful to test serialization? https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... File ios/chrome/browser/web/tab_id_tab_helper.h (right): https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... ios/chrome/browser/web/tab_id_tab_helper.h:12: class TabIDTabHelper : public web::WebStateUserData<TabIDTabHelper> { s/TabIDTabHelper/TabIdTabHelper to follow C++ Style Guide? https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#... Yes, Chromium code is very inconsistent with this :)
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...)
https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/tab... File ios/chrome/browser/tabs/tab.mm (right): https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/tab... ios/chrome/browser/tabs/tab.mm:613: return TabIDTabHelper::FromWebState(self.webState)->tab_id(); Since tab_id() method is not virtual, this will lead to hard to understand errors if TabIDTabHelper::FromWebState() returns null. Maybe add a DCHECK here. TabIDTabHelper* tab_id_helper = TabIDTabHelper::FromWebState(self.webState); DCHECK(tab_id_helper); return tab_id_helper->tab_id(); https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/tab... File ios/chrome/browser/tabs/tab_helper_util.mm (right): https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/tab... ios/chrome/browser/tabs/tab_helper_util.mm:43: // TabIDTabHelper provides the TabID, which is necessary in other helpers. I would say "tab ID" instead of "TabID" in this comment. And I would say that this is necessary for the creation of the Tab instance (which happens in when LegacyTabHelper is created). Maybe something like this: // TabIDHelper sets up the tab ID which is required for the creation of the // Tab by LegacyTabHelper. https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/tab... ios/chrome/browser/tabs/tab_helper_util.mm:52: // IOSChromeSessionTabHelper comes first because it sets up the session ID, This is now the third tab helper installed, so "comes first" part of the comment is incorrect. Could you change this to: // IOSChromeSessionTabHelper sets up the session ID used by other helper and // and needs to be create before them. https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... File ios/chrome/browser/web/tab_id_tab_helper.h (right): https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... ios/chrome/browser/web/tab_id_tab_helper.h:12: class TabIDTabHelper : public web::WebStateUserData<TabIDTabHelper> { On 2017/06/23 22:44:57, Eugene But wrote: > s/TabIDTabHelper/TabIdTabHelper to follow C++ Style Guide? > > https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#... > > Yes, Chromium code is very inconsistent with this :) Well technically, Id is an abbreviation and should not be used either (https://google.github.io/styleguide/cppguide.html#General_Naming_Rules), so this should be TabIdentifierTabHelper. I agree that TabIdTabHelper looks better than TabIDTabHelper. https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... ios/chrome/browser/web/tab_id_tab_helper.h:21: friend class web::WebStateUserData<TabIDTabHelper>; optional: move this before the declaration of the constructor (this is a class declaration and the style recommend the following order in a section "types (including typedef, using, and nested structs and classes), constants, factory functions, constructors, assignment operators, destructor, all other methods, data members". So that would be: private: friend class web::WebStateUserData<TabIDTabHelper>; explicit TabIDTabHelper(web::WebState* web_state); __strong NSString* tab_id_; https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... File ios/chrome/browser/web/tab_id_tab_helper_unittest.mm (right): https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... ios/chrome/browser/web/tab_id_tab_helper_unittest.mm:37: } Maybe test that the TabID is stable across calls, i.e. that successive calls to tab_id() on the same object return the same value. I was broken at some point for deserialized objects and caused a RBS.
lgtm
https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... File ios/chrome/browser/web/tab_id_tab_helper.h (right): https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... ios/chrome/browser/web/tab_id_tab_helper.h:12: class TabIDTabHelper : public web::WebStateUserData<TabIDTabHelper> { On 2017/06/26 10:28:55, sdefresne wrote: > On 2017/06/23 22:44:57, Eugene But wrote: > > s/TabIDTabHelper/TabIdTabHelper to follow C++ Style Guide? > > > > > https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#... > > > > Yes, Chromium code is very inconsistent with this :) > > Well technically, Id is an abbreviation and should not be used either > (https://google.github.io/styleguide/cppguide.html#General_Naming_Rules), so > this should be TabIdentifierTabHelper. > > I agree that TabIdTabHelper looks better than TabIDTabHelper. I think Style Guide is fine with ID abbreviation. It even has an example: int cstmr_id; // Deletes internal letters. where |cstmr| is not fine, but |id| part is.
The CQ bit was checked by edchin@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...
TBD: two more unit tests. All other comments addressed. https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/tabs... File ios/chrome/browser/tabs/tab_helper_util.mm (right): https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/tabs... ios/chrome/browser/tabs/tab_helper_util.mm:43: // TabIDTabHelper provides the TabID, which is necessary in other helpers. On 2017/06/23 22:44:56, Eugene But wrote: > nit: Does this comment add anything? Updated to be more specific. https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... File ios/chrome/browser/web/tab_id_tab_helper.mm (right): https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... ios/chrome/browser/web/tab_id_tab_helper.mm:28: if (!unique_id || ![unique_id length]) { On 2017/06/23 22:44:56, Eugene But wrote: > No need for |!unique_id || | check. |[unique_id length]| will return nil if > |unique_id| is nil Done. https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... File ios/chrome/browser/web/tab_id_tab_helper_unittest.mm (right): https://codereview.chromium.org/2956483003/diff/80001/ios/chrome/browser/web/... ios/chrome/browser/web/tab_id_tab_helper_unittest.mm:25: TEST_F(TabIDTabHelperTest, TabIDForWebState) { On 2017/06/23 22:44:56, Eugene But wrote: > Would it be useful to test serialization? Will do. https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/tab... File ios/chrome/browser/tabs/tab.mm (right): https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/tab... ios/chrome/browser/tabs/tab.mm:613: return TabIDTabHelper::FromWebState(self.webState)->tab_id(); On 2017/06/26 10:28:55, sdefresne wrote: > Since tab_id() method is not virtual, this will lead to hard to understand > errors if TabIDTabHelper::FromWebState() returns null. Maybe add a DCHECK here. > > TabIDTabHelper* tab_id_helper = TabIDTabHelper::FromWebState(self.webState); > DCHECK(tab_id_helper); > return tab_id_helper->tab_id(); Done. https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/tab... File ios/chrome/browser/tabs/tab_helper_util.mm (right): https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/tab... ios/chrome/browser/tabs/tab_helper_util.mm:43: // TabIDTabHelper provides the TabID, which is necessary in other helpers. On 2017/06/26 10:28:55, sdefresne wrote: > I would say "tab ID" instead of "TabID" in this comment. And I would say that > this is necessary for the creation of the Tab instance (which happens in when > LegacyTabHelper is created). Maybe something like this: > > // TabIDHelper sets up the tab ID which is required for the creation of the > // Tab by LegacyTabHelper. Done. https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/tab... ios/chrome/browser/tabs/tab_helper_util.mm:52: // IOSChromeSessionTabHelper comes first because it sets up the session ID, On 2017/06/26 10:28:55, sdefresne wrote: > This is now the third tab helper installed, so "comes first" part of the comment > is incorrect. Could you change this to: > > // IOSChromeSessionTabHelper sets up the session ID used by other helper and > // and needs to be create before them. > Done. https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... File ios/chrome/browser/web/tab_id_tab_helper.h (right): https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... ios/chrome/browser/web/tab_id_tab_helper.h:12: class TabIDTabHelper : public web::WebStateUserData<TabIDTabHelper> { On 2017/06/26 14:09:44, Eugene But wrote: > On 2017/06/26 10:28:55, sdefresne wrote: > > On 2017/06/23 22:44:57, Eugene But wrote: > > > s/TabIDTabHelper/TabIdTabHelper to follow C++ Style Guide? > > > > > > > > > https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#... > > > > > > Yes, Chromium code is very inconsistent with this :) > > > > Well technically, Id is an abbreviation and should not be used either > > (https://google.github.io/styleguide/cppguide.html#General_Naming_Rules), so > > this should be TabIdentifierTabHelper. > > > > I agree that TabIdTabHelper looks better than TabIDTabHelper. > I think Style Guide is fine with ID abbreviation. It even has an example: > > int cstmr_id; // Deletes internal letters. > > where |cstmr| is not fine, but |id| part is. > Done. https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... ios/chrome/browser/web/tab_id_tab_helper.h:21: friend class web::WebStateUserData<TabIDTabHelper>; On 2017/06/26 10:28:55, sdefresne wrote: > optional: move this before the declaration of the constructor (this is a class > declaration and the style recommend the following order in a section "types > (including typedef, using, and nested structs and classes), constants, factory > functions, constructors, assignment operators, destructor, all other methods, > data members". > > So that would be: > > private: > friend class web::WebStateUserData<TabIDTabHelper>; > explicit TabIDTabHelper(web::WebState* web_state); > __strong NSString* tab_id_; > Done. https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... File ios/chrome/browser/web/tab_id_tab_helper_unittest.mm (right): https://codereview.chromium.org/2956483003/diff/100001/ios/chrome/browser/web... ios/chrome/browser/web/tab_id_tab_helper_unittest.mm:37: } On 2017/06/26 10:28:55, sdefresne wrote: > Maybe test that the TabID is stable across calls, i.e. that successive calls to > tab_id() on the same object return the same value. I was broken at some point > for deserialized objects and caused a RBS. Will do.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== [ios] TabIDTabHelper This CL creates a tab helper that provides a TabID for WebStates without a dependency on Tab. TabID is stable across cold starts. This CL also refactors Tab to use this new tab helper, instead of creating it's own TabID. BUG=none ========== to ========== [ios] TabIdTabHelper This CL creates a tab helper that provides a tab ID for WebStates without a dependency on Tab. The tab ID is stable across cold starts. This CL also refactors Tab to use this new tab helper, instead of creating it's own. BUG=none ==========
The CQ bit was checked by edchin@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...
Patchset #5 (id:120001) has been deleted
Additional unit tests were added. All comments have been addressed, and lgtm's have been received. I plan to land within a few hours, not expecting additional comments.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by edchin@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from marq@chromium.org, eugenebut@chromium.org, sdefresne@chromium.org Link to the patchset: https://codereview.chromium.org/2956483003/#ps140001 (title: "Additional unit tests and other edits.")
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": 140001, "attempt_start_ts": 1498507865978170,
"parent_rev": "47ab7b47d09193a1c924ec9349d541e3a5ed7809", "commit_rev":
"54e0d8ff5b6b7d5622defda8f873ae5262c4716c"}
CQ is committing da patch.
Bot data: {"patchset_id": 140001, "attempt_start_ts": 1498507865978170,
"parent_rev": "52b3fa865e5be5eb8533fdbac751ea489705fad5", "commit_rev":
"7dc10a6d12ef01c2b01800863fe517fd62d5bade"}
Message was sent while issue was closed.
Description was changed from ========== [ios] TabIdTabHelper This CL creates a tab helper that provides a tab ID for WebStates without a dependency on Tab. The tab ID is stable across cold starts. This CL also refactors Tab to use this new tab helper, instead of creating it's own. BUG=none ========== to ========== [ios] TabIdTabHelper This CL creates a tab helper that provides a tab ID for WebStates without a dependency on Tab. The tab ID is stable across cold starts. This CL also refactors Tab to use this new tab helper, instead of creating it's own. BUG=none Review-Url: https://codereview.chromium.org/2956483003 Cr-Commit-Position: refs/heads/master@{#482375} Committed: https://chromium.googlesource.com/chromium/src/+/7dc10a6d12ef01c2b01800863fe5... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:140001) as https://chromium.googlesource.com/chromium/src/+/7dc10a6d12ef01c2b01800863fe5... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
