Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Webview navigation history getter #72

Open
wants to merge 3 commits into
base: nw22
Choose a base branch
from
Open

Webview navigation history getter #72

wants to merge 3 commits into from

Conversation

janRucka
Copy link
Contributor

@janRucka janRucka commented Mar 7, 2017 •
edited

This was referenced Mar 7, 2017
Closed
Closed
rogerwang pushed a commit that referenced this pull request Mar 11, 2017
@LuciferTian2010
[Android] 'Open in new Chrome (incognito) tab under context menu does…
498c3d6 
… not work for images and videos

For links of images and videos, the options 'Open in new Chrome tab'
and 'Open in Chrome incognito tab' under context menu do not work,
because the linkUrl is empty.

To fix this, check whether the link is video or image, if so, pass the
srcUrl instead of linkUrl.

Also for downloaded images and videos, the context menu should be
disabled because current options ('Share' and 'Download image/video')
could be achieved through overflow menu.

To fix this, check if the srcUrl starts with 'file://'.

BUG=696417
NOTRY=true
NOPRESUBMIT=true

Original-Review-Url: https://codereview.chromium.org/2725473002
Cr-Original-Commit-Position: refs/heads/master@{#455181}
(cherry picked from commit 2fc04cd)

Review-Url: https://codereview.chromium.org/2739723004
Cr-Commit-Position: refs/branch-heads/3029@{#72}
Cr-Branched-From: 939b32e-refs/heads/master@{#454471}
rogerwang
rogerwang requested changes Mar 13, 2017
View reviewed changes
extensions/renderer/resources/guest_view/web_view/web_view.js
@@ -148,11 +148,12 @@ WebViewImpl.prototype.onFrameNameChanged = function(name) {
// Updates state upon loadcommit.
WebViewImpl.prototype.onLoadCommit = function(
baseUrlForDataUrl, currentEntryIndex, entryCount,
processId, url, isTopLevel) {
processId, url, isTopLevel, pagesHistory) {
this.baseUrlForDataUrl = baseUrlForDataUrl;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would send the whole history from browser to renderer process on every navigation, which could hurt performance. Is there any better way to do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s true it is send with every navigation. But I consider it to be insignificant. After all it is just few strings. I guess it would be possible to make changes just in javascript code. Even without ‘pagesHistory’ you get ‘url’ and ‘currentEntryIndex’ from which you can create array of everything you need. But then all logic needs to be there. E.g. if you go back and then navigate to other page you should delete some record.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The history will grow longer and longer. So eventually the data will grow big enough to hurt performance. And there is a size limit of IPC message. Should find another way to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t know about limit for IPC message. But it will not grow longer and longer. For navigation history there is some limit. When you have about 30 or so items it will remove the oldest. Or at least it seems to be the case after quick test.

@janRucka
Copy link
Contributor Author

Updated to current nw21.
The session history is really limited see "const int kMaxSessionHistoryEntries = 50;" here:
https://github.com/nwjs/chromium.src/blob/nw20/content/public/common/content_constants.cc

so it will not grow longer and longer. In some cases, even 50 entries could be quite big. But we never encountered any issue related to this.

Regarding the performance, I don't think it is significant. Currently we send everything at once synchronously. This suit us well since we use it to show session history. We could send it one at a time asynchronously but I guess that would be even more performance demanding. Other way could be to restrict it more e.g. to last 10 entries. On the other hand, if use case would be to get just one entry at a time it seems to be Ok.

@rogerwang rogerwang force-pushed the nw21 branch 5 times, most recently from 9396d7f to 8adf1c9 Compare April 4, 2017 13:14
@rogerwang rogerwang force-pushed the nw21 branch 3 times, most recently from f1e4ded to 4fd96a3 Compare April 6, 2017 01:18
GnorTech pushed a commit that referenced this pull request Apr 26, 2017
@claudemasso
Fix release builds for iOS. Add stub chromium impl for preferences.
0b4caf9 
BUG=711612

Review-Url: https://codereview.chromium.org/2818153002
Cr-Commit-Position: refs/heads/master@{#464763}
(cherry picked from commit 3b1c2e7)

Review-Url: https://codereview.chromium.org/2827173002 .
Cr-Commit-Position: refs/branch-heads/3071@{#72}
Cr-Branched-From: a106f0a-refs/heads/master@{#464641}
chrome-release-bot and others added 3 commits May 1, 2017 11:54
Publish DEPS for Chromium 58.0.3029.96
ab55a22
@rogerwang
nwjs master
5e7b9cd
@janRucka
Webview navigation history getter.
a6710b6 
Webview now has:
-getPagesHistory function, returning array of URLs, titles and favicons of pages in history. Titles and favicons are not known for current page because array is created before page is fully loaded.
-getCurrentHistoryIndex function, returning current history index
@janRucka janRucka changed the base branch from nw21 to nw22 May 5, 2017 13:55
@janRucka janRucka mentioned this pull request May 5, 2017
Open
@rogerwang rogerwang force-pushed the nw22 branch 4 times, most recently from 65748ff to 90fd209 Compare May 10, 2017 06:28
GnorTech pushed a commit that referenced this pull request Aug 5, 2017
Update stylus settings for lock screen note taking
7270d2d 
Fixes a number of issues with stylus settings for note taking on lock
screen:
 *  Provide a NoteTakingHelper observer interface called when the
    preferred app changes (or when it's lock screen status changes)
     *  Settings UI can use this to update itself when the preferred
        app changes.
     *  Switch lock_screen_apps::AppManagerImpl to observe this event
        for preferred app changes (instead of observing note taking
        pref directly)
  *  Introduces kNoteTakingAppsAllowedOnLockScreen pref, that will be
    used by a user policy to whitelist apps available on the lock
    screen (to be added in dependent patch)
 *  Disable lock screen support for note taking apps in non-primary
    profiles (the profile that supports lock screen use case is set
    by lock_screen_apps::StateController during its initialization).
 *  Redo settings UI for enabling apps on the lock screen so its
    state (whether it's disabled, the policy indicator) does not
    depend on prefs directly, instead derive the state from the
    note taking app's NoteAppInfo (in particular lockScreenSupport
    property)

While here, did some cleanup in test code:
 *  Provided utility methods to NoteTakingHelper unit tests to
    reduce code duplication:
     *  Method to create/install lock screen enabled note taking app
     *  Methods for verifying preferred app and available apps info
 *  In stylus device page browser tests, made fake browser proxy
    smarter, so test don't have to "manually" trigger note taking app
    changes

TBR=tbarzic@google.com

(cherry picked from commit e4a5c06)

Bug: 741940
Bug: 741053
Bug: 746116
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I5e2ee138df620d3832a8ad0d1b4d0db285fba0da
Reviewed-on: https://chromium-review.googlesource.com/572842
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#489142}
Reviewed-on: https://chromium-review.googlesource.com/588341
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#72}
Cr-Branched-From: ff259ba-refs/heads/master@{#488528}
GnorTech pushed a commit that referenced this pull request Sep 14, 2017
@xunjieli
Revert "[cronet] Re-use Direct ByteBuffer for Cronet upload"
11fcec9 
This reverts commit 9580368.

Reason for revert: Suspect that there is a leak in this CL

Original change's description:
> [cronet] Re-use Direct ByteBuffer for Cronet upload
>
> This CL makes Cronet upload reuse a Java ByteBuffer object if the
> underlying net::IOBuffer's address and buffer length are unchanged.
>
> This should reduce the number of constructor calls to
> NewDirectByteBuffer().
>
> Bug: 756841
> Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
> Change-Id: I751242095e2ba5793750d5a91e2bc3b10ec8b7a1
> Reviewed-on: https://chromium-review.googlesource.com/624196
> Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org>
> Commit-Queue: Helen Li <xunjieli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#496067}

TBR=kapishnikov@chromium.org, xunjieli@chromium.org


(cherry picked from commit f9121d1)

Bug: 756841
Change-Id: I186de0df7b316e4284648b1b9cca1addc7f7845d
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Reviewed-on: https://chromium-review.googlesource.com/656109
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#500411}
Reviewed-on: https://chromium-review.googlesource.com/656002
Cr-Commit-Position: refs/branch-heads/3202@{#72}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
GnorTech pushed a commit that referenced this pull request Oct 27, 2017
Record metrics for abandoned printer setups.
5c56ee9 
To determine what printers users are having difficulty setting up,
record the printer if setup is abandoned from the PPD selection screen.
This will tell us where our PPD coverage is lacking.

TBR=skau@chromium.org

(cherry picked from commit c397a59)

Bug: 756576
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Id1e0913a3096d8a674a14cf230f6b97af456b330
Reviewed-on: https://chromium-review.googlesource.com/602696
Commit-Queue: Sean Kau <skau@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509654}
Reviewed-on: https://chromium-review.googlesource.com/728348
Reviewed-by: Sean Kau <skau@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#72}
Cr-Branched-From: adb61db-refs/heads/master@{#508578}
rogerwang pushed a commit that referenced this pull request Jan 29, 2018
@dgozman
[DevTools] Initialize InspectorSession later
f698044 
Restoring InspectorSession may send protocol notifications
synchronously, so we should setup all bindings beforehand.

TBR=dgozman@chromium.org

(cherry picked from commit c27cabf)

Bug: 804214
Change-Id: I68989990fe210a61251dc20047b891fa4155d596
Reviewed-on: https://chromium-review.googlesource.com/879137
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#531349}
Reviewed-on: https://chromium-review.googlesource.com/884510
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#72}
Cr-Branched-From: bc084a8-refs/heads/master@{#530369}
GnorTech pushed a commit that referenced this pull request Mar 20, 2018
[iOS] Should create IOSUserEventService as soon as possible
b7e5331 
IOSUserEventService is not synchronous. To avoid to have the DCHECK in
ConsentAuditor::RecordGaiaConsent() failing, this service should be
created as soon as possible, to make sure it is correctly initialized
before using it.
With this fix, it is now possible to put back the DCHECK in
ConsentAuditor::RecordGaiaConsent().

Bug: 819176
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I66be7843eb3cc73686c07b0ffdee232c90dad7dd
Reviewed-on: https://chromium-review.googlesource.com/951484
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#541123}(cherry picked from commit 1f2f87f)
Reviewed-on: https://chromium-review.googlesource.com/953463
Reviewed-by: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#72}
Cr-Branched-From: 66afc5e-refs/heads/master@{#540276}
rogerwang pushed a commit that referenced this pull request Apr 25, 2018
Change the ChromeOS Audio Player to no longer use panels
d8acff3 
Change the ChromeOS Audio Player to no longer use panels, but behave as
a regular app.
Panels are now deprecated. See the bug for more details.

Bug: 800990
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ia081ac12da133a31ca89719b501b373152a0441c
Reviewed-on: https://chromium-review.googlesource.com/1011863
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Sasha Morrissey <sashab@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550930}(cherry picked from commit 8b54e20)
Reviewed-on: https://chromium-review.googlesource.com/1016140
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#72}
Cr-Branched-From: 9ef2aa8-refs/heads/master@{#550428}
rogerwang pushed a commit that referenced this pull request Jun 19, 2018
Request and directly upload an EID if no enrollment certificate
7b3fb48 
A device that is enrolled pre-M68 may not be able to obtain an
enrollment certificate until it is wiped, but can request a computation
of its EID immediately and upload that to the management servers.

BUG=chromium:840496
TEST=unit_tests

Change-Id: Ib1c4d2652110c49d1370fcc0dfbcfddb336c2de9
Reviewed-on: https://chromium-review.googlesource.com/1069599
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Darren Krahn <dkrahn@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Yves Arrouye <drcrash@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#561890}(cherry picked from commit b7bfd93)
Reviewed-on: https://chromium-review.googlesource.com/1080971
Reviewed-by: Yves Arrouye <drcrash@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#72}
Cr-Branched-From: 010ddcf-refs/heads/master@{#561733}
GnorTech pushed a commit that referenced this pull request Aug 4, 2018
@FHorschig
[Android, Crash] Prevent Accessory trying to send data too early
b022902 
There is a nullptr exception connected to SendViewItems. This CL guesses
a solution to this problem:
1. It's only called if the controller was checked to be null, so it must
be in the implementation.
2. The view_ is synchronously constructed at construction time and can
outside of tests never be null.
3. It's bound to web_contents_, so this also cannot be null.

The only dereference left is the GetFocusedFrame() method which can be
null. Therefore, accessing the correct frame moves to the driver-side
which is much more appropriate for frame-based actions anyway.

TBR=fhorschig@chromium.org

(cherry picked from commit f178cf1)

Bug: 865447
Change-Id: Ie5ea7aed980cdf53e784e5d918a9d54717ad6ab1
Reviewed-on: https://chromium-review.googlesource.com/1143477
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577496}
Reviewed-on: https://chromium-review.googlesource.com/1150229
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#72}
Cr-Branched-From: 271eaf5-refs/heads/master@{#576753}
rogerwang pushed a commit that referenced this pull request Sep 15, 2018
Incrementing VERSION to 70.0.3538.9
bbaf922 
TBR=govind@chromium.org

Change-Id: I9371fb574654fda477097dd156076be0c651cec6
Reviewed-on: https://chromium-review.googlesource.com/1208415
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#72}
Cr-Branched-From: 79f7c91-refs/heads/master@{#587811}
rogerwang pushed a commit that referenced this pull request Oct 31, 2018
[Explore Sites] Fixing SQL query to gather icons for category tiles.
7965d1b 
Previously, the SQL query included site icons that were NULL or empty,
resulting in category tiles often having less than 4 site icons present.
This fix skips the empty icons, and grabs the next non-empty ones.

Change-Id: I314c78f1797769e598640d06162811e1672c2449
Reviewed-on: https://chromium-review.googlesource.com/c/1279155
Reviewed-by: Peter Williamson <petewil@chromium.org>
Commit-Queue: Jonathan Freed <freedjm@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599347}(cherry picked from commit cdef234)
Reviewed-on: https://chromium-review.googlesource.com/c/1285097
Reviewed-by: Cathy Li <chili@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#72}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
rogerwang pushed a commit that referenced this pull request Dec 18, 2018
[iOS][MF] Fix content instets on rotation
2df866f 
Bug: 845472
Change-Id: I3afc1e03c280c88cce039ac8d4a6b288edd7be78
Reviewed-on: https://chromium-review.googlesource.com/c/1355173
Reviewed-by: Yi Su <mrsuyi@chromium.org>
Commit-Queue: Javier Ernesto Flores Robles <javierrobles@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612608}(cherry picked from commit 39cf762)
Reviewed-on: https://chromium-review.googlesource.com/c/1363176
Reviewed-by: Javier Ernesto Flores Robles <javierrobles@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#72}
Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
rogerwang pushed a commit that referenced this pull request Feb 10, 2019
@mitoshima
Don't mark event handled unless it's handled.
971df88 
in OverviewSession::OnKeyEvent

Bug: 925878
Test: covered by unittest
Change-Id: I712acfcc7127e86e30d5d1be3ef8f86831c12368
Reviewed-on: https://chromium-review.googlesource.com/c/1440913
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#626807}(cherry picked from commit 8d4ae6d)
Reviewed-on: https://chromium-review.googlesource.com/c/1446404
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#72}
Cr-Branched-From: e510299-refs/heads/master@{#625896}
rogerwang pushed a commit that referenced this pull request Mar 24, 2019
@robbiemc
Fix segfault in ThrottlingURLLoader
809cf1b 
The following sequence of events can cause a segfault:
  1. ThrottlingURLLoader::Start is called to kick off a request
  2. A throttle's WillStartRequest method redirects the request by
     changing ResourceRequest::url
  3. ThrottlingURLLoader::StartNow is called, possibly after deferral
  4. StartNow calls ThrottlingURLLoader::OnReceivedRedirect because the
     request URL was changed in step 2
  5. A throttle's WillRedirectRequest method defers the redirect
  6. When handling the defer, OnReceivedRedirect calls
     client_binding_.PauseIncomingMethodCallProcessing, which segaults
     because client_binding_ is unbound because the request never
     actually started

Bug: 933538
Change-Id: I0f7033d159d34601421da781f7902b6ae207c58a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1511620
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#639178}(cherry picked from commit a86e86d90b35de30ed66b5f0337a082474a01913)
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1519287
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/branch-heads/3729@{#72}
Cr-Branched-From: d4a8972-refs/heads/master@{#638880}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rogerwang rogerwang rogerwang requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@janRucka @rogerwang