Set display mode for browser

Set display mode for browser

This CL provides content::WebContentsDelegate::GetDisplayMode() implementation for the Browser class.

BUG=521999
BUG=471703
Committed: https://crrev.com/76208cb6482119683fab256ca5bfbdd859094700
Cr-Commit-Position: refs/heads/master@{#347620}

[Mikhail, 2015-08-31 08:39:15]

mikhail.pozdnyakov@intel.com changed reviewers:
+ msw@chromium.org, sky@chromium.org

[Mikhail, 2015-08-31 08:41:41]

PTAL

[sky, 2015-08-31 15:57:27]

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser.h
File chrome/browser/ui/browser.h (right):

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser.h...
chrome/browser/ui/browser.h:617: blink::WebDisplayMode GetDisplayMode(
Generally we want a block of overrides like this (starts on 543) there are no
newlines separating functions.

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
File chrome/browser/ui/browser_browsertest.cc (right):

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:3128: public:
run git cl format as your spacing is off.

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:3142: };
DISALLOW...

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:3147: 
remove newline.

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:3148: base::string16 js_funtcion =
function

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:3151: scoped_ptr<JSBooleanResultGetter>
js_result_getter(
Create on stack.

[Mikhail, 2015-09-01 08:13:46]

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser.h
File chrome/browser/ui/browser.h (right):

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser.h...
chrome/browser/ui/browser.h:617: blink::WebDisplayMode GetDisplayMode(
On 2015/08/31 15:57:27, sky wrote:
> Generally we want a block of overrides like this (starts on 543) there are no
> newlines separating functions.

Done.

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
File chrome/browser/ui/browser_browsertest.cc (right):

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:3128: public:
On 2015/08/31 15:57:27, sky wrote:
> run git cl format as your spacing is off.

Done.

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:3142: };
On 2015/08/31 15:57:27, sky wrote:
> DISALLOW...

Done.

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:3147: 
On 2015/08/31 15:57:27, sky wrote:
> remove newline.

Done.

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:3148: base::string16 js_funtcion =
On 2015/08/31 15:57:27, sky wrote:
> function

Done.

https://codereview.chromium.org/1323733002/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:3151: scoped_ptr<JSBooleanResultGetter>
js_result_getter(
On 2015/08/31 15:57:27, sky wrote:
> Create on stack.

Done.

[sky, 2015-09-01 16:25:29]

LGTM

[Mikhail, 2015-09-02 05:50:14]

The CQ bit was checked by mikhail.pozdnyakov@intel.com

[commit-bot: I haz the power, 2015-09-02 05:50:19]

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

[commit-bot: I haz the power, 2015-09-02 06:16:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-02 06:16:52]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mikhail, 2015-09-03 15:11:23]

On 2015/09/02 06:16:52, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

The unittests fail because TestBrowserWindow class is not initialized enough to
work with Browser: ExclusiveAccessContext is missing there. This is fixed at
https://codereview.chromium.org/1312773005/. 
@sky could you pls have a look? I made a separate CL for that, since IMO it is a
separate issue but I can merge everything into one patch (within current CL) if
you wish. Thanks

[sky, 2015-09-03 17:57:45]

https://codereview.chromium.org/1323733002/diff/20001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/1323733002/diff/20001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:1740: if
(exclusive_access_manager_->context()->IsFullscreen())
Actually, one question that is now worth asking (I was tempted before, but it
seemed like they are routed to the same place and it wouldn't matter). Can you
use window_->IsFullscreen(), which I believe is routed to the same place and
shouldn't have the timing issues you ran into?

[Mikhail, 2015-09-04 08:31:09]

On 2015/09/03 17:57:45, sky wrote:
>
https://codereview.chromium.org/1323733002/diff/20001/chrome/browser/ui/brows...
> File chrome/browser/ui/browser.cc (right):
> 
>
https://codereview.chromium.org/1323733002/diff/20001/chrome/browser/ui/brows...
> chrome/browser/ui/browser.cc:1740: if
> (exclusive_access_manager_->context()->IsFullscreen())
> Actually, one question that is now worth asking (I was tempted before, but it
> seemed like they are routed to the same place and it wouldn't matter). Can you
> use window_->IsFullscreen(), which I believe is routed to the same place and
> shouldn't have the timing issues you ran into?

Thanks for the proposal, that indeed solved all the issues!

[sky, 2015-09-04 14:58:36]

LGTM

https://codereview.chromium.org/1323733002/diff/40001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/1323733002/diff/40001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:110: #include
"chrome/browser/ui/exclusive_access/exclusive_access_context.h"
You can remove this include now.

[Mikhail, 2015-09-07 09:54:42]

https://codereview.chromium.org/1323733002/diff/40001/chrome/browser/ui/brows...
File chrome/browser/ui/browser.cc (right):

https://codereview.chromium.org/1323733002/diff/40001/chrome/browser/ui/brows...
chrome/browser/ui/browser.cc:110: #include
"chrome/browser/ui/exclusive_access/exclusive_access_context.h"
On 2015/09/04 14:58:36, sky wrote:
> You can remove this include now.
Indeed! Thanks!

[Mikhail, 2015-09-07 09:54:55]

The CQ bit was checked by mikhail.pozdnyakov@intel.com

[Mikhail, 2015-09-07 09:54:56]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1323733002/#ps60001
(title: "Removed unneeded include.")

[commit-bot: I haz the power, 2015-09-07 09:55:03]

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

[commit-bot: I haz the power, 2015-09-07 10:31:48]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-07 10:32:55]

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