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

Issue 56094: Fix a case where non-tabbed browser windows could open tabs. Also includes UI... (Closed)

Created:
11 years, 8 months ago by Ben Goodger (Google)
Modified:
9 years, 6 months ago
Reviewers:
ananta, sky
CC:
chromium-reviews_googlegroups.com
Base URL:
svn://chrome-svn.corp.google.com/chrome/trunk/src/
Visibility:
Public.

Description

Fix a case where non-tabbed browser windows could open tabs. Also includes UI test for this. http://crbug.com/8472 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=13036

Patch Set 1 #

Total comments: 3

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 1

Patch Set 4 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+108 lines, -15 lines) Patch
M chrome/browser/browser.h View 3 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/browser/browser.cc View 1 2 3 1 chunk +19 lines, -15 lines 0 comments Download
A chrome/browser/browser_unittest.cc View 3 1 chunk +83 lines, -0 lines 0 comments Download
M chrome/test/unit/unittests.vcproj View 3 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Ben Goodger (Google)
11 years, 8 months ago (2009-03-31 00:41:48 UTC) #1
ananta
Please check if you need to update the reference build for chrome.
11 years, 8 months ago (2009-03-31 01:18:50 UTC) #2
sky
http://codereview.chromium.org/56094/diff/1/6 File chrome/browser/automation/automation_provider.cc (right): http://codereview.chromium.org/56094/diff/1/6#newcode1007 Line 1007: Send(reply_message); How come you send the response immediately? ...
11 years, 8 months ago (2009-03-31 15:39:25 UTC) #3
sky
LGTM
11 years, 8 months ago (2009-03-31 18:59:24 UTC) #4
Ben Goodger (Google)
Hi Scott, Here's the updated version with an in process browser test. Ananta, Since it ...
11 years, 8 months ago (2009-03-31 23:28:01 UTC) #5
sky
11 years, 8 months ago (2009-03-31 23:29:57 UTC) #6
LGTM

http://codereview.chromium.org/56094/diff/25/29
File chrome/browser/browser_unittest.cc (right):

http://codereview.chromium.org/56094/diff/25/29#newcode24
Line 24: // This test loads a page with frames and starts FindInPage requests
Something tells me you don't mean FindInPage here;)

Powered by Google App Engine
This is Rietveld 408576698