Issue 335293002: Dismiss WebsiteSettings when a new page is committed.

Issue 335293002: Dismiss WebsiteSettings when a new page is committed. (Closed)

Created:
6 years, 6 months ago by Yaron

Modified:
6 years, 6 months ago

Reviewers:
Ted C

CC:
chromium-reviews, darin-cc_chromium.org, jam

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

Project:
chromium

Visibility:
Public.

More Reviews

Description

Dismiss WebsiteSettings when a new page is committed. Since we don't keep track of the current entry when showing the dialog, it should be dismissed on a navigation to avoid reading the wrong data. BUG=384176 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278229

Patch Set 1 #

Total comments: 1

Patch Set 2 : bounce to ui #

Patch Set 3 : #

Patch Set 4 : also fix chrome-layer tests #

Created: 6 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+68 lines, -17 lines)			Patch
M	chrome/android/java/src/org/chromium/chrome/browser/WebsiteSettingsPopup.java	View		2 chunks	+11 lines, -0 lines	0 comments	Download
M	chrome/test/android/javatests/src/org/chromium/chrome/test/util/TabUtils.java	View	1 2 3	4 chunks	+19 lines, -3 lines	0 comments	Download
M	content/browser/android/web_contents_observer_android.cc	View		2 chunks	+6 lines, -5 lines	0 comments	Download
M	content/public/android/java/src/org/chromium/content/browser/WebContentsObserverAndroid.java	View	1	3 chunks	+10 lines, -2 lines	0 comments	Download
M	content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestCallbackHelperContainer.java	View	1 2	2 chunks	+12 lines, -3 lines	0 comments	Download
M	content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestWebContentsObserver.java	View	1 2 3	2 chunks	+10 lines, -4 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Yaron

https://codereview.chromium.org/335293002/diff/1/content/public/android/java/src/org/chromium/content/browser/WebContentsObserverAndroid.java File content/public/android/java/src/org/chromium/content/browser/WebContentsObserverAndroid.java (right): https://codereview.chromium.org/335293002/diff/1/content/public/android/java/src/org/chromium/content/browser/WebContentsObserverAndroid.java#newcode21 content/public/android/java/src/org/chromium/content/browser/WebContentsObserverAndroid.java:21: this(contentViewCore.getWebContents()); Damn. The test failures are real. Turns out ...

6 years, 6 months ago (2014-06-17 05:00:29 UTC) #2

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yfriedman@chromium.org/335293002/60001

6 years, 6 months ago (2014-06-18 17:40:18 UTC) #5

Message was sent while issue was closed.

Change committed as 278229

Expand Messages | Collapse Messages