Issue 12593012: Instant: Possible fix for crash.

Issue 12593012: Instant: Possible fix for crash. (Closed)

Created:
7 years, 9 months ago by sreeram

Modified:
7 years, 9 months ago

Reviewers:
Jered

CC:
chromium-reviews, melevin, sreeram, gideonwald, dominich, David Black, samarth+watch_chromium.org, Jered

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Instant: Possible fix for crash. BUG=204925 NOTRY=true TBR=jered@chromium.org TEST=See if the crashes stop. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188644

Patch Set 2 : . #

Total comments: 2

Created: 7 years, 9 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+2 lines, -1 line)			Patch
	M	chrome/browser/instant/instant_controller.cc	View	1	1 chunk	+2 lines, -1 line	2 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sreeram@chromium.org/12593012/2001

7 years, 9 months ago (2013-03-17 17:27:25 UTC) #1

Jered

lgtm https://chromiumcodereview.appspot.com/12593012/diff/2001/chrome/browser/instant/instant_controller.cc File chrome/browser/instant/instant_controller.cc (right): https://chromiumcodereview.appspot.com/12593012/diff/2001/chrome/browser/instant/instant_controller.cc#newcode634 chrome/browser/instant/instant_controller.cc:634: last_suggestion_.text.clear(); Why do this?

7 years, 9 months ago (2013-03-17 17:43:38 UTC) #3

sreeram

7 years, 9 months ago (2013-03-17 17:53:27 UTC) #4

Message was sent while issue was closed.

https://chromiumcodereview.appspot.com/12593012/diff/2001/chrome/browser/inst...
File chrome/browser/instant/instant_controller.cc (right):

https://chromiumcodereview.appspot.com/12593012/diff/2001/chrome/browser/inst...
chrome/browser/instant/instant_controller.cc:634: last_suggestion_.text.clear();
On 2013/03/17 17:43:38, Jered wrote:
> Why do this?

Line 636 calls Focus() on the web contents. I assumed this might lead to
OmniboxLostFocus(), which will try to "commit" the suggested text (gray text),
since all of the conditions in that 'if' statement will be true.

Incidentally, this also fixes http://crbug.com/196811.

All this is in theory. I haven't actually duplicated the crash, or verified that
this actually fixes anything. We'll know soon.

Expand Messages | Collapse Messages