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

Issue 12334072: Instant: Don't commit unless the page supports Instant. (Closed)

Created:
7 years, 10 months ago by sreeram
Modified:
7 years, 10 months ago
Reviewers:
samarth, Shishir
CC:
chromium-reviews, melevin, sreeram, gideonwald, dominich, David Black, samarth+watch_chromium.org, Jered
Visibility:
Public.

Description

Instant: Don't commit unless the page supports Instant. BUG=177948 R=samarth@chromium.org TBR=shishir@chromium.org TEST=Follow these steps: 1. Install the Google search app. 2. Enable Instant from chrome://settings. 3. Make sure you are signed in so that http://goto.google.com/instant-redirect works. 4. Start chrome with --instant-url="http://goto.google.com/instant-redirect". Before: Infinite loop of loading and committing google.com After: No infinite loop. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=184467

Patch Set 1 #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -0 lines) Patch
M chrome/browser/instant/instant_overlay.cc View 1 chunk +16 lines, -0 lines 8 comments Download

Messages

Total messages: 7 (0 generated)
sreeram
Samarth, please review. Shishir knows this stuff, but he isn't in yet. Since this is ...
7 years, 10 months ago (2013-02-25 16:40:22 UTC) #1
sreeram
+shishir
7 years, 10 months ago (2013-02-25 17:05:25 UTC) #2
samarth
A couple of questions below and a test, please? But this lgtm to fix the ...
7 years, 10 months ago (2013-02-25 17:15:05 UTC) #3
Shishir
lgtm https://codereview.chromium.org/12334072/diff/1/chrome/browser/instant/instant_overlay.cc File chrome/browser/instant/instant_overlay.cc (right): https://codereview.chromium.org/12334072/diff/1/chrome/browser/instant/instant_overlay.cc#newcode147 chrome/browser/instant/instant_overlay.cc:147: load_params.is_cross_site_redirect = params.is_cross_site_redirect; Shouldn't this always be true ...
7 years, 10 months ago (2013-02-25 17:20:03 UTC) #4
sreeram
On 2013/02/25 17:15:05, samarth wrote: > a test, please? Sure. I promise to add one ...
7 years, 10 months ago (2013-02-25 17:48:01 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sreeram@chromium.org/12334072/1
7 years, 10 months ago (2013-02-25 18:43:53 UTC) #6
commit-bot: I haz the power
7 years, 10 months ago (2013-02-25 20:09:57 UTC) #7
Message was sent while issue was closed.
Change committed as 184467

Powered by Google App Engine
This is Rietveld 408576698