From ee1f3acf7b7e15a58425d071e56f25a845d61a56 Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Fri, 31 Mar 2017 21:27:06 +0200 Subject: [PATCH 1/2] Don't use anonymous namespace in header file. Anonymous namespace should be forbidden in header files even for the forward declarations: * As declarations defined in anonymous namespace are internal linkage, each translation unit which includes this header will get unique copy, which wastes space. * It is easy to violate C++ ODR rule. Consider the following "foo.h": ```cpp namespace { class Foo; } class Bar { public: Foo* getFoo(); Foo* foo; } ``` If the 'foo.h' is included in multiple `.cc` files, the compiler will put `Foo` into a different anonymous namespace in each `.cc`, which means there are different definitions of `Foo` in the program (a violation of the ODR). --- .../views/inspectable_web_contents_view_views.cc | 5 +++-- .../views/inspectable_web_contents_view_views.h | 11 ++--------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/brightray/browser/views/inspectable_web_contents_view_views.cc b/brightray/browser/views/inspectable_web_contents_view_views.cc index 6a9997f232..073da870e5 100644 --- a/brightray/browser/views/inspectable_web_contents_view_views.cc +++ b/brightray/browser/views/inspectable_web_contents_view_views.cc @@ -178,7 +178,7 @@ void InspectableWebContentsViewViews::SetIsDocked(bool docked) { views::Widget::InitParams params; params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET; - params.delegate = GetDevToolsWindowDelegate(); + params.delegate = devtools_window_delegate_; params.bounds = inspectable_web_contents()->GetDevToolsBounds(); #if defined(USE_X11) @@ -203,7 +203,8 @@ void InspectableWebContentsViewViews::SetContentsResizingStrategy( void InspectableWebContentsViewViews::SetTitle(const base::string16& title) { if (devtools_window_) { - GetDevToolsWindowDelegate()->SetWindowTitle(title); + static_cast(devtools_window_delegate_) + ->SetWindowTitle(title); devtools_window_->UpdateWindowTitle(); } } diff --git a/brightray/browser/views/inspectable_web_contents_view_views.h b/brightray/browser/views/inspectable_web_contents_view_views.h index 1af53e92fd..1d2963fd19 100644 --- a/brightray/browser/views/inspectable_web_contents_view_views.h +++ b/brightray/browser/views/inspectable_web_contents_view_views.h @@ -10,14 +10,11 @@ namespace views { class WebView; class Widget; +class WidgetDelegate; } namespace brightray { -namespace { // NOLINT -class DevToolsWindowDelegate; -} - class InspectableWebContentsImpl; class InspectableWebContentsViewViews : public InspectableWebContentsView, @@ -27,10 +24,6 @@ class InspectableWebContentsViewViews : public InspectableWebContentsView, InspectableWebContentsImpl* inspectable_web_contents_impl); ~InspectableWebContentsViewViews(); - DevToolsWindowDelegate* GetDevToolsWindowDelegate() const { - return devtools_window_delegate_; - } - // InspectableWebContentsView: views::View* GetView() override; views::View* GetWebView() override; @@ -61,7 +54,7 @@ class InspectableWebContentsViewViews : public InspectableWebContentsView, DevToolsContentsResizingStrategy strategy_; bool devtools_visible_; - DevToolsWindowDelegate* devtools_window_delegate_; + views::WidgetDelegate* devtools_window_delegate_; DISALLOW_COPY_AND_ASSIGN(InspectableWebContentsViewViews); }; From e80a9bbb932c552b030d082060a32bc81d33eefe Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Sat, 1 Apr 2017 14:56:51 +0200 Subject: [PATCH 2/2] Move title_ to InspectableWebContentsViewVies to get rid of cast. --- .../views/inspectable_web_contents_view_views.cc | 14 +++++--------- .../views/inspectable_web_contents_view_views.h | 3 +++ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/brightray/browser/views/inspectable_web_contents_view_views.cc b/brightray/browser/views/inspectable_web_contents_view_views.cc index 073da870e5..8f52ca5699 100644 --- a/brightray/browser/views/inspectable_web_contents_view_views.cc +++ b/brightray/browser/views/inspectable_web_contents_view_views.cc @@ -24,8 +24,7 @@ class DevToolsWindowDelegate : public views::ClientView, : views::ClientView(widget, view), shell_(shell), view_(view), - widget_(widget), - title_(base::ASCIIToUTF16("Developer Tools")) { + widget_(widget) { // A WidgetDelegate should be deleted on DeleteDelegate. set_owned_by_client(); @@ -34,15 +33,13 @@ class DevToolsWindowDelegate : public views::ClientView, } virtual ~DevToolsWindowDelegate() {} - void SetWindowTitle(const base::string16& title) { title_ = title; } - // views::WidgetDelegate: void DeleteDelegate() override { delete this; } views::View* GetInitiallyFocusedView() override { return view_; } bool CanResize() const override { return true; } bool CanMaximize() const override { return true; } bool CanMinimize() const override { return true; } - base::string16 GetWindowTitle() const override { return title_; } + base::string16 GetWindowTitle() const override { return shell_->GetTitle(); } gfx::ImageSkia GetWindowAppIcon() override { return GetWindowIcon(); } gfx::ImageSkia GetWindowIcon() override { return icon_; } views::Widget* GetWidget() override { return widget_; } @@ -62,7 +59,6 @@ class DevToolsWindowDelegate : public views::ClientView, InspectableWebContentsViewViews* shell_; views::View* view_; views::Widget* widget_; - base::string16 title_; gfx::ImageSkia icon_; DISALLOW_COPY_AND_ASSIGN(DevToolsWindowDelegate); @@ -82,7 +78,8 @@ InspectableWebContentsViewViews::InspectableWebContentsViewViews( contents_web_view_(nullptr), devtools_web_view_(new views::WebView(nullptr)), devtools_visible_(false), - devtools_window_delegate_(nullptr) { + devtools_window_delegate_(nullptr), + title_(base::ASCIIToUTF16("Developer Tools")) { set_owned_by_client(); if (inspectable_web_contents_->GetWebContents()->GetNativeView()) { @@ -203,8 +200,7 @@ void InspectableWebContentsViewViews::SetContentsResizingStrategy( void InspectableWebContentsViewViews::SetTitle(const base::string16& title) { if (devtools_window_) { - static_cast(devtools_window_delegate_) - ->SetWindowTitle(title); + title_ = title; devtools_window_->UpdateWindowTitle(); } } diff --git a/brightray/browser/views/inspectable_web_contents_view_views.h b/brightray/browser/views/inspectable_web_contents_view_views.h index 1d2963fd19..9094b9f35d 100644 --- a/brightray/browser/views/inspectable_web_contents_view_views.h +++ b/brightray/browser/views/inspectable_web_contents_view_views.h @@ -40,6 +40,8 @@ class InspectableWebContentsViewViews : public InspectableWebContentsView, return inspectable_web_contents_; } + const base::string16& GetTitle() const { return title_; } + private: // views::View: void Layout() override; @@ -55,6 +57,7 @@ class InspectableWebContentsViewViews : public InspectableWebContentsView, DevToolsContentsResizingStrategy strategy_; bool devtools_visible_; views::WidgetDelegate* devtools_window_delegate_; + base::string16 title_; DISALLOW_COPY_AND_ASSIGN(InspectableWebContentsViewViews); };