Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(360)

Issue 7745049: Reverts debugging code added for 91396. I've tracked down the bug and (Closed)

Created:
9 years, 4 months ago by sky
Modified:
9 years, 4 months ago
CC:
chromium-reviews, dhollowa
Visibility:
Public.

Description

Reverts debugging code added for 91396. I've tracked down the bug and will do a patch after this so that it can be cleanly merged over. TBR since it's just a revert. BUG=91396 TEST=none R=ben@chromium.org TBR=ben@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98451

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -39 lines) Patch
M chrome/browser/ui/views/chrome_views_delegate.cc View 3 chunks +3 lines, -10 lines 0 comments Download
M views/widget/widget.h View 3 chunks +0 lines, -20 lines 0 comments Download
M views/widget/widget.cc View 2 chunks +1 line, -9 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
sky
9 years, 4 months ago (2011-08-26 16:04:54 UTC) #1
Ben Goodger (Google)
9 years, 4 months ago (2011-08-26 16:05:30 UTC) #2
LGTM

On Fri, Aug 26, 2011 at 9:04 AM, <sky@chromium.org> wrote:

> Reviewers: Ben Goodger (Google),
>
> Description:
> Reverts debugging code added for 91396. I've tracked down the bug and
> will do a patch after this so that it can be cleanly merged over.
>
> TBR since it's just a revert.
>
> BUG=91396
> TEST=none
> R=ben@chromium.org
> TBR=ben@chromium.org
>
>
> Please review this at
http://codereview.chromium.**org/7745049/<http://codereview.chromium.org/7745...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M chrome/browser/ui/views/**chrome_views_delegate.cc
>  M views/widget/widget.h
>  M views/widget/widget.cc
>
>
> Index: chrome/browser/ui/views/**chrome_views_delegate.cc
> diff --git a/chrome/browser/ui/views/**chrome_views_delegate.cc
> b/chrome/browser/ui/views/**chrome_views_delegate.cc
> index e198f84547c275a066f191e7709c70**c750e1ab7e..**
> a471e6f06e4cdacd996f65c2b3f529**b8bc624013 100644
> --- a/chrome/browser/ui/views/**chrome_views_delegate.cc
> +++ b/chrome/browser/ui/views/**chrome_views_delegate.cc
> @@ -8,7 +8,6 @@
>  #include "base/string_util.h"
>  #include "base/utf_string_conversions.**h"
>  #include "chrome/browser/browser_**process.h"
> -#include "chrome/browser/browser_**shutdown.h"
>  #include "chrome/browser/prefs/pref_**service.h"
>  #include "chrome/browser/prefs/scoped_**user_pref_update.h"
>  #include "chrome/browser/profiles/**profile_manager.h"
> @@ -33,16 +32,13 @@ namespace {
>  // been initialized.
>  // TODO(mirandac): This function will also separate windows by profile in
> a
>  // multi-profile environment.
> -PrefService* GetPrefsForWindow(const views::Widget* window,
> -                               bool* using_local_state) {
> +PrefService* GetPrefsForWindow(const views::Widget* window) {
>   Profile* profile = reinterpret_cast<Profile*>(
>       window->**GetNativeWindowProperty(**Profile::kProfileKey));
>   if (!profile) {
>     // Use local state for windows that have no explicit profile.
> -    *using_local_state = true;
>     return g_browser_process->local_**state();
>   }
> -  *using_local_state = false;
>   return profile->GetPrefs();
>  }
>
> @@ -66,14 +62,11 @@ void ChromeViewsDelegate::**SaveWindowPlacement(const
> views::Widget* window,
>                                               const std::wstring&
> window_name,
>                                               const gfx::Rect& bounds,
>                                               bool maximized) {
> -  bool using_local_state = false;
> -  PrefService* prefs = GetPrefsForWindow(window, &using_local_state);
> +  PrefService* prefs = GetPrefsForWindow(window);
>   if (!prefs)
>     return;
>
> -  CHECK(prefs->FindPreference(**WideToUTF8(window_name).c_str(**))) << "
> " <<
> -      browser_shutdown::**GetShutdownType() << " " << using_local_state
> << " " <<
> -      window->destroy_state();
> +  DCHECK(prefs->FindPreference(**WideToUTF8(window_name).c_str(**)));
>   DictionaryPrefUpdate update(prefs, WideToUTF8(window_name).c_str(**));
>   DictionaryValue* window_preferences = update.Get();
>   window_preferences->**SetInteger("left", bounds.x());
> Index: views/widget/widget.cc
> diff --git a/views/widget/widget.cc b/views/widget/widget.cc
> index 7a42923d6fc6aee1feef095bd245d0**d6b8b208b4..**
> 83401991184d0e0fd116b8e12c7973**3b81c71bfc 100644
> --- a/views/widget/widget.cc
> +++ b/views/widget/widget.cc
> @@ -153,13 +153,10 @@ Widget::Widget()
>       saved_maximized_state_(false),
>       minimum_size_(100, 100),
>       focus_on_creation_(true),
> -      is_top_level_(false),
> -      destroy_state_(DESTROY_STATE_**NONE) {
> +      is_top_level_(false) {
>  }
>
>  Widget::~Widget() {
> -  destroy_state_ = DESTROY_STATE_DELETED;
> -
>   while (!event_stack_.empty()) {
>     event_stack_.top()->reset();
>     event_stack_.pop();
> @@ -865,17 +862,12 @@ void Widget::OnNativeWidgetCreated(**) {
>
>  void Widget::**OnNativeWidgetDestroying() {
>   FOR_EACH_OBSERVER(Observer, observers_, OnWidgetClosing(this));
> -  if (destroy_state_ == DESTROY_STATE_NONE)
> -    destroy_state_ = DESTROY_STATE_IN_DESTROYING;
>   if (non_client_view_)
>     non_client_view_->**WindowClosing();
>   widget_delegate_->**WindowClosing();
>  }
>
>  void Widget::**OnNativeWidgetDestroyed() {
> -  if (destroy_state_ == DESTROY_STATE_IN_DESTROYING ||
> -      destroy_state_ == DESTROY_STATE_NONE)
> -    destroy_state_ = DESTROY_STATE_DESTROYED;
>   widget_delegate_->**DeleteDelegate();
>   widget_delegate_ = NULL;
>  }
> Index: views/widget/widget.h
> diff --git a/views/widget/widget.h b/views/widget/widget.h
> index 70e527c57b694a787bc2d9d9144682**fac2c3b9ef..**
> a85b6190cbd18e07de393c7e86065b**46eccbb1b6 100644
> --- a/views/widget/widget.h
> +++ b/views/widget/widget.h
> @@ -101,20 +101,6 @@ class VIEWS_EXPORT Widget : public internal::**
> NativeWidgetDelegate,
>
>   typedef std::set<Widget*> Widgets;
>
> -  enum DestroyState {
> -    // The default, everything is good and alive.
> -    DESTROY_STATE_NONE = 1,
> -
> -    // Set once OnNativeWidgetDestroying has been invoked.
> -    DESTROY_STATE_IN_DESTROYING,
> -
> -    // Set once OnNativeWidgetDestroyed has been invoked.
> -    DESTROY_STATE_DESTROYED,
> -
> -    // Set once the destructor is invoked.
> -    DESTROY_STATE_DELETED,
> -  };
> -
>   enum FrameType {
>     FRAME_TYPE_DEFAULT,         // Use whatever the default would be.
>     FRAME_TYPE_FORCE_CUSTOM,    // Force the custom frame.
> @@ -578,8 +564,6 @@ class VIEWS_EXPORT Widget : public internal::**
> NativeWidgetDelegate,
>   // TYPE_CONTROL and TYPE_TOOLTIP is not considered top level.
>   bool is_top_level() const { return is_top_level_; }
>
> -  DestroyState destroy_state() const { return destroy_state_; }
> -
>   // Overridden from NativeWidgetDelegate:
>   virtual bool IsModal() const OVERRIDE;
>   virtual bool IsDialogBox() const OVERRIDE;
> @@ -743,10 +727,6 @@ class VIEWS_EXPORT Widget : public internal::**
> NativeWidgetDelegate,
>   // Factory used to create Compositors. Settable by tests.
>   static ui::Compositor*(*compositor_**factory_)();
>
> -  // Tracks destroy state.
> -  // TODO(sky): remove this, used in tracking 91396.
> -  DestroyState destroy_state_;
> -
>   DISALLOW_COPY_AND_ASSIGN(**Widget);
>  };
>
>
>
>

Powered by Google App Engine
This is Rietveld 408576698