ios/chrome/browser/web/tab_id_tab_helper.h - Issue 2956483003: [ios] TabIdTabHelper

Side by Side Diff: ios/chrome/browser/web/tab_id_tab_helper.h

Issue 2956483003: [ios] TabIdTabHelper (Closed)

Patch Set: Rebase. Created 3 years, 5 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« ios/chrome/browser/tabs/tab_helper_util.mm ('K') | « ios/chrome/browser/web/BUILD.gn ('k') | ios/chrome/browser/web/tab_id_tab_helper.mm » ('j') | ios/chrome/browser/web/tab_id_tab_helper_unittest.mm » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 // Copyright 2017 The Chromium Authors. All rights reserved.

	2 // Use of this source code is governed by a BSD-style license that can be

	3 // found in the LICENSE file.

	4

	5 #ifndef IOS_CHROME_BROWSER_WEB_TAB_ID_TAB_HELPER_H_

	6 #define IOS_CHROME_BROWSER_WEB_TAB_ID_TAB_HELPER_H_

	7

	8 #include "base/macros.h"

	9 #import "ios/web/public/web_state/web_state_user_data.h"

	10

	11 // Handles creating a unique identifier, which is stable across cold starts.

	12 class TabIDTabHelper : public web::WebStateUserData<TabIDTabHelper> {
	Eugene But (OOO till 7-30) 2017/06/23 22:44:57 s/TabIDTabHelper/TabIdTabHelper to follow C++ Styl 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 :) sdefresne 2017/06/26 10:28:55 Well technically, Id is an abbreviation and should Show quoted text 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. Eugene But (OOO till 7-30) 2017/06/26 14:09:44 I think Style Guide is fine with ID abbreviation. Show quoted text 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. edchin 2017/06/26 16:20:57 Done. Show quoted text 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.
	13 public:

	14 ~TabIDTabHelper() override;

	15

	16 // Returns a unique identifier for this tab.

	17 NSString* tab_id() const { return tab_id_; }

	18

	19 private:

	20 explicit TabIDTabHelper(web::WebState* web_state);

	21 friend class web::WebStateUserData<TabIDTabHelper>;
	sdefresne 2017/06/26 10:28:55 optional: move this before the declaration of the 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_; edchin 2017/06/26 16:20:57 Done. Show quoted text 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.
	22 __strong NSString* tab_id_;

	23

	24 DISALLOW_COPY_AND_ASSIGN(TabIDTabHelper);

	25 };

	26

	27 #endif // IOS_CHROME_BROWSER_WEB_TAB_ID_TAB_HELPER_H_

OLD	NEW