Hello andreip,

I'd like you to do a code review.  Please execute
        g4 diff -c 9736781

or point your web browser to
        http://mondrian/9736781

to review the following code:

Change 9736781 by stevebl...@steveblock-gears1 on 2009/01/15 16:17:10 *pending*

        Cleans up some resource linking ...
        - On WinCE, add setup.res to the module DLL - this was inadvertently 
removed when refactoring the autoupdater in CL 8749874).
        - We don't need the custom buttons or alert dialog as resources on 
WinCE as they're not used.
        - We don't need buttons.css as a resource, as it's included inline in 
the dialog HTML.
        - Add ui_resources.res and module.res to the IE Mobile module DLL - 
these were inadvertently removed when renaming IE to IEMOBILE on WinCE in CL 
9700664.
        
        R=andreip
        [email protected]
        DELTA=31  (27 added, 2 deleted, 2 changed)
        OCL=9736781

Affected files ...

... //depot/googleclient/gears/opensource/gears/Makefile#228 edit
... //depot/googleclient/gears/opensource/gears/tools/rules.mk#100 edit
... //depot/googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4#8 edit

31 delta lines: 27 added, 2 deleted, 2 changed

If you can't do the review, please let me know as soon as possible.  During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately.  Visit
http://www/eng/code_review.html for more information.

This is a semiautomated message from "g4 mail".  Complaints or suggestions?
Mail [email protected].
Change 9736781 by stevebl...@steveblock-gears1 on 2009/01/15 16:17:10 *pending*

        Cleans up some resource linking ...
        - On WinCE, add setup.res to the module DLL - this was inadvertently 
removed when refactoring the autoupdater in CL 8749874).
        - We don't need the custom buttons or alert dialog as resources on 
WinCE as they're not used.
        - We don't need buttons.css as a resource, as it's included inline in 
the dialog HTML.
        - Add ui_resources.res and module.res to the IE Mobile module DLL - 
these were inadvertently removed when renaming IE to IEMOBILE on WinCE in CL 
9700664.

Affected files ...

... //depot/googleclient/gears/opensource/gears/Makefile#228 edit
... //depot/googleclient/gears/opensource/gears/tools/rules.mk#100 edit
... //depot/googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4#8 edit

==== //depot/googleclient/gears/opensource/gears/Makefile#228 - 
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/Makefile ====
# action=edit type=text
--- googleclient/gears/opensource/gears/Makefile        2009-01-16 
11:46:14.000000000 +0000
+++ googleclient/gears/opensource/gears/Makefile        2009-01-16 
10:35:58.000000000 +0000
@@ -872,6 +872,10 @@
                $(IE_OUTDIR)/module.res \
                $(NULL)
 
+IEMOBILE_LINK_EXTRAS   += \
+               $(IEMOBILE_OUTDIR)/module.res \
+               $(NULL)
+
 #-----------------------------------------------------------------------------
 # base/npapi
 
@@ -1558,6 +1562,10 @@
                $(IE_OUTDIR)/ui_resources.res \
                $(NULL)
 
+IEMOBILE_LINK_EXTRAS   += \
+               $(IEMOBILE_OUTDIR)/ui_resources.res \
+               $(NULL)
+
 ifeq ($(OS),wince)
 # The string table uses multiple languages, which are not supported by
 # LoadString on WinCE.
@@ -2059,7 +2067,11 @@
                setup.cc \
                $(NULL)
 
+# setup.res contains strings used by both the module DLL and setup.dll.
 IEMOBILE_WINCESETUP_LINK_EXTRAS += \
+               $(IEMOBILE_OUTDIR)/setup.res \
+               $(NULL)
+IEMOBILE_LINK_EXTRAS += \
                $(IEMOBILE_OUTDIR)/setup.res \
                $(NULL)
 
@@ -2082,7 +2094,11 @@
                setup.cc \
                $(NULL)
 
+# setup.res contains strings used by both the module DLL and setup.dll.
 OPERA_WINCESETUP_LINK_EXTRAS += \
+               $(OPERA_OUTDIR)/setup.res \
+               $(NULL)
+OPERA_LINK_EXTRAS += \
                $(OPERA_OUTDIR)/setup.res \
                $(NULL)
 
@@ -2212,6 +2228,7 @@
 OPERA_IDLSRCS += $(NPAPI_IDLSRCS)
 OPERA_HTML_M4SRCS += $(NPAPI_HTML_M4SRCS)
 OPERA_LIBS += $(NPAPI_LIBS)
+# OPERA_LINK_EXTRAS is set independently of NPAPI_LINK_EXTRAS
 endif
 endif
 
