Issue 8897017: Fix null pointer error.

Issue 8897017: Fix null pointer error. (Closed)

Created:
9 years ago by Jamie

Modified:
9 years ago

Reviewers:
Sergey Ulanov, Wez

CC:
chromium-reviews, jamiewalch+watch_chromium.org, hclam+watch_chromium.org, simonmorris+watch_chromium.org, wez+watch_chromium.org, amit, sanjeevr, garykac+watch_chromium.org, lambroslambrou+watch_chromium.org, sergeyu+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Fix null pointer error. BUG=106860 TEST=Manual Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113882

Patch Set 1 #

Total comments: 3

Patch Set 2 : Review #

Created: 9 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+13 lines, -12 lines)			Patch
M	remoting/webapp/me2mom/client_screen.js	View	1	1 chunk	+1 line, -2 lines	0 comments	Download
M	remoting/webapp/me2mom/client_session.js	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	remoting/webapp/me2mom/toolbar.js	View	1	2 chunks	+11 lines, -9 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Sergey Ulanov

http://codereview.chromium.org/8897017/diff/1/remoting/webapp/me2mom/client_screen.js File remoting/webapp/me2mom/client_screen.js (right): http://codereview.chromium.org/8897017/diff/1/remoting/webapp/me2mom/client_screen.js#newcode126 remoting/webapp/me2mom/client_screen.js:126: if (remoting.toolbar) Maybe change remoting.Toolbar to add onResize handler ...

9 years ago (2011-12-09 22:36:03 UTC) #2

Wez

lgtm http://codereview.chromium.org/8897017/diff/1/remoting/webapp/me2mom/client_screen.js File remoting/webapp/me2mom/client_screen.js (right): http://codereview.chromium.org/8897017/diff/1/remoting/webapp/me2mom/client_screen.js#newcode126 remoting/webapp/me2mom/client_screen.js:126: if (remoting.toolbar) On 2011/12/09 22:36:04, sergeyu wrote: > ...

9 years ago (2011-12-09 22:46:53 UTC) #3

Jamie

9 years ago (2011-12-09 23:11:48 UTC) #4

http://codereview.chromium.org/8897017/diff/1/remoting/webapp/me2mom/client_s...
File remoting/webapp/me2mom/client_screen.js (right):

http://codereview.chromium.org/8897017/diff/1/remoting/webapp/me2mom/client_s...
remoting/webapp/me2mom/client_screen.js:126: if (remoting.toolbar)
On 2011/12/09 22:46:53, Wez wrote:
> On 2011/12/09 22:36:04, sergeyu wrote:
> > Maybe change remoting.Toolbar to add onResize handler to toolbar-container
so
> > that we can keep toolbar-specific logic in one file? I can imagine that in
> some
> > cases we may want toolbar to be located in a corner instead of screen edge
> (e.g.
> > for touch-based UI) - in such cases this code would have to be changed
instead
> > of just swapping Toolbar implementations.
> 
> That sounds reasonable; we should also rename
> ClientSession.onWindowSizeChanged() to onResize(), to match.  If you'd prefer
to
> do that in a follow-up CL, though, that's fine by me..

Both good points. Done.

Expand Messages | Collapse Messages