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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1386043002: Open distiller UI setting through JavaScript (Closed)

Created:
5 years, 2 months ago by mdjones
Modified:
5 years, 2 months ago
Reviewers:
nyquist, wychen
CC:
chromium-reviews, mlamouri+watch-content_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, jam, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin-cc_chromium.org, mkwst+moarreviews-renderer_chromium.org, darin (slow to review), ben+mojo_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Open distiller UI setting through JavaScript This change adds support for opening the android view distiller settings from JavaScript via "distiller.openSettings". To avoid passing numerous objects into the DomDistillerViewerSource, the ExternalFeedbackReporter has been refactored into a generic UI handle that is responsible for any interaction between Chrome and Android specific controls and the distiller component. The actual UI in the page is not yet implemented. BUG= Committed: https://crrev.com/dcd10462fb49c72544719c490238f3a35edf3fc6 Cr-Commit-Position: refs/heads/master@{#354123}

Patch Set 1 #

Patch Set 2 : remove extra action log #

Total comments: 4

Patch Set 3 : Add missing file #

Total comments: 4

Patch Set 4 : Comment nits #

Unified diffs Side-by-side diffs Delta from patch set Stats (+203 lines, -225 lines) Patch
D chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerFeedbackReporter.java View 1 chunk +0 lines, -57 lines 0 comments Download
A chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java View 1 2 3 1 chunk +98 lines, -0 lines 0 comments Download
M chrome/browser/android/chrome_jni_registrar.cc View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
A + chrome/browser/android/dom_distiller/distiller_ui_handle_android.h View 1 chunk +11 lines, -9 lines 0 comments Download
A + chrome/browser/android/dom_distiller/distiller_ui_handle_android.cc View 2 chunks +15 lines, -14 lines 0 comments Download
D chrome/browser/android/dom_distiller/external_feedback_reporter_android.h View 1 chunk +0 lines, -36 lines 0 comments Download
D chrome/browser/android/dom_distiller/external_feedback_reporter_android.cc View 1 chunk +0 lines, -49 lines 0 comments Download
M chrome/browser/dom_distiller/profile_utils.cc View 3 chunks +7 lines, -6 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 2 chunks +3 lines, -3 lines 0 comments Download
M components/dom_distiller.gypi View 1 chunk +1 line, -1 line 0 comments Download
M components/dom_distiller/content/BUILD.gn View 1 chunk +1 line, -1 line 0 comments Download
M components/dom_distiller/content/browser/distiller_javascript_service_impl.h View 3 chunks +7 lines, -4 lines 0 comments Download
M components/dom_distiller/content/browser/distiller_javascript_service_impl.cc View 1 4 chunks +16 lines, -7 lines 0 comments Download
A + components/dom_distiller/content/browser/distiller_ui_handle.h View 1 2 2 chunks +10 lines, -7 lines 0 comments Download
M components/dom_distiller/content/browser/dom_distiller_viewer_source.h View 3 chunks +5 lines, -4 lines 0 comments Download
M components/dom_distiller/content/browser/dom_distiller_viewer_source.cc View 3 chunks +4 lines, -4 lines 0 comments Download
M components/dom_distiller/content/common/distiller_javascript_service.mojom View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M components/dom_distiller/content/renderer/distiller_native_javascript.h View 1 chunk +0 lines, -6 lines 0 comments Download
M components/dom_distiller/content/renderer/distiller_native_javascript.cc View 2 chunks +19 lines, -15 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
mdjones
PTAL
5 years, 2 months ago (2015-10-05 23:57:13 UTC) #2
nyquist
lgtm https://codereview.chromium.org/1386043002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java File chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java (right): https://codereview.chromium.org/1386043002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java#newcode86 chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java:86: ContentViewCore contentView = ContentViewCore.fromWebContents(webContents); Does fromWebContents support taking ...
5 years, 2 months ago (2015-10-14 17:48:22 UTC) #3
wychen
lgtm https://codereview.chromium.org/1386043002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java File chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java (right): https://codereview.chromium.org/1386043002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java#newcode1 chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java:1: // Copyright 2014 The Chromium Authors. All rights ...
5 years, 2 months ago (2015-10-14 17:51:30 UTC) #4
mdjones
https://codereview.chromium.org/1386043002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java File chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java (right): https://codereview.chromium.org/1386043002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java#newcode1 chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/DomDistillerUIUtils.java:1: // Copyright 2014 The Chromium Authors. All rights reserved. ...
5 years, 2 months ago (2015-10-14 20:55:42 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1386043002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1386043002/60001
5 years, 2 months ago (2015-10-14 20:56:09 UTC) #8
commit-bot: I haz the power
Committed patchset #4 (id:60001)
5 years, 2 months ago (2015-10-14 21:51:35 UTC) #9
commit-bot: I haz the power
5 years, 2 months ago (2015-10-14 21:52:09 UTC) #10
Message was sent while issue was closed. 
Patchset 4 (id:??) landed as
https://crrev.com/dcd10462fb49c72544719c490238f3a35edf3fc6
Cr-Commit-Position: refs/heads/master@{#354123}

Powered by Google App Engine
This is Rietveld 408576698