OpaqueBrowserFrameViewPlatformSpecific::Create no longer returns NULL.

OpaqueBrowserFrameViewPlatformSpecific::Create no longer returns NULL.

This avoids duplicating the default logic for all of its methods.
Previously, every time a method was called, we had to test for NULL and
assume the default value for that method (even though the default method
implementation already exists; it was never used). Now the caller
assumes platform_observer_ is non-NULL (via a DCHECK) and it is the base
class's responsibility to provide default methods.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243819

[James Cook, 2014-01-06 17:54:12]

LGTM

https://codereview.chromium.org/124893002/diff/1/chrome/browser/ui/views/fram...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/124893002/diff/1/chrome/browser/ui/views/fram...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:431:
DCHECK(platform_observer_);
nit: I don't think you need to DCHECK these pointers, since the fact that you
immediately dereference them tells the reader you don't expect them to ever be
NULL.

[Matt Giuca, 2014-01-06 23:51:40]

https://codereview.chromium.org/124893002/diff/1/chrome/browser/ui/views/fram...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/124893002/diff/1/chrome/browser/ui/views/fram...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:431:
DCHECK(platform_observer_);
On 2014/01/06 17:54:12, James Cook wrote:
> nit: I don't think you need to DCHECK these pointers, since the fact that you
> immediately dereference them tells the reader you don't expect them to ever be
> NULL.

Done.

[commit-bot: I haz the power, 2014-01-07 00:36:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mgiuca@chromium.org/124893002/80001

[commit-bot: I haz the power, 2014-01-07 00:36:35]

Failed to apply patch for
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
  Hunk #1 FAILED at 428.
  Hunk #2 FAILED at 439.
  Hunk #3 succeeded at 464 (offset -6 lines).
  2 out of 3 hunks FAILED -- saving rejects to file
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc.rej

Patch:       chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
Index: chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
diff --git a/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
b/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
index
19ad04299e523b4456ea48419dfa68b213a8b27f..f370c85a8039cadb8d6536f4986bf91686247f51
100644
--- a/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
+++ b/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
@@ -428,10 +428,8 @@ void OpaqueBrowserFrameView::Observe(
 
 bool OpaqueBrowserFrameView::ShouldShowWindowIcon() const {
   views::WidgetDelegate* delegate = frame()->widget_delegate();
-  // ShouldShowTitleBar defaults to true.
-  if (platform_observer_ && !platform_observer_->ShouldShowTitleBar())
-    return false;
-  return delegate && delegate->ShouldShowWindowIcon();
+  return platform_observer_->ShouldShowTitleBar() && delegate &&
+         delegate->ShouldShowWindowIcon();
 }
 
 bool OpaqueBrowserFrameView::ShouldShowWindowTitle() const {
@@ -439,10 +437,8 @@ bool OpaqueBrowserFrameView::ShouldShowWindowTitle() const
{
   // a window is being destroyed.
   // See more discussion at http://crosbug.com/8958
   views::WidgetDelegate* delegate = frame()->widget_delegate();
-  // ShouldShowTitleBar defaults to true.
-  if (platform_observer_ && !platform_observer_->ShouldShowTitleBar())
-    return false;
-  return delegate && delegate->ShouldShowWindowTitle();
+  return platform_observer_->ShouldShowTitleBar() && delegate &&
+         delegate->ShouldShowWindowTitle();
 }
 
 base::string16 OpaqueBrowserFrameView::GetWindowTitle() const {
@@ -470,8 +466,6 @@ gfx::Size
OpaqueBrowserFrameView::GetBrowserViewMinimumSize() const {
 bool OpaqueBrowserFrameView::ShouldShowCaptionButtons() const {
   if (!OpaqueBrowserFrameViewLayout::ShouldAddDefaultCaptionButtons())
     return false;
-  if (!platform_observer_)
-    return true;
   return platform_observer_->ShouldShowCaptionButtons();
 }

[commit-bot: I haz the power, 2014-01-08 23:03:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mgiuca@chromium.org/124893002/80001

[commit-bot: I haz the power, 2014-01-09 00:24:47]

Retried try job too often on linux_rel for step(s) cacheinvalidation_unittests,
cc_unittests, check_deps, check_deps2git, chromedriver_unittests,
components_unittests, content_browsertests, content_unittests, crypto_unittests,
google_apis_unittests, gpu_unittests, ipc_tests, jingle_unittests,
media_unittests, nacl_integration, ppapi_unittests, printing_unittests,
remoting_unittests, sandbox_linux_unittests, sql_unittests,
sync_integration_tests, sync_unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2014-01-09 06:28:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mgiuca@chromium.org/124893002/80001

[commit-bot: I haz the power, 2014-01-09 08:32:45]

Change committed as 243819