New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 735153 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression
Team-Security-UX

Blocking:
issue 678332



Sign in to add a comment

Regression: https-string is hidden in Omnibox when visiting a page

Project Member Reported by meh...@chromium.org, Jun 20 2017

Issue description

Chrome Version: Canary 61.0.3136.0
OS: OS X 10.12.5 / MacBook Air 11 Mid 2012 NonRetina

What steps will reproduce the problem?
(1) start Canary and open a new window
(2) focus the Omnibox and go to https://www.google.com/
(3)

What is the expected result? What happens instead?
The https-string is hidden. (Try step 2 again, if it is not reproducible).
After focussing the Omnibox, the https-string appears again.

I also noticed when it happens is that I have to press CMD-L twice to focus the whole text in the Omnibox.

I discussed this with krb@ in issue 729415. But he can not reproduce it.

May be one of you in CC can reproduce it in latest Canary?

Thanks in advance!
Mehmet

A screencast is attached.
 
Ohne Titel.mov
1.9 MB Download

Comment 1 by sdy@chromium.org, Jun 20 2017

Could this be caused by an experiment?
Labels: Needs-Feedback
Cannot reproduce this.

Comment 3 by shrike@chromium.org, Jun 21 2017

Cc: emilyschechter@chromium.org

Comment 4 by meh...@chromium.org, Jun 21 2017

> Could this be caused by an experiment?

Yes, may be?! I do not have any flag enabled in chrome://flags intentionally.

I'm still seeing it in todays Canary.

Is there any way to find out, if my Canary Build is part of an experiment? Can I start Chrome from terminal with all experiments deactivated?

Thanks for your help in advance.
Cc: tommycli@chromium.org
+tommy, is this caused by an experiment? i can't repro
The experiment hasn't been committed yet, so it can't be caused by that.

Comment 7 by sdy@chromium.org, Jun 21 2017

chrome://version should show a list of variation hashes that can be decoded in UMA.

Comment 8 by meh...@chromium.org, Jun 21 2017

> chrome://version should show a list of variation hashes that can be decoded in UMA.

Can I attach it here and you could have a look at it please? Unfortunately I have no access to UMA.

Comment 9 by shrike@chromium.org, Jun 21 2017

Yes, add that list and we can take a look at it.
> Yes, add that list and we can take a look at it.

Thanks! Here it is:

