chrome/browser/ui/browser_commands.h - Issue 2448943002: Refactor SecurityStateModel/Clients for simplicity and reusability.

Side by Side Diff: chrome/browser/ui/browser_commands.h

Issue 2448943002: Refactor SecurityStateModel/Clients for simplicity and reusability. (Closed)

Patch Set: refactor -> SecurityStateTabHelper. Created 4 years, 1 month ago

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

Jump to:

OLD	NEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.	1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

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

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #ifndef CHROME_BROWSER_UI_BROWSER_COMMANDS_H_	5 #ifndef CHROME_BROWSER_UI_BROWSER_COMMANDS_H_

6 #define CHROME_BROWSER_UI_BROWSER_COMMANDS_H_	6 #define CHROME_BROWSER_UI_BROWSER_COMMANDS_H_

7	7

8 #include <string>	8 #include <string>

9	9

10 #include "build/build_config.h"	10 #include "build/build_config.h"

11 #include "chrome/browser/devtools/devtools_toggle_action.h"	11 #include "chrome/browser/devtools/devtools_toggle_action.h"

12 #include "chrome/browser/ui/tabs/tab_strip_model_delegate.h"	12 #include "chrome/browser/ui/tabs/tab_strip_model_delegate.h"

13 #include "components/security_state/security_state_model.h"	13 #include "components/security_state/core/security_state_model.h"
	estark 2016/11/02 05:22:02 Looks like you could just delete this and forward- Looks like you could just delete this and forward-declare SecurityInfo instead. Eric Seckler 2016/11/02 09:56:53 I think that only works if we move SecurityInfo ou Show quoted text On 2016/11/02 05:22:02, estark wrote: > Looks like you could just delete this and forward-declare SecurityInfo instead. I think that only works if we move SecurityInfo out of SecurityStateModel (it's a nested class at the moment). WDYT? estark 2016/11/03 04:38:45 Oh, that's right -- fine to leave as is. However, Show quoted text On 2016/11/02 09:56:53, Eric Seckler wrote: > On 2016/11/02 05:22:02, estark wrote: > > Looks like you could just delete this and forward-declare SecurityInfo > instead. > > I think that only works if we move SecurityInfo out of SecurityStateModel (it's > a nested class at the moment). WDYT? Oh, that's right -- fine to leave as is. However, I left a comment in security_state_model.h; I do think we might want to remove SecurityStateModel the class (not necessarily in this CL) and replace it with a couple constants, structs, and a GetSecurityInfo() utility function. If we do that later, we'll have to come back and replace all security_state::SecurityStateModel::SecurityInfo references with security_state::SecurityInfo. So, if you want to do that now, it might save some time later, but not necessary to do now IMO. Eric Seckler 2016/11/03 17:01:06 You're right, makes sense to do it in one review. Show quoted text On 2016/11/03 04:38:45, estark wrote: > On 2016/11/02 09:56:53, Eric Seckler wrote: > > On 2016/11/02 05:22:02, estark wrote: > > > Looks like you could just delete this and forward-declare SecurityInfo > > instead. > > > > I think that only works if we move SecurityInfo out of SecurityStateModel > (it's > > a nested class at the moment). WDYT? > > Oh, that's right -- fine to leave as is. However, I left a comment in > security_state_model.h; I do think we might want to remove SecurityStateModel > the class (not necessarily in this CL) and replace it with a couple constants, > structs, and a GetSecurityInfo() utility function. If we do that later, we'll > have to come back and replace all > security_state::SecurityStateModel::SecurityInfo references with > security_state::SecurityInfo. So, if you want to do that now, it might save some > time later, but not necessary to do now IMO. You're right, makes sense to do it in one review. SecurityStateModel is no more, includes replaced by forward-decls. Hope I got them all :)
14 #include "content/public/common/page_zoom.h"	14 #include "content/public/common/page_zoom.h"

15 #include "ui/base/window_open_disposition.h"	15 #include "ui/base/window_open_disposition.h"

16	16

17 class Browser;	17 class Browser;

18 class CommandObserver;	18 class CommandObserver;

19 class GURL;	19 class GURL;

20 class Profile;	20 class Profile;

21	21

22 namespace content {	22 namespace content {

23 class PageState;	23 class PageState;

(...skipping 139 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
163 void CreateApplicationShortcuts(Browser* browser);	163 void CreateApplicationShortcuts(Browser* browser);

164 void CreateBookmarkAppFromCurrentWebContents(Browser* browser);	164 void CreateBookmarkAppFromCurrentWebContents(Browser* browser);

165 bool CanCreateApplicationShortcuts(const Browser* browser);	165 bool CanCreateApplicationShortcuts(const Browser* browser);

166 bool CanCreateBookmarkApp(const Browser* browser);	166 bool CanCreateBookmarkApp(const Browser* browser);

167	167

168 void ConvertTabToAppWindow(Browser* browser, content::WebContents* contents);	168 void ConvertTabToAppWindow(Browser* browser, content::WebContents* contents);

169	169

170 } // namespace chrome	170 } // namespace chrome

171	171

172 #endif // CHROME_BROWSER_UI_BROWSER_COMMANDS_H_	172 #endif // CHROME_BROWSER_UI_BROWSER_COMMANDS_H_

OLD	NEW

« chrome/browser/ssl/security_state_tab_helper.h ('K') | « chrome/browser/ui/browser.cc ('k') | chrome/browser/ui/browser_dialogs.h » ('j') | chrome/browser/ui/browser_dialogs.h » ('J')