Support "Report Issue" in case of no-browser windows in ChromeOS.

chrome/browser/ui/webui/feedback_ui.cc

 // Copyright (c) 2012 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/webui/feedback_ui.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 368 matching lines @@
             &query_str, 0, kTimestampParameter, "");
         timestamp_ = query_str;
         continue;
       }
 #endif
     }
   }
 
   // If we don't have a page url specified, get it from the tab index.
   if (custom_page_url.empty()) {
-    if (session_id == -1 || index == -1)
+    if (session_id == -1)
       return false;
 
+#if !defined(OS_CHROMEOS)

[rkc1, 2012/07/09 18:23:22]

We set the index in ShowFeedbackView for all platforms, so why would we want to
make the index check only non-cros?

[Jun Mukai, 2012/07/10 11:08:20]

That's browser->active_index() in line 178, right?  I think most of platforms
expects that a tab is active for reporting issue.

However, in ChromeOS only, the user may want to open the feedback UI even though
there are no browser windows, for reporting the desktop itself.  In that case,
active_index() returns -1.  That's why I moved the check only for non-ChromeOS.

Can we just allow for all platforms?  I think either would be fine because the
index has to be set correctly in non-ChromeOS too.

[rkc1, 2012/07/10 17:20:10]

Yes, let's remove it for all platforms. There are an increasing number of cases
where we may not actually have the tab id; in which case our code already
compensates by leaving the target tab url as empty.

[Jun Mukai, 2012/07/11 12:42:39]

Sure, then removed the check itself.  now index==-1 is valid here.

On 2012/07/10 17:20:10, g_rkc wrote:

+    if (index == -1)
+      return false;
+#endif
+
     Browser* browser = browser::FindBrowserWithID(session_id);
     // Sanity checks.
     if (!browser || index >= browser->tab_count())
       return false;
 
-    WebContents* target_tab = chrome::GetWebContentsAt(browser, index);
-    if (target_tab)
-      target_tab_url_ = target_tab->GetURL().spec();
+    if (index >= 0) {
+      WebContents* target_tab = chrome::GetWebContentsAt(browser, index);
+      if (target_tab)
+        target_tab_url_ = target_tab->GetURL().spec();
+    }
 
     // Note: We don't need to setup a screenshot source if we're using a
     // custom page URL since we were invoked from JS/an extension, in which
     // case we don't actually have a screenshot anyway.
     SetupScreenshotsSource();
   } else {
     target_tab_url_ = custom_page_url;
   }
 
   return true;
@@ skipping 235 matching lines @@
   size_t sort_size = std::min(max_saved, screenshot_filepaths.size());
   std::partial_sort(screenshot_filepaths.begin(),
                     screenshot_filepaths.begin() + sort_size,
                     screenshot_filepaths.end(),
                     ScreenshotTimestampComp);
   for (size_t i = 0; i < sort_size; ++i)
     saved_screenshots->push_back(std::string(kSavedScreenshotsUrl) +
                                    screenshot_filepaths[i]);
 }
 #endif