Classification of TODOs in Panel code.

chrome/browser/ui/panels/panel_browser_view.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/panels/panel_browser_view.h"
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "chrome/browser/ui/panels/panel.h"
 #include "chrome/browser/ui/panels/panel_browser_frame_view.h"
@@ skipping 75 matching lines @@
       panel_->manager()->GetNextBrowserWindowToActivate(panel_.get());
   if (browser_window)
     native_window = browser_window->GetNativeHandle();
   else
     native_window = ::GetDesktopWindow();
   if (native_window)
     ::SetForegroundWindow(native_window);
   else
     ::SetFocus(NULL);
 #else
-  // TODO(jianli): to be implemented for other platform.
+  // TODO(jianli): Implement for USE_AURA.

[Dmitry Titov, 2011/11/02 22:15:01]

Didn't want a bug for this, maybe we shoudl just remove this because
NOTIMPLEMENTED() is enough?

[jennb, 2011/11/02 22:25:31]

Agree w/ removing TODO.

[Dmitry Titov, 2011/11/02 23:32:19]

Done.

+  NOTIMPLEMENTED();
   BrowserView::Deactivate();
 #endif
 }
 
 bool PanelBrowserView::CanResize() const {
   return false;
 }
 
 bool PanelBrowserView::CanMaximize() const {
   return false;
@@ skipping 33 matching lines @@
   *bounds = bounds_;
   *show_state = ui::SHOW_STATE_NORMAL;
   return true;
 }
 
 void PanelBrowserView::OnWidgetActivationChanged(views::Widget* widget,
                                                  bool active) {
   ::BrowserView::OnWidgetActivationChanged(widget, active);
 
 #if defined(USE_AURA)
-  // TODO(beng):
+  // TODO(beng): Implement for USE_AURA.
   NOTIMPLEMENTED();
   bool focused = active;
 #elif defined(OS_WIN)
   // The panel window is in focus (actually accepting keystrokes) if it is
   // active and belongs to a foreground application.
   bool focused = active &&
       GetFrameView()->GetWidget()->GetNativeView() == ::GetForegroundWindow();
 #else
-  // TODO(jianli): Investigate focus behavior for ChromeOS
+  NOTREACHED();  // View-based Panels are not enabled anywhere else.

[jennb, 2011/11/02 22:25:31]

Can we delete the #else clause completely and convert the #elif into #else?

[Dmitry Titov, 2011/11/02 23:32:19]

Done.
Made it consistent with the case in Deactivate(): 


#if defined(OS_WIN) && !defined(USE_AURA)
...
#else
  NOTIMPLEMENTED();
#endif

   bool focused = active;
 #endif
 
   if (focused_ == focused)
     return;
   focused_ = focused;
 
   GetFrameView()->OnFocusChanged(focused);
 
   if (focused_) {
@@ skipping 334 matching lines @@
   panel_browser_view_->OnTitlebarMousePressed(point);
 }
 
 void NativePanelTestingWin::ReleaseMouseButtonTitlebar() {
   panel_browser_view_->OnTitlebarMouseReleased();
 }
 
 void NativePanelTestingWin::DragTitlebar(int delta_x, int delta_y) {
   // TODO(jianli): Need a comment here that explains why we use
   // mouse_pressed_point_ and not current bounds as obtained by
-  // GetRestoredBounds().
+  // GetRestoredBounds(). http://crbug.com/102730
   panel_browser_view_->OnTitlebarMouseDragged(gfx::Point(
       panel_browser_view_->mouse_pressed_point_.x() + delta_x,
       panel_browser_view_->mouse_pressed_point_.y() + delta_y));
 }
 
 void NativePanelTestingWin::CancelDragTitlebar() {
   panel_browser_view_->OnTitlebarMouseCaptureLost();
 }
 
 void NativePanelTestingWin::FinishDragTitlebar() {
@@ skipping 21 matching lines @@
 }
 
 bool NativePanelTestingWin::IsWindowSizeKnown() const {
   return true;
 }
 
 bool NativePanelTestingWin::IsAnimatingBounds() const {
   return panel_browser_view_->bounds_animator_.get() &&
          panel_browser_view_->bounds_animator_->is_animating();
 }