Abstract ToolbarModelImpl dependencies on //content.

Abstract ToolbarModelImpl dependencies on //content.

Add following methods to ToolbarModelImpl to abstract the dependencies
on content::WebContents.

- GetURL(): fetches the URL to display, if it exists, or return false,

- ShouldDisplayURL(): returns whether the URL should be displayed in the
  toolbar or not (embedder specific)

- GetSecurityLevel(): returns the security level of the underlying page,
  independently of the editing status of the toolbar,

- GetSearchTerms(): extracts the search terms for the current page from
  the content::WebContents (implementation depends on the embedder),

- GetCertificate(): returns the certificate for the current page, if
  any, otherwise returns null.

Inject the constant content::kMaxURLDisplayChars through the constructor
of ToolbarModelImpl to allow sharing the code with iOS that cannot depend
on content.

TBR=dzhioev@chromium.org
BUG=582921
Committed: https://crrev.com/4d623d703112ab992b4d48ca98d21ed78f850c68
Cr-Commit-Position: refs/heads/master@{#373317}

[sdefresne, 2016-02-01 17:08:52]

sdefresne@chromium.org changed reviewers:
+ pkasting@chromium.org, rohitrao@chromium.org

[sdefresne, 2016-02-01 17:08:53]

pkasting: please take a look.

rohitrao: FYI

[Peter Kasting, 2016-02-02 23:51:31]

I don't really get why we have to inject the character limit.  That constant
isn't part of WebContents, so why can't we continue to depends on it directly?

LGTM otherwise.

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.cc (right):

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.cc:41: if (entry) {
Nit: Reverse conditional and early-return to avoid needing {} and indent less
code

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.cc:83: return
ChromeSecurityStateModelClient::FromWebContents(web_contents)
Nit: Shorter:

  auto* client = ChromeSecurityStateModelClient::FromWebContents(web_contents);
  return client->GetSecurityInfo().security_level;

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.cc:121: : search_terms;
Nit: Shorter:

  bool extract_search_terms =
      security_level == security_state::SecurityStateModel::NONE ||
      security_level == security_state::SecurityStateModel::SECURITY_ERROR;
  return extract_search_terms ? search_terms : base::string16();

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/toolbar_model_delegate.h (right):

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/toolbar_model_delegate.h:27: typedef
security_state::SecurityStateModel::SecurityLevel SecurityLevel;
Nit: Prefer type aliases to typedefs

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/toolbar_model_delegate.h:39: // Returns true and set
|url| to the current navigation entry URL if it
Nit: sets

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/toolbar_model_delegate.h:44: // in the location bar.
Nit: should be displayed in

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/toolbar_model_impl.cc (right):

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/toolbar_model_impl.cc:92: return url;
Nit: Shorter:

  return delegate_->GetURL(&url) ? url : GURL(url::kAboutBlankURL);

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/toolbar_model_impl.cc:107: return
delegate_->GetSecurityLevel();
Nit: Shorter:

  return (input_in_progress() && !ignore_editing) ?
    SecurityStateModel::NONE : delegate_->GetSecurityLevel();

[sdefresne, 2016-02-03 13:14:17]

I'm injecting content::kMaxURLDisplayChars because I want to reuse
ToolbarModelImpl with iOS and iOS cannot depends on src/content (thus cannot use
content::kMaxURLDisplayChars).

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.cc (right):

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.cc:41: if (entry) {
On 2016/02/02 at 23:51:30, Peter Kasting wrote:
> Nit: Reverse conditional and early-return to avoid needing {} and indent less
code

Done.

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.cc:83: return
ChromeSecurityStateModelClient::FromWebContents(web_contents)
On 2016/02/02 at 23:51:30, Peter Kasting wrote:
> Nit: Shorter:
> 
>   auto* client =
ChromeSecurityStateModelClient::FromWebContents(web_contents);
>   return client->GetSecurityInfo().security_level;

Done.

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/chrome_toolbar_model_delegate.cc:121: : search_terms;
On 2016/02/02 at 23:51:30, Peter Kasting wrote:
> Nit: Shorter:
> 
>   bool extract_search_terms =
>       security_level == security_state::SecurityStateModel::NONE ||
>       security_level == security_state::SecurityStateModel::SECURITY_ERROR;
>   return extract_search_terms ? search_terms : base::string16();

Done.

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/toolbar_model_delegate.h (right):

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/toolbar_model_delegate.h:27: typedef
security_state::SecurityStateModel::SecurityLevel SecurityLevel;
On 2016/02/02 at 23:51:30, Peter Kasting wrote:
> Nit: Prefer type aliases to typedefs

Done.

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/toolbar_model_delegate.h:44: // in the location bar.
On 2016/02/02 at 23:51:30, Peter Kasting wrote:
> Nit: should be displayed in

Done.

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
File chrome/browser/ui/toolbar/toolbar_model_impl.cc (right):

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/toolbar_model_impl.cc:92: return url;
On 2016/02/02 at 23:51:30, Peter Kasting wrote:
> Nit: Shorter:
> 
>   return delegate_->GetURL(&url) ? url : GURL(url::kAboutBlankURL);

Done.

https://codereview.chromium.org/1653013002/diff/60001/chrome/browser/ui/toolb...
chrome/browser/ui/toolbar/toolbar_model_impl.cc:107: return
delegate_->GetSecurityLevel();
On 2016/02/02 at 23:51:31, Peter Kasting wrote:
> Nit: Shorter:
> 
>   return (input_in_progress() && !ignore_editing) ?
>     SecurityStateModel::NONE : delegate_->GetSecurityLevel();

Done.

[sdefresne, 2016-02-03 13:15:45]

Description was changed from

==========
Abstract ToolbarModelImpl dependencies on //content.

Add following methods to ToolbarModelImpl to abstract the dependencies
on content::WebContents.

- GetURL(): fetches the URL to display, if it exists, or return false,

- ShouldDisplayURL(): returns whether the URL should be displayed in the
  toolbar or not (embedder specific)

- GetSecurityLevel(): returns the security level of the underlying page,
  independently of the editing status of the toolbar,

- GetSearchTerms(): extracts the search terms for the current page from
  the content::WebContents (implementation depends on the embedder),

- GetCertificate(): returns the certificate for the current page, if
  any, otherwise returns null.

Inject the constant content::kMaxURLDisplayChars through the constructor.

BUG=582921
==========

to

==========
Abstract ToolbarModelImpl dependencies on //content.

Add following methods to ToolbarModelImpl to abstract the dependencies
on content::WebContents.

- GetURL(): fetches the URL to display, if it exists, or return false,

- ShouldDisplayURL(): returns whether the URL should be displayed in the
  toolbar or not (embedder specific)

- GetSecurityLevel(): returns the security level of the underlying page,
  independently of the editing status of the toolbar,

- GetSearchTerms(): extracts the search terms for the current page from
  the content::WebContents (implementation depends on the embedder),

- GetCertificate(): returns the certificate for the current page, if
  any, otherwise returns null.

Inject the constant content::kMaxURLDisplayChars through the constructor
of ToolbarModelImpl to allow sharing the code with iOS that cannot depend
on content.

BUG=582921
==========

[sdefresne, 2016-02-03 13:19:51]

Description was changed from

==========
Abstract ToolbarModelImpl dependencies on //content.

Add following methods to ToolbarModelImpl to abstract the dependencies
on content::WebContents.

- GetURL(): fetches the URL to display, if it exists, or return false,

- ShouldDisplayURL(): returns whether the URL should be displayed in the
  toolbar or not (embedder specific)

- GetSecurityLevel(): returns the security level of the underlying page,
  independently of the editing status of the toolbar,

- GetSearchTerms(): extracts the search terms for the current page from
  the content::WebContents (implementation depends on the embedder),

- GetCertificate(): returns the certificate for the current page, if
  any, otherwise returns null.

Inject the constant content::kMaxURLDisplayChars through the constructor
of ToolbarModelImpl to allow sharing the code with iOS that cannot depend
on content.

BUG=582921
==========

to

==========
Abstract ToolbarModelImpl dependencies on //content.

Add following methods to ToolbarModelImpl to abstract the dependencies
on content::WebContents.

- GetURL(): fetches the URL to display, if it exists, or return false,

- ShouldDisplayURL(): returns whether the URL should be displayed in the
  toolbar or not (embedder specific)

- GetSecurityLevel(): returns the security level of the underlying page,
  independently of the editing status of the toolbar,

- GetSearchTerms(): extracts the search terms for the current page from
  the content::WebContents (implementation depends on the embedder),

- GetCertificate(): returns the certificate for the current page, if
  any, otherwise returns null.

Inject the constant content::kMaxURLDisplayChars through the constructor
of ToolbarModelImpl to allow sharing the code with iOS that cannot depend
on content.

TBR=dzhioev@chromium.org
BUG=582921
==========

[sdefresne, 2016-02-03 13:19:51]

sdefresne@chromium.org changed reviewers:
+ dzhioev@chromium.org

[sdefresne, 2016-02-03 13:20:09]

TBR-ing dzhioev@chromium.org for
  chrome/browser/chromeos/login/ui/simple_web_view_dialog.cc
  chrome/browser/chromeos/login/ui/simple_web_view_dialog.h

[sdefresne, 2016-02-03 15:42:40]

Patchset #5 (id:80001) has been deleted

[sdefresne, 2016-02-03 16:10:16]

Patchset #5 (id:100001) has been deleted

[sdefresne, 2016-02-03 18:12:58]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2016-02-03 18:12:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/1653013002/#ps120001
(title: "Address comments")

[commit-bot: I haz the power, 2016-02-03 18:14:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1653013002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1653013002/120001

[commit-bot: I haz the power, 2016-02-03 20:08:05]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-02-03 20:10:43]

Description was changed from

==========
Abstract ToolbarModelImpl dependencies on //content.

Add following methods to ToolbarModelImpl to abstract the dependencies
on content::WebContents.

- GetURL(): fetches the URL to display, if it exists, or return false,

- ShouldDisplayURL(): returns whether the URL should be displayed in the
  toolbar or not (embedder specific)

- GetSecurityLevel(): returns the security level of the underlying page,
  independently of the editing status of the toolbar,

- GetSearchTerms(): extracts the search terms for the current page from
  the content::WebContents (implementation depends on the embedder),

- GetCertificate(): returns the certificate for the current page, if
  any, otherwise returns null.

Inject the constant content::kMaxURLDisplayChars through the constructor
of ToolbarModelImpl to allow sharing the code with iOS that cannot depend
on content.

TBR=dzhioev@chromium.org
BUG=582921
==========

to

==========
Abstract ToolbarModelImpl dependencies on //content.

Add following methods to ToolbarModelImpl to abstract the dependencies
on content::WebContents.

- GetURL(): fetches the URL to display, if it exists, or return false,

- ShouldDisplayURL(): returns whether the URL should be displayed in the
  toolbar or not (embedder specific)

- GetSecurityLevel(): returns the security level of the underlying page,
  independently of the editing status of the toolbar,

- GetSearchTerms(): extracts the search terms for the current page from
  the content::WebContents (implementation depends on the embedder),

- GetCertificate(): returns the certificate for the current page, if
  any, otherwise returns null.

Inject the constant content::kMaxURLDisplayChars through the constructor
of ToolbarModelImpl to allow sharing the code with iOS that cannot depend
on content.

TBR=dzhioev@chromium.org
BUG=582921

Committed: https://crrev.com/4d623d703112ab992b4d48ca98d21ed78f850c68
Cr-Commit-Position: refs/heads/master@{#373317}
==========

[commit-bot: I haz the power, 2016-02-03 20:10:44]

Patchset 5 (id:??) landed as
https://crrev.com/4d623d703112ab992b4d48ca98d21ed78f850c68
Cr-Commit-Position: refs/heads/master@{#373317}