Google Chrome	61.0.3137.0 (Offizieller Build) canary (64-Bit)
Überarbeitung	59326d74274f028a233a2561f940a84675e0c967-refs/heads/master@{#481056}
Betriebssystem	Mac OS X
JavaScript	V8 6.1.209
Flash	24.0.0.189 internal-not-yet-present
User-Agent	Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3137.0 Safari/537.36
Befehlszeile	/Applications/Google Chrome Canary.app/Contents/MacOS/Google Chrome Canary --flag-switches-begin --flag-switches-end
Ausführbarer Pfad	/Applications/Google Chrome Canary.app/Contents/MacOS/Google Chrome Canary
Profilpfad	/Users/memo/Library/Application Support/Google/Chrome Canary/Default

Varianten	

16e0dd70-3f4a17df
da89714-1e61946b
6c18ba9d-d68b7e6
c68ab9a3-1f8c5973
b130ecb8-2e32ee7e
6025934e-3f4a17df
7c1bc906-ff96eb58
47e5d3db-3d47f4f4
776de70c-e0278d3d
79616653-ca7d8d80
a605b19e-f23d1dea
5ca89f9-f23d1dea
ed7ba060-3f4a17df
9e201a2b-d151e2f5
5274eb09-3f4a17df
57f575bb-f23d1dea
68812885-f23d1dea
332a4d9b-ca7d8d80
2ce29c0a-3d47f4f4
eab812f7-f23d1dea
b791c1b8-f23d1dea
9773d3bd-2f79df73
9e5c75f1-272b9dce
e79c5ffa-3f4a17df
d6db5f84-3f4a17df
f79cb77b-3d47f4f4
27219e67-b2047178
23a898eb-a6a00d44
e856c60b-f267a6f9
4ea303a6-3d47f4f4
56302f8c-2f882e70
de03e059-e65e20f2
ad6d27cc-3e870323
2697ea25-3fc362e4
f56e0452-f23d1dea
b2f0086-b099a502
ef25c1eb-3f4a17df
1354da85-8aacded3
494d8760-3d47f4f4
3ac60855-486e2a9c
f296190c-96d26288
4442aae2-a90023b1
ed1d377-e1cc0f14
75f0f0a0-d7f6b13c
e2b18481-6e3b1976
e7e71889-e1cc0f14
f5fff3a2-f23d1dea
644b8345-726d8ace
828a5926-8f2c913
da4aaa01-3f4a17df

Owner: nick@chromium.org
Nothing suspicious in the experiments.

Looking at the changelist for 61.0.3135.0..61.0.3136.0, wondering about:

commit	bcce9cb42a1578ba95ead8b64e66101a8ae5e40b	
author	nick <nick@chromium.org>	Mon Jun 19 22:17:09 2017

Always update the omnibox URL when cancelling via onbeforeunload

Review-Url: https://codereview.chromium.org/2919593007

Comment 12 by sdy@chromium.org, Jun 21 2017

Labels: ReleaseBlock-Stable
Owner: jkarlin@chromium.org
Status: Assigned (was: Untriaged)
--enable-features=NetworkSchedulerYielding

Comment 13 by sdy@chromium.org, Jun 21 2017

Labels: -Needs-Feedback -Needs-Bisect

Comment 14 by sdy@chromium.org, Jun 21 2017

Cc: jkarlin@chromium.org
Owner: nick@chromium.org
Sorry, that was a hasty change. It doesn't look like that experiment was touched recently. I'll try a bisect to confirm the change shrike@ mentioned.

Comment 15 by nick@chromium.org, Jun 21 2017

Thanks for the report. Based on the symptoms this was probably my change.

r480598 was reverted as r480669. 61.0.3136.0 was cut at r480665, so the revert just barely missed it, and subsequent canaries ought to be fine.

I'll be sure to address this before re-landing the CL.
Great. Thanks to you all for the help. 
mehmet@ - according to c#15 only Canary 61.0.3136.0 should show the problem. Do you see it in 61.0.3137.0?
shrike@: Oh yes, sorry. You are right. I am still seeing the issue in 61.0.3137.0 (my comment 10). I have misread comment 15 and have thought, it will be fixed in the next Canary. :-(
> shrike@: Oh yes, sorry. You are right. I am still seeing the issue in 61.0.3137.0 (my comment 10). I have misread comment 15 and have thought, it will be fixed in the next Canary. :-(

Update: I am on 61.0.3138.0 now and I am still seeing it on my MacBookAir. I also checked it on my iMac (21,5 End 2012 NonRetina) and I am also seeing this issue there. I would like to help to find the regression range, but when I install a Snapshot Build, I can't reproduce :-(




OMG. I think I have found the culprit after hours of investigation :/

It is caused by the Settings flag "Use a prediction service to load pages more quickly" under Privacy and security.

Here is a screencast.
Ohne Titel.mov
24.3 MB Download
Components: Internals>Preload
+Internals>Preload per comment #20
Owner: alexilin@chromium.org
Thanks mehmet@ for the hard work!

Suspecting

commit	f78c0a40a1a08e2c3820d15b46a38bbccdeadc6f	
author	Alexandr Ilin <alexilin@chromium.org>	Fri Jun 16 16:35:35 2017
Fix incorrect arguments order in Network Predictor.

Const bool and const int arguments were passed to PreconnectUrl() in the
incorrect order. true->1 and 1->true convertions are fixed in C++ Standard so
the bug is hidden now.

Reviewed-on: https://chromium-review.googlesource.com/538637
Reviewed-by: Matt Menke <mmenke@chromium.org>

shrike@, did a bisect point on this commit?
I don't see any possibility how my CL could cause this bug.
alexilin@ - if you're asking if I did a bisect the answer is no because at the time I wasn't able to reproduce. And looking deeper into your change it doesn't seem like it could be the cause.

I was able to get my Canary profile into a state where I can reproduce the problem. Bisecting suggests a change between 61.0.3115.0 and 61.0.3117.0 (note that there doesn't appear to be a 61.0.3116.0). Suspecting this change:


commit	eb0f08a8b0768450acac29a5230cc2202fc89077	
author	Alexandr Ilin <alexilin@chromium.org>	Tue May 30 10:55:34 2017

predictors: Refactor Glowplug Database.

All Glowplug tables have the same structure. A table has two columns
representing string as a key and protobuf message as a value.

The goal of this refactor is to ease adding new tables in Glowplug and reduce
amount of the code. This CL also makes easier a replacement of database backend.

Bug:  715525 
Reviewed-on: https://chromium-review.googlesource.com/506087

The change in c#24 touches only the experimental code. mehmet@ doesn't have a corresponding variation set to enable this code. So it's unlikely to be the cause.

shrike@ can you share the steps how to get the repro? Can you see the bug on other platforms?
I'll take another look at it on Monday.
It was tricky narrowing down a set of repro steps. These work fairly consistently for me:

1. Launch Canary with a new user-data-dir
2. Go to Settings and set to Restart where you left off
3. Close the Settings tab
4. In the remaining tab type google.de and press Return - if the Translate bubble appears, choose Never translate German from the options dropdown so that you never see this bubble again (seems to be important)
5. Cmd-T to create a new tab - type youtube.de and press Return
6. Repeat step 5. four more times
7. Click the google.de tab and then Quit
8. Restart Chrome and as soon as the window appears, type Cmd-L to select the google.de omnibox text and type "you" (the first three letters of youtube.de - the omnibox should autocomplete it to youtube.de), then press Return

If you get the full domain (with scheme), Cmd-[ to go back and quit and then do step 8. again.

When you follow step 8. you are basically typing in the youtube.de URL while that site is loading in at least one other tab. It has seemed like it's been important for youtube.de to be loading in a tab while reproing the bug, but it's not always consistent. With 5 tabs you will probably have two of them loading at once, which may be required. This bug does not seem to manifest itself with google.com and youtube.com. I also couldn't repro with Chromium but I also didn't try that hard.

I have a Windows machine but I can't get Canary to load so I'm unable to test there.

I have also attached a link to a user-data-dir that should be pre-configured to reproduce the problem. Using it, you should be able to start with step 8.

https://drive.google.com/open?id=0BwcLL5PHtZQHaTlFcEk3N0pRWHM

Project Member

Comment 27 by sheriffbot@chromium.org, Jun 26 2017

This issue is marked as a release blocker with no milestone associated. Please add an appropriate milestone.

All release blocking issues should have milestones associated to it, so that the issue can tracked and the fixes can be pushed promptly.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: M-61
Cc: pasko@chromium.org
+pasko who suggested that it could be prerenderer issue.
shrike@, mehmet@, could I ask you to check if the issue is caused by the prerenderer? To check this you need to go in chrome://flags and set "No-State Prefetch" flag to "Enabled Simple load" and then check if you still can reproduce the issue.

I would check it by myself but I still don't have a proper mac device. Thanks!

If it turns out to be prerenderer then we don't need to fix this bug because the prerenderer will be disabled starting with M61.

Hi alexilin@: Yes, switching the "No-State Prefetch" flag to "Enabled Simple load" fixes the problem for me :-)

Tested with Chrome Version 61.0.3141.0 on macOS 10.12.5

Blocking: 678332
Awesome, thank you! 

Another small request: could you check that "Enabled No-state prefetch" value for the "No-State Prefetch" flag doesn't cause the issue as well? Most likely it will be the default value for M61.

Yep, it is also fixed with the "Enabled No-state prefetch" value for the "No-State Prefetch" flag.

Could you please add me in CC to bug 678332, so that I can receive latest updates? It seems that bug 678332 has a Google-only permission.

Thank you very much!
Components: -UI>Browser>Omnibox UI>Browser>Omnibox>SecurityIndicators
Triaging more specifically (the security indicator component covers the colored protocol).
Cc: lgar...@chromium.org
 Issue 736566  has been merged into this issue.
A friendly reminder that M61 branch is coming soon on 07/20! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix ASAP to trunk. This way we branch M61 from a high quality trunk. Thank you.
@alexilin: Is there any latest update's available on this issue, since this issue is tagged with RB-Stable?

Thanks!!

Comment 38 by pasko@chromium.org, Jul 25 2017

We are disabling prerender in M61 by default: 
https://chromium-review.googlesource.com/c/584871

The Finch config (NoStatePrefetchValidation_*.json) may be used to roll out nostate-prefetch, which we did not decide to do, so we are not changing the config yet.
URGENT - PTAL.
Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the M61 branch #3163 ASAP to have enough baking time in Beta before Stable promotion. Thank you!

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label.

Project Member

Comment 40 by bugdroid1@chromium.org, Aug 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8c189a52acf6acc7a5dd9ee190beb0b35de2a8b

commit c8c189a52acf6acc7a5dd9ee190beb0b35de2a8b
Author: Egor Pasko <pasko@chromium.org>
Date: Thu Aug 03 10:11:01 2017

Disable prerender by default

While we are still compiling the report from the field trial, disable prerender
to be on the safer side in M61.

Bug: 678332,  735153 , 520275
Change-Id: I30a2169533d30cbb85f53291223897caceddeb89
Reviewed-on: https://chromium-review.googlesource.com/584871
Commit-Queue: Egor Pasko <pasko@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491691}
[modify] https://crrev.com/c8c189a52acf6acc7a5dd9ee190beb0b35de2a8b/chrome/browser/extensions/activity_log/activity_log_unittest.cc
[modify] https://crrev.com/c8c189a52acf6acc7a5dd9ee190beb0b35de2a8b/chrome/browser/predictors/autocomplete_action_predictor_unittest.cc
[modify] https://crrev.com/c8c189a52acf6acc7a5dd9ee190beb0b35de2a8b/chrome/browser/prerender/prerender_browsertest.cc
[modify] https://crrev.com/c8c189a52acf6acc7a5dd9ee190beb0b35de2a8b/chrome/browser/prerender/prerender_field_trial.cc
[modify] https://crrev.com/c8c189a52acf6acc7a5dd9ee190beb0b35de2a8b/chrome/browser/prerender/prerender_manager.cc
[modify] https://crrev.com/c8c189a52acf6acc7a5dd9ee190beb0b35de2a8b/chrome/browser/prerender/prerender_unittest.cc
[modify] https://crrev.com/c8c189a52acf6acc7a5dd9ee190beb0b35de2a8b/chrome/browser/ui/search/instant_search_prerenderer_unittest.cc

Project Member

Comment 41 by bugdroid1@chromium.org, Aug 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73388f4870a69403e5048a996538796bb9c3aaa9

commit 73388f4870a69403e5048a996538796bb9c3aaa9
Author: John Mellor <johnme@chromium.org>
Date: Fri Aug 04 16:51:00 2017

Revert "Disable prerender by default"

This reverts commit c8c189a52acf6acc7a5dd9ee190beb0b35de2a8b.

Reason for revert: broke many prerender tests

All(?) prerender tests broke on official builds:
https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin%20tester/builds/8357

and 3 Android tests broke:
- chrome_public_test_apk ExternalPrerenderHandlerTest#testAddSeveralPrerenders
- unit_tests PrerenderAdapterTest.StartPrerenderSucceeds
- unit_tests PrerenderAdapterTest.StartPrerenderFailsForUnsupportedScheme
on bots like https://build.chromium.org/p/chromium.swarm/builders/Android%20N5X%20Swarm/builds/1957

Original change's description:
> Disable prerender by default
> 
> While we are still compiling the report from the field trial, disable prerender
> to be on the safer side in M61.
> 
> Bug: 678332,  735153 , 520275
> Change-Id: I30a2169533d30cbb85f53291223897caceddeb89
> Reviewed-on: https://chromium-review.googlesource.com/584871
> Commit-Queue: Egor Pasko <pasko@chromium.org>
> Reviewed-by: David Roger <droger@chromium.org>
> Reviewed-by: Reilly Grant <reillyg@chromium.org>
> Reviewed-by: Marc Treib <treib@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#491691}

TBR=pasko@chromium.org,droger@chromium.org,reillyg@chromium.org,treib@chromium.org
NOTRY=true

Bug: 678332,  735153 , 520275
Change-Id: Ie4b4bd7c93175176c625696762e57356ff9c2413
Reviewed-on: https://chromium-review.googlesource.com/602347
Commit-Queue: John Mellor <johnme@chromium.org>
Reviewed-by: John Mellor <johnme@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492051}
[modify] https://crrev.com/73388f4870a69403e5048a996538796bb9c3aaa9/chrome/browser/extensions/activity_log/activity_log_unittest.cc
[modify] https://crrev.com/73388f4870a69403e5048a996538796bb9c3aaa9/chrome/browser/predictors/autocomplete_action_predictor_unittest.cc
[modify] https://crrev.com/73388f4870a69403e5048a996538796bb9c3aaa9/chrome/browser/prerender/prerender_browsertest.cc
[modify] https://crrev.com/73388f4870a69403e5048a996538796bb9c3aaa9/chrome/browser/prerender/prerender_field_trial.cc
[modify] https://crrev.com/73388f4870a69403e5048a996538796bb9c3aaa9/chrome/browser/prerender/prerender_manager.cc
[modify] https://crrev.com/73388f4870a69403e5048a996538796bb9c3aaa9/chrome/browser/prerender/prerender_unittest.cc
[modify] https://crrev.com/73388f4870a69403e5048a996538796bb9c3aaa9/chrome/browser/ui/search/instant_search_prerenderer_unittest.cc

[Bulk Edit]
URGENT - PTAL.
Your bug is labelled as M61 Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP.

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label.

Thank you.

Project Member

Comment 43 by bugdroid1@chromium.org, Aug 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ef465ffd2c1501d51dd66a3c24d57527de9c39d

commit 2ef465ffd2c1501d51dd66a3c24d57527de9c39d
Author: Egor Pasko <pasko@chromium.org>
Date: Thu Aug 10 19:05:05 2017

Re-land: Disable prerender by default

Disable prerender to be on the safer side when the study expires.

On top of the original patch (http://crrev.com/491691) these changes are made:
* remove overrides in fieldtrial_testing_config.json to have the same
  prerender mode in tests as on the official bots
* SetUpOnMainThread() in prerender_browsertest.cc
  * Avoid crashing when PRERENDER_DISABLED
  * replace ASSERT_* with CHECK* in SetUpOnMainThread to get better
    diagnostics from problems with prerender configuration
* Enable prerendering for all of tests in prerender_unittest rather than
  selectively
* For offline_pages/ tests enable prerendering to the correct mode without
  requiring an instance of PrerenderManager - in the current state the
  PrerenderManager is always found, but it was not the case for
  PRERENDER_MODE_DISABLED,
* The CustomTabs tests force prerendering, but they still require an instance of
  PrerenderManager, which is not available with PRERENDER_MODE_DISABLED, added a
  comment about it.

Bug: 678332,  735153 , 520275
Change-Id: I4761e0b99ebca615402ac1e71332c79bd3e4899f
Reviewed-on: https://chromium-review.googlesource.com/603614
Commit-Queue: Egor Pasko <pasko@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493484}
[modify] https://crrev.com/2ef465ffd2c1501d51dd66a3c24d57527de9c39d/chrome/browser/extensions/activity_log/activity_log_unittest.cc
[modify] https://crrev.com/2ef465ffd2c1501d51dd66a3c24d57527de9c39d/chrome/browser/offline_pages/prerender_adapter_unittest.cc
[modify] https://crrev.com/2ef465ffd2c1501d51dd66a3c24d57527de9c39d/chrome/browser/predictors/autocomplete_action_predictor_unittest.cc
[modify] https://crrev.com/2ef465ffd2c1501d51dd66a3c24d57527de9c39d/chrome/browser/prerender/prerender_browsertest.cc
[modify] https://crrev.com/2ef465ffd2c1501d51dd66a3c24d57527de9c39d/chrome/browser/prerender/prerender_field_trial.cc
[modify] https://crrev.com/2ef465ffd2c1501d51dd66a3c24d57527de9c39d/chrome/browser/prerender/prerender_manager.cc
[modify] https://crrev.com/2ef465ffd2c1501d51dd66a3c24d57527de9c39d/chrome/browser/prerender/prerender_manager.h
[modify] https://crrev.com/2ef465ffd2c1501d51dd66a3c24d57527de9c39d/chrome/browser/prerender/prerender_test_utils.cc
[modify] https://crrev.com/2ef465ffd2c1501d51dd66a3c24d57527de9c39d/chrome/browser/prerender/prerender_unittest.cc
[modify] https://crrev.com/2ef465ffd2c1501d51dd66a3c24d57527de9c39d/chrome/browser/ui/search/instant_search_prerenderer_unittest.cc
[modify] https://crrev.com/2ef465ffd2c1501d51dd66a3c24d57527de9c39d/testing/variations/fieldtrial_testing_config.json

[Bulk Edit]
URGENT - PTAL.
Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch ASAP. Thank you!

Know that this issue shouldn't block the release?  Remove the ReleaseBlock-Stable label or move to M62.

Comment 45 by pasko@chromium.org, Aug 16 2017

Owner: pasko@chromium.org
Status: Fixed (was: Assigned)
Prerender is disabled in trunk and in Finch. Marking as fixed.
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-61; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-61 label, otherwise remove Merge-TBD label. Thanks.

Comment 47 by pasko@chromium.org, Aug 16 2017

Labels: Merge-Request-61
Status: Started (was: Fixed)
OK, disabling in M61 would be even safer. Requesting a merge.
Project Member

Comment 48 by sheriffbot@chromium.org, Aug 16 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: -tommycli@chromium.org
Before we approve merge to M61, please answer followings:
* Is this M61 regression? Is it critical?
* Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M61?
* Any other important details to justify the merge.

Please note We're only few weeks away from M61 Stable promotion, so merge bar is very high. Thank you.

Comment 51 by sdy@chromium.org, Aug 16 2017

Cc: -sdy@chromium.org

Comment 52 by pasko@chromium.org, Aug 16 2017

> * Is this M61 regression? Is it critical?

Seems like appeared in M61 and critical. We do not know the root cause, but we
are somewhat confident that it will go away if we disable prerender.

Importance of the merge is not a clear cut because we used the kill switch via
Finch already. If that is considered enough these days, I'm happy not to merge.

> * Is the change well baked/verified in Canary, having enough automation tests
> coverage and safe to merge to M61?

The change landed 5 days ago (2ef465ff), with no associated issues since. It
first appeared in 62.0.3182.0 which was released at 2017-08-11 (last Friday).

On merging: looks like a straightforward cherry-pick. The change is just 3 lines
in prerender_manager.cc. There will be easy-to-resolve conflicts in test code
though. 

> * Any other important details to justify the merge.

I attempted to inline all relevant details above.

Cc: abdulsyed@chromium.org
Thank you  pasko@. 
If feature is disabled by Finch for M61 and is good enough, then I don't think we need to take this merge in. +abdulsyed@, what do you think?
Per comment 52, seems like it's a straightforward cherry pick and a critical Release Blocking regression. Would disabling the feature via Finch remove this regression? If yes, then let's avoid the merge. 

Comment 55 by pasko@chromium.org, Aug 17 2017

Status: Fixed (was: Started)
Yes, users receiving Finch seeds would have this issue fixed. Avoiding the merge as per recommendation in comment 54.
Cc: rbasuvula@chromium.org
Labels: TE-Verified-M62 TE-Verified-62.0.3188.0
Tested the issue on Mac OS 10.12.6 using chrome latest Canary M62-62.0.3188.0 by following steps mentioned in the Comment#30&32 . Observed that https-string is displaying as expected. Hence adding TE-Verified label.

Please find the screen shot for reference.

Thank you!
735153.mp4
3.8 MB View Download
Labels: -Merge-Review-61 Merge-Rejected-61
Rejecting merge based on comment #55. Thank you.
Labels: -Merge-TBD
Is this disable now by finch for M61?

Comment 59 by pasko@chromium.org, Aug 21 2017

> Is this disable now by finch for M61?

yes

Sign in to add a comment