@@ -2227,6 +2244,7 @@
 IEMOBILE_IDLSRCS += $(IE_IDLSRCS)
 IEMOBILE_HTML_M4SRCS += $(IE_HTML_M4SRCS)
 IEMOBILE_LIBS += $(IE_LIBS)
+# IEMOBILE_LINK_EXTRAS is set independently of IE_LINK_EXTRAS
 endif
 endif
 
==== //depot/googleclient/gears/opensource/gears/tools/rules.mk#100 - 
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/tools/rules.mk ====
# action=edit type=text
--- googleclient/gears/opensource/gears/tools/rules.mk  2009-01-16 
11:46:14.000000000 +0000
+++ googleclient/gears/opensource/gears/tools/rules.mk  2009-01-16 
00:09:41.000000000 +0000
@@ -1008,6 +1008,8 @@
        $(ECHO) $(THIRD_PARTY_OBJS) | $(TRANSLATE_LINKER_FILE_LIST) >> 
$(OUTDIR)/obj_list.temp
        $(MKDLL) $(DLLFLAGS) $($(BROWSER)_DLLFLAGS) $($(BROWSER)_LINK_EXTRAS) 
$($(BROWSER)_LIBS) $(SKIA_LIB) $(EXT_LINKER_CMD_FLAG)$(OUTDIR)/obj_list.temp
        rm $(OUTDIR)/obj_list.temp
+endif # !symbian
+endif # !android
 
 OPERA_OBJS1 = $(wordlist 1, 100, $(OPERA_OBJS))
 OPERA_OBJS2 = $(wordlist 101, 999, $(OPERA_OBJS))
@@ -1022,8 +1024,6 @@
        $(ECHO) $(THIRD_PARTY_OBJS) | $(TRANSLATE_LINKER_FILE_LIST) >> 
$(OUTDIR)/obj_list.temp
        $(MKDLL) $(DLLFLAGS) $($(BROWSER)_DLLFLAGS) $($(BROWSER)_LINK_EXTRAS) 
$($(BROWSER)_LIBS) $(SKIA_LIB) $(EXT_LINKER_CMD_FLAG)$(OUTDIR)/obj_list.temp
        rm $(OUTDIR)/obj_list.temp
-endif # !symbian
-endif # !android
 
 # Note the use of DLLFLAGS_NOPDB instead of DLLFLAGS here.
 $(OPERA_WINCESETUP_DLL): $(OPERA_WINCESETUP_OBJS) 
$(OPERA_WINCESETUP_LINK_EXTRAS)
==== //depot/googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4#8 - 
c:\MyDocs\Gears1/googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4 
====
# action=edit type=text
--- googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4        
2009-01-16 11:46:14.000000000 +0000
+++ googleclient/gears/opensource/gears/ui/ie/ui_resources.rc.m4        
2009-01-15 16:06:23.000000000 +0000
@@ -41,8 +41,9 @@
 // using something else? It seems to work as is.
 //-----------------------------------------------------------------------------
 
-button.css                   HTML  "ui/common/button.css"
-button.css.end               HTML  {"\0END\0"}
+#ifdef WINCE
+// We don't use custom buttons on WinCE.
+#else
 button_bg.gif                HTML  "ui/common/button_bg.gif"
 button_bg.gif.end            HTML  {"\0END\0"}
 button_corner_black.gif      HTML  "ui/common/button_corner_black.gif"
@@ -51,6 +52,8 @@
 button_corner_blue.gif.end   HTML  {"\0END\0"}
 button_corner_grey.gif       HTML  "ui/common/button_corner_grey.gif"
 button_corner_grey.gif.end   HTML  {"\0END\0"}
+#endif
+
 html_dialog.css              HTML  "ui/common/html_dialog.css"
 html_dialog.css.end          HTML  {"\0END\0"}
 icon_32x32.png               HTML  "ui/common/icon_32x32.png"
@@ -60,8 +63,12 @@
 location_data.png            HTML  "ui/common/location_data.png"
 location_data.png.end        HTML  {"\0END\0"}
 
+#ifdef WINCE
+// We don't use the alert dialog on WinCE.
+#else
 alert_dialog.html            HTML "genfiles/alert_dialog.html"
 alert_dialog.html.end        HTML {"\0END\0"}
+#endif
 permissions_dialog.html      HTML "genfiles/permissions_dialog.html"
 permissions_dialog.html.end  HTML {"\0END\0"}
 settings_dialog.html         HTML "genfiles/settings_dialog.html"

Reply via email to