Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2825513002:
Only show the genius app in Google-branded builds (Closed)
|
Created:
3 years, 8 months ago by nicholasbishop Modified:
3 years, 8 months ago CC:
chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionOnly show the genius app in Google-branded builds
The three chrome help URL constants incorrectly used OFFICIAL_BUILD
for a feature that should only be in Google-branded builds.
BUG=none
TEST=Build browser with is_official_build=true and target_os=chromeos. Click the help button in the system menu, verify it shows a support.google.com URL instead of the genius app.
Review-Url: https://codereview.chromium.org/2825513002
Cr-Commit-Position: refs/heads/master@{#465715}
Committed: https://chromium.googlesource.com/chromium/src/+/88d101dc26621ff440fc6a41f88ef932d48bb2b3
Patch Set 1 #
Messages
Total messages: 20 (6 generated)
nbishop@neverware.com changed reviewers: + thestig@chromium.org
thestig@chromium.org changed reviewers: + derat@chromium.org
derat: Do you know about this extension or know someone that does? BTW, there is another instance of this in chrome/browser/ui/webui/chromeos/keyboard_overlay_ui.cc .
derat@chromium.org changed reviewers: + cnwan@chromium.org, cylee@chromium.org, morsed@chromium.org
i don't know anything about it, but cc-ing a few people who show up as having made recent changes to some code that showed up when i did an internal code search.
On 2017/04/17 18:55:48, Daniel Erat wrote: > i don't know anything about it, but cc-ing a few people who show up as having > made recent changes to some code that showed up when i did an internal code > search. Git blame says you touched it last, but that was 5 years ago...
On 2017/04/17 18:57:15, Lei Zhang wrote: > On 2017/04/17 18:55:48, Daniel Erat wrote: > > i don't know anything about it, but cc-ing a few people who show up as having > > made recent changes to some code that showed up when i did an internal code > > search. > > Git blame says you touched it last, but that was 5 years ago... maybe i touched this line, but i suspect it was just in the course of cleaning up a bunch of URL constants that were scattered all over the tree. :-P
On 2017/04/17 at 19:02:25, derat wrote: > On 2017/04/17 18:57:15, Lei Zhang wrote: > > On 2017/04/17 18:55:48, Daniel Erat wrote: > > > i don't know anything about it, but cc-ing a few people who show up as having > > > made recent changes to some code that showed up when i did an internal code > > > search. > > > > Git blame says you touched it last, but that was 5 years ago... > > maybe i touched this line, but i suspect it was just in the course of cleaning up a bunch of URL constants that were scattered all over the tree. :-P Some additional context: https://codereview.chromium.org/2822793002/
On 2017/04/18 15:33:02, nicholasbishop wrote: > Some additional context: https://codereview.chromium.org/2822793002/ So I see chrome/browser/chromeos/genius_app/app_id.h says the Genius app ID is ljoammodoonkhnehlncldjelhidljdpi, but the ones here are for honijodknafkokifofgiaalefdiedpko. I'm guessing that's a help app, but I have no idea what its relationship with the Genius app is. Waiting for one of the other reviewers to chime in.
The HelpApp (App ID: `honijodknafkokifofgiaalefdiedpko`) is a companion, or child, app to the Get Help/genius app. It's a legacy app that was bundled under the Get Help app some time ago. The HelpApp merely holds text content for some panels in the OOBE and the Files app. It does nothing on its own, other than copy text that is managed in gKMS/Redwood to the ChromeOS repo for offline use. I am not familiar with the issues surrounding Google-branded builds and why it is desirable to restrict the appearance of the Get Help app (App ID: `ljoammodoonkhnehlncldjelhidljdpi`).
On 2017/04/18 19:23:05, morsed wrote: > I am not familiar with the issues surrounding Google-branded builds and why it > is desirable to restrict the appearance of the Get Help app (App ID: > `ljoammodoonkhnehlncldjelhidljdpi`). Do you know if the app is available in ChromeOS only? i.e. not available in ChromiumOS. If that's the case, then this CL is the correct thing to do, because OFFICIAL_BUILD means "an optimized build intended for release" whereas GOOGLE_CHROME_BUILD means just that.
On 2017/04/18 at 19:28:05, thestig wrote: > On 2017/04/18 19:23:05, morsed wrote: > > I am not familiar with the issues surrounding Google-branded builds and why it > > is desirable to restrict the appearance of the Get Help app (App ID: > > `ljoammodoonkhnehlncldjelhidljdpi`). > > Do you know if the app is available in ChromeOS only? i.e. not available in ChromiumOS. If that's the case, then this CL is the correct thing to do, because OFFICIAL_BUILD means "an optimized build intended for release" whereas GOOGLE_CHROME_BUILD means just that. No. Both Help App and Get Help are only available in Google-Brand Chrome OS (i.e. You should not see them in Chromium OS build.) honijodknafkokifofgiaalefdiedpko - HelpApp is a platform extension which serves as a place to hold Help Center articles that are hyperlinked along the OOBE flow. (We planned to replace it with "Get Help" but the OOBE flow does not support Packaged App properly, so HelpApp stays as it is) ljoammodoonkhnehlncldjelhidljdpi - Get Help (a.k.a. geniusapp) is a platform packaged app with a green help icon with which users could search for frequently-asked question/answers and as an entry-point to contact customer support.
Thanks for the info. LGTM then.
The CQ bit was checked by nbishop@neverware.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/04/19 at 18:55:32, thestig wrote: > Thanks for the info. LGTM then. Thanks all for the careful review!
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1492628642384710, "parent_rev":
"734e5ee3426a4a4f1f5f0e1fb0a893c97202380d", "commit_rev":
"88d101dc26621ff440fc6a41f88ef932d48bb2b3"}
Message was sent while issue was closed.
Description was changed from ========== Only show the genius app in Google-branded builds The three chrome help URL constants incorrectly used OFFICIAL_BUILD for a feature that should only be in Google-branded builds. BUG=none TEST=Build browser with is_official_build=true and target_os=chromeos. Click the help button in the system menu, verify it shows a support.google.com URL instead of the genius app. ========== to ========== Only show the genius app in Google-branded builds The three chrome help URL constants incorrectly used OFFICIAL_BUILD for a feature that should only be in Google-branded builds. BUG=none TEST=Build browser with is_official_build=true and target_os=chromeos. Click the help button in the system menu, verify it shows a support.google.com URL instead of the genius app. Review-Url: https://codereview.chromium.org/2825513002 Cr-Commit-Position: refs/heads/master@{#465715} Committed: https://chromium.googlesource.com/chromium/src/+/88d101dc26621ff440fc6a41f88e... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/88d101dc26621ff440fc6a41f88e... